Add strategies to deal with overlapping text in draw_axis()

[paleolimbot]

This is a PR as part of #3322 to build in some basic strategies to the default axis renderer (draw_axis()). The inspiration for this PR is #3281, although the issue can't be closed until there is actually a way to change the arguments to draw_axis(), which is the almost next step for #3322.

The three strategies are (1) recursively prioritize the "middle" label, and pass check.overlap = TRUE to textGrob(), (2) rotate the axis text, and (3) dodge the text into multiple rows/columns. These are the strategies I chose because they can operate independent of each other, don't require overhauling the existing axis code, and don't cause any visual changes with the default parameters. There are probably better strategies that can be implemented when axis guides are extensible (I imagine that @clauswilke's gridtext would introduce a number of alternatives).

A reprex so that any/all can play around with the new options:

library(ggplot2)
library(grid)
library(gtable)

# theme with which axes should be tested
theme_test_axis <- theme_test() + theme(axis.line = element_line(size = 0.5))

# function to test several axes at once
test_draw_axis <- function(n_breaks = 10,
                           # make long labels
                           labels = function(breaks) scales::comma(breaks * 1e7),
                           positions = c("top", "bottom"),
                           theme = theme_test_axis,
                           ...) {
  
  break_positions <- seq_len(n_breaks) / (n_breaks + 1)
  break_labels <- labels(seq_len(n_breaks))
  
  # create the axes
  axes <- lapply(positions, function(position) {
    ggplot2:::draw_axis(break_positions, break_labels, axis_position = position, theme = theme, ...)
  })
  axes_grob <- gTree(children = do.call(gList, axes))
  
  # arrange them so there's some padding on each side
  gt <- gtable(
    widths = unit(c(0.05, 0.9, 0.05), "npc"),
    heights = unit(c(0.05, 0.9, 0.05), "npc")
  )
  gt <- gtable_add_grob(gt, list(axes_grob), 2, 2, clip = "off")
  plot(gt)
}

test_draw_axis(check.overlap = TRUE)

test_draw_axis(angle = 45)

test_draw_axis(n_dodge = 2)

^{Created on 2019-06-19 by the reprex package (v0.2.1)}

[hadley]

This is looking very promising!

Do you also want to do the edge alignment for first and last labels in this PR, or will that come later?

[paleolimbot]

Aligning the end labels is a bit of extra code, but if you think it's worth it, I think what I have works:

library(ggplot2)
library(grid)
library(gtable)

# theme with which axes should be tested
theme_test_axis <- theme_test() + theme(axis.line = element_line(size = 0.5))

# function to test several axes at once
test_draw_axis <- function(n_breaks = 10,
                           break_positions = seq_len(n_breaks) / (n_breaks + 1),
                           labels = function(breaks) scales::comma(breaks * 1e7),
                           positions = c("top", "bottom"),
                           theme = theme_test_axis,
                           ...) {
  
  break_labels <- labels(seq_along(break_positions))
  
  # create the axes
  axes <- lapply(positions, function(position) {
    ggplot2:::draw_axis(break_positions, break_labels, axis_position = position, theme = theme, ...)
  })
  axes_grob <- gTree(children = do.call(gList, axes))
  
  # arrange them so there's some padding on each side
  gt <- gtable(
    widths = unit(c(0.05, 0.9, 0.05), "npc"),
    heights = unit(c(0.05, 0.9, 0.05), "npc")
  )
  gt <- gtable_add_grob(gt, list(axes_grob), 2, 2, clip = "off")
  plot(gt)
}

test_draw_axis(
  break_positions = c(0.01, 0.99),
  positions = c("top", "bottom"),
  angle = 0,
  align_ends = TRUE
)

test_draw_axis(
  break_positions = c(0.01, 0.99),
  positions = c("left", "right"),
  angle = 90,
  align_ends = TRUE
)

^{Created on 2019-06-20 by the reprex package (v0.2.1)}

[clauswilke]

@paleolimbot I'm trying to figure out how exactly this code works, and I can't map it to my mental model of how I think this should be done. Could you help me figure out whether I'm thinking about this wrong?

Normally, in grid, if we want to write a grob that can make drawing decisions based on the dimensions of text, we need to do this inside a makeContent() function for an appropriate grob class, so that the drawing decisions adjust appropriately in interactive use. For an example, see my implementation of isolines_grob(): https://github.com/wilkelab/isoband/blob/master/R/isolines-grob.R

So for an axis, I would expect that there is a grob class that draws all axis labels, and inside the makeContent() function for that class the code would calculate the dimensions of the labels, drop labels if needed, rotate them, etc. I can't find this structure in your current pull request, but maybe I'm overlooking something.

[paleolimbot]

You're right, I don't use the custom grob + makeContent() method. In fact, I think this PR could be summarised as "all the things you can do for axis labels that don't require a makeContent S3". I got the aligned labels to work using the fact that min.unit(), max.unit(), grobWidth() and grobHeight() are lazy, so that you can do:

first_label <- element_grob(..., label = break_labels[1])
first_label_width <- grobWidth(first_label)
break_positions[1] <- max(break_positions[1], unit(0, "npc") + first_label_width * 0.5)

Of course, this fails on R 3.2, which I'm working on tracking down.

[paleolimbot]

It appears that assigning part of a unit vector with a lazy unit calculation didn't work until R 3.3. I don't know enough about grid to know whether or not relying on this kind of lazy unit math is a good idea, especially because I'm measuring a grob that will never be drawn. @clauswilke - any opinions?

[clauswilke]

Yeah, I don't know. Until this morning I didn't even know this was possible, though now that I think about it it can see how it can work.

I'm a bit worried that the text rendering code in ggplot2 is not well encapsulated and makes inconsistent assumptions in different places (e.g., the whole titleGrob() code assumes that we're drawing only a single text label [as otherwise adding descent data to grob width/height doesn't make much sense], but then it's sometimes used to draw multiple labels at once). As we rewrite drawing code, we should make sure we're moving forward and are cleaning things up, not making code even more complex and confusing.

If I understand correctly, your code draws separate text labels with separate grobs for each axis label, so that's good. It means we should be able to use a different text drawing function in the future without too much difficulty.

I'm a bit behind on where I wanted to be with gridtext, but I think I'll quickly (over the next week or so) hack together a simple replacement text grob that we can use for testing. If it can be used instead of the current titleGrob() for your axis drawing code, then we're on the right track. If not, then the failure mode will likely tell us where we're going wrong.

[paleolimbot]

This PR is about adding features to the axis drawing code, so that when axis guides are customizable, there's something to customize. I think that getting the details of text right is important, but it's outside the scope of what I'm trying to do here, unless I'm adding a feature that will have to be removed when titleGrob()s are replaced by something more appropriate. While the implementations of n_dodge, angle, align_ends, and check.overlap would probably change, I think they would still be relevant, useful, and possible with a different underlying grob.

I don't personally think that the end alignment is that useful or looks very nice, so I could easily be convinced to remove that option if the code is getting too muddy (it's also quite slow because of the lazy unit stuff).

[hadley]

If the end unit alignment is the only thing that needs lazy units, I think we should just remove it.

[clauswilke]

Aren't lazy units required for all three features, since overlap can also change dynamically as the plot window is resized?

Dewey, what I was trying to get at is the following: If the code is modular, then it should be possible to provide a drop-in replacement for element_text() that provides more capabilities without requiring any changes in the underlying ggplot2 code base. This currently is possible for things like plot title, subtitle, axis title, etc., but it is not possible for axis tick labels. If there's a chance that after your rewrite, it will also be possible for axis tick labels, then we should make this happen.

I realize this will make more sense with an example, and I'll put one together over the weekend.

[paleolimbot]

Done! check.overlap is built in to textGrob(), so it's very fast and doesn't require any lazy unit code (or any code, really, just making sure that the argument gets passed all the way through the theme/element/titleGrob system). Axis labels are just (vectorized) rendered theme elements, so anything you do for those will work for the code I've written.

[clauswilke]

Axis labels are just (vectorized) rendered theme elements, so anything you do for those will work for the code I've written.

The vectorization has been the problem, actually. But never mind, your refactoring will make it possible to plop in different axis drawing code in the future, so some of the limitations I see could be addressed this way then.

[clauswilke]

(Basically, it is not clearly defined whether element_text() creates one or more individual labels or whether it creates one container with one or more labels inside. This distinction causes inconsistencies and weird effects when we try to replace element_text() with something else.)

[paleolimbot]

I'm with you...rendering a vectorized text element is how the axis labels have always been drawn, and I'm trying to be very careful about never inducing any visual changes since the axes are drawn on pretty much every visual test. I think it should be fixed, but I think this isn't the PR in which it should be done.

[hadley]

Not sure if it's worth a news bullet now, or if we should wait until it's user accessible.

[paleolimbot]

I forgot about the NEWS bullet - I think I'd prefer to write a news bullet later on when I can give an example of how one might use the improvements.

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/