Minor ticks

[teunbrand]

This PR aims to fix #4387.

Briefly, it adds 3 arguments to guide_axis():

• minor.ticks which takes an element_{line/blank} or TRUE/FALSE to draw minor ticks or not.
• minor.length to control the length of the minor ticks, relative to the theme setting.
• major.length to control the length of the major ticks, relative to the theme setting.

Less briefly:
I wasn't particularly keen to add the whole parade of axis.ticks.minor.x.top and axis.ticks.length.minor.x.top etc. theme settings for the minor axes. Mostly because I was being lazy, but also the theme arguments are busy enough as it is. The current options provide the same flexibility though.

I found out that the axis ticks were anchored incorrectly, which you of course don't see if you have one tick size. As a result of fixing this, the x/y xend/yend order in the colourbar/bin guides' ticks are swapped, which is the cause of all the changes in the visual tests (the alternative was to do this in the axis, which would affect every plot). Note that this affects the svgs, but not what is drawn to screen.

Here is a bit of a contrived example to show the minor ticks, and that they could also work for (fake) discrete axes, if only they had get_breaks_minor() implementations. Turning the major ticks off gives the discrete labels nice 'fences' between their areas on the plot.

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

ggplot(mpg, aes(match(class, sort(unique(class))), displ, group = class)) +
  geom_boxplot() +
  # Fake discrete axis
  scale_x_continuous(
    breaks = 1:length(unique(mpg$class)),
    labels = sort(unique(mpg$class)),
    guide = guide_axis(minor.ticks = TRUE, major.length = 0)
  ) +
  guides(y = guide_axis(minor.ticks = element_line(colour = "red"))) +
  theme(axis.ticks.length = unit(8, "pt"))

^{Created on 2023-04-26 with reprex v2.0.2}

[thomasp85]

I would very much suggest we move all these visual settings to the theming. It is where they belong even if the arguments feel a bit much. axis.ticks.minor would default to element_blank() and the rest inherits from that

We are kinda stuck with theming being applicable from both the guide constructor and theme with some of the old guides but there is no reason to repeat old mistakes :-)

[teunbrand]

Yeah I can see why they should live in the theme. The only problem I could foresee is that at the time the information is taken from the scale (in Coord$train_panel_guides()), it is unknown whether minor ticks should be drawn. We could of course always extract the minor positions, but I'm not sure (yet) what the performance implications of that are.

Edit: Another drawback of always extracting minor positions, is that the axis capping from #5289 will snap to minor positions even if the minor ticks aren't drawn.

[teunbrand]

The minor ticks currently need to be set TRUE/FALSE at the guide level, but styling comes from the theme.

Couple thoughts:

• Ideally, the minor tick length should be a fraction of the major tick length. We'd need to implement Suggestion: rel-class can inherit value from unit-class theme elements #3951 to do this properly, so that the minor tick lengths can inherit from their major tick equivalents.

• A neat thing we could do, is to derive from the scale whether the user had supplied minor breaks, and if they did, draw minor breaks. That'd be a visual change though.

• We might let the minor tick setting be purely derived from the theme, but we'd need so provide the theme through the coord$setup_panel_guides() method.

[thomasp85]

Did you ever look into the performance implications of always extracting minor tick positions? I have a feeling it is neigh zero...

Also, should we look into #3951 before merging this?

[teunbrand]

performance implications of always extracting minor tick positions

Currently, extracting minor positions too is about 2x as expensive as just major positions: it goes from ~1ms to ~2ms for guide training.

Also, should we look into #3951 before merging this?

Yeah let's try

[teunbrand]

I was surprised to learn that apparently ggplot2's theme system works perfectly fine when a el_def() has multiple parents, despite having no elements with multiple parents.

In any case, now the leaf axis.minor.ticks.length.{aes}.{position} nodes have axis.minor.ticks.length.{aes} as one parent, and axis.ticks.length.{aes}.{position} (the major equalent) as the other parent. This allows us to inherit a <rel> through the first parent, and resolve this as a proper <unit> through the second parent. I'm pretty happy about this.

Small demo

devtools::load_all("~/packages/ggplot2")
#> ℹ Loading ggplot2

minor_axis <- guide_axis(minor.ticks = TRUE)

ggplot(mpg, aes(displ, hwy)) +
  geom_point() +
  guides(x = minor_axis, x.sec = minor_axis,
         y = minor_axis, y.sec = minor_axis) +
  theme(
    axis.ticks.length = unit(1, "cm"),
    axis.ticks.length.y = rel(0.5),
    axis.minor.ticks.length = rel(0.5),
    axis.minor.ticks.length.x.top = unit(1, "cm"),
    axis.minor.ticks.length.y.right = rel(4)
  )

^{Created on 2023-09-12 with reprex v2.0.2}

[thomasp85]

LGTM