Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polygon segments with coord_polar() near 0 #5023

Open
Darxor opened this issue Oct 27, 2022 · 5 comments · May be fixed by #6226
Open

Polygon segments with coord_polar() near 0 #5023

Darxor opened this issue Oct 27, 2022 · 5 comments · May be fixed by #6226
Labels
coord 🗺️ visual change 👩‍🎨 Rendering change that will affect look of output

Comments

@Darxor
Copy link

Darxor commented Oct 27, 2022

In some (undeniably edge, but still real) cases current version of ggplot2 draws polygons near 0 with too few segments. Short example is this:

library(ggplot2)

df <- data.frame(
  x = 1:3,
  y = c(2, 4, 20)
)

ggplot(df, aes(x, y, fill = factor(y))) +
  geom_col(width = 1, show.legend = FALSE) +
  scale_y_continuous(limits = c(0, 50)) +
  coord_polar() +
  theme_void()

Which leads to a plot like this (heavily zoomed in), where you can see red bar only has two outer segments and looks like a diamond, rather than a segment of a circle.
lowpoly

This becomes more obvious, when displaying plots in high real-world resolution - on a big screen, projector, etc., because it doesn't depend on a graphics device.

I've found out (from a stackoverflow post discussing this very same issue) that changing the default value of segment_length in coord_munch() from 0.01 to 0.002 improves quality (see image below), and doesn't seem to degrade performance too much (in my not-very-thorough benchmarks it was down 0-4%).
highpoly

Given that, my proposal is to change the default value of segment_length to something smaller.

@Darxor
Copy link
Author

Darxor commented Oct 27, 2022

Possible alternative:
Add a small / decreasing value to dist inside munch_data()

ggplot2/R/coord-munch.r

Lines 53 to 54 in a58b48c

# How many endpoints for each old segment, not counting the last one
extra <- pmax(floor(dist / segment_length), 1)

This does the trick and has near-zero performance hit. Coefficient "2" was chosen arbitrarily, based solely on the looks of the result.

extra <- pmax(floor((dist + (1 - exp(-dist * 2))) / segment_length), 1)

@teunbrand teunbrand added coord 🗺️ visual change 👩‍🎨 Rendering change that will affect look of output labels Jan 7, 2023
@Darxor
Copy link
Author

Darxor commented Jul 26, 2023

I've tried to tackle this issue, but I don't think my geometry is enough to solve this properly. A more general solution should rely on curvature (rapid change over small distance = more subsegments). Perhaps there is a ready-made algorithm for this.

Maybe its wildly wrong, but its my best attempt. Seems to fix an issue for lines near origin in polar coordinates, and doesn't seem to explode with subdivisons elsewhere:

# Calculate the change in angle of the tangent vector
d_angle <- abs(diff(atan2(diff(data$x), diff(data$y))))
d_angle <- c(d_angle, d_angle[1])

extra <- pmax(ceiling(dist * d_angle / segment_length), 1)

Maybe segment_length could be exposed to end-user somehow?
I would gladly use it, because now my solution for these types of charts is to create subdivions myself before plotting or to monkey-patch ggplot2.

@teunbrand
Copy link
Collaborator

I think it probably might be more straightforward to just 'lie' about at what radius point is in the distance calculation.
The code below is for demonstration purposes only and not a recommendation for how to write ggproto classes, but it shows what I mean by lying. We can get a smoother finish at near-minimum radius points by instead of rescaling to [0, 1], we simply rescale to [0.2, 1] so that the distance calculation thinks the radius is larger.

library(ggplot2)

df <- data.frame(
  x = 1:3,
  y = c(2, 4, 20)
)

cp <- coord_polar()
new <- ggproto(
  NULL, cp,
  # similar to CoordPolar$distance()
  distance = function(self, x, y, details) {
    if (self$theta == "x") {
      r <- scales::rescale(y, from = details$r.range, to = c(0.2, 1)) # changed
      theta <- ggplot2:::theta_rescale_no_clip(self, x, details)
    } else {
      r <- scales::rescale(x, from = details$r.range, to = c(0.2, 1)) # changed
      theta <- ggplot2:::theta_rescale_no_clip(self, x, details)
    }
    ggplot2:::dist_polar(r, theta)
  }
)


ggplot(df, aes(x, y, fill = factor(y))) +
  geom_col(width = 1, show.legend = FALSE) +
  scale_y_continuous(limits = c(0, 50)) +
  new +
  theme_void()

Created on 2023-07-26 with reprex v2.0.2

@Darxor
Copy link
Author

Darxor commented Jul 26, 2023

Huh, its great at its simplicity! Thanks!

But is it something that would be added into default coord_polar() or would I have to re-implement it for my usage?

@teunbrand
Copy link
Collaborator

I have no idea about what adverse effects this might have as I'm not too familiar with calculating polar distances, but visual tests don't seem too much affected. Until I better understand that calculation, I think I'll hold off on putting in a PR. You're of course free to take this experimental adjustment for your own usage.

@teunbrand teunbrand linked a pull request Dec 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coord 🗺️ visual change 👩‍🎨 Rendering change that will affect look of output
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants