Skip to content

Expose shape argument in geom_curve() #6523

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fmarotta
Copy link

Hi! there was some discussion in issue #5998 about exposing the shape argument in geom_curve(). This PR attempts to do that.

I've run some tests to understand the effect of the curveGrob() arguments, and it seems that elbow-style lines can indeed be achieved with the following combination: curvature = 1, ncp = 1, angle = 90, square = TRUE (see the reprex below).

library(grid)
library(gridExtra)
grobs <- list(
  `curvature=0.1` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 0.1),
  `curvature=1` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 1),
  `ncp=5` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 1, ncp = 5),
  `ncp=1` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 1, ncp = 1),
  `angle=60` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 1, ncp = 1, angle = 60),
  `angle=90` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 1, ncp = 1, angle = 90),
  `square=FALSE` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 1, ncp = 1, angle = 90, square = FALSE),
  `square=TRUE` = curveGrob(x1 = 0.2, x2 = 0.8, y1 = 0.1, y2 = 0.6, shape = 0, curvature = 1, ncp = 1, angle = 90, square = TRUE)
)
grobs_with_titles <- mapply(function(grob, title) {
  grobTree(
    grob,
    textGrob(title, x=0.5, y=0.7, gp=gpar(fontsize=10))
  )
}, grobs, names(grobs), SIMPLIFY = FALSE)
grid.newpage()
grid.arrange(grobs = grobs_with_titles, ncol = 2, nrow = 4)

Created on 2025-06-19 with reprex v2.1.1

The PR adds the shape argument, but I also had to change the square argument. square is not exposed in geom_curve() but it is hard-coded to FALSE. The grid.curve() documentation says that

When ncp is 1 and angle is 90, this is typically TRUE, otherwise this should probably be set to FALSE.

so in this PR I follow such prescription.

Using square = FALSE results in "tilted" elbows, whereas when square = TRUE the lines are horizontal and vertical (see reprex above). The tilted effect can still be achieved by setting angle to something different than 90.

Copy link
Collaborator

@teunbrand teunbrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me in principle. Would you mind adding an example to the docs using the new parameter (or integrating it in some example)?

We're currently in the process of preparing a release, so we've temporily halted adding new features. If all is well we can consider merging this after the release (and perhaps a hotfix) is out.

Comment on lines +15 to +33
df <- data.frame(x = c(1, 3), xend = c(2, 4), y = c(0, 1), yend = c(2, 1.5))

p_0 <- ggplot(df, aes(x, y, xend = xend, yend = yend)) +
geom_curve(arrow = arrow(), shape = 0, ncp = 1, curvature = 1)

# This will use `square = FALSE` in curveGrob because angle != 90
p_0_not_square <- ggplot(df, aes(x, y, xend = xend, yend = yend)) +
geom_curve(arrow = arrow(), shape = 0, ncp = 1, curvature = 1, angle = 60)

p_1 <- ggplot(df, aes(x, y, xend = xend, yend = yend)) +
geom_curve(arrow = arrow(), shape = 1)

p_m1 <- ggplot(df, aes(x, y, xend = xend, yend = yend)) +
geom_curve(arrow = arrow(), shape = -1, angle = 40)

expect_doppelganger("shape=0 geom_curve", p_0)
expect_doppelganger("shape=0 geom_curve", p_0_not_square)
expect_doppelganger("shape=1 geom_curve", p_1)
expect_doppelganger("shape=-1 geom_curve", p_m1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this 1 plot with 4 geom_curve() layers, each with something like aes(colour = "not square") that is indicative of the setting, so that it sort of becomes self-descriptive?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the tests indicate that the snapshot would need updating. Did you have an older svglite/vdiffr version when you generated the snapshots?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants