Improvements to geom_label()

[teunbrand]

This PR aims to fix #2785 and fix #4753.

Briefly:

• Viewports of individual labels rotate according to the angle aesthetic.
• Textboxes now take grobDescent(text_grob) into account.
• The viewport sets a fontsize parameter, that allows the typical unit(0.25, "lines") label padding to get appropriate text sizes.

The 2nd and 3rd points lead to visual changes, but I'd argue that these are appropriate.

The reprex provided in #2785 (comment)

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

data <- data.frame(
  text = c('abc', 'def', 'gjq'),
  angle = c(0, 45, 90),
  x = c(1, 2, 3)
)

ggplot(data, aes(x, label = text, angle = angle)) +
  geom_label(aes(y = 1), label.padding = unit(0, "lines"), size = 10) +
  geom_label(aes(y = 2), size = 10) + 
  xlim(0.5, 3.5) + 
  ylim(0.5, 2.5)

The reprex from #4753 (comment)

ggplot() +
  annotate(
    "label",
    x = 0, 
    y = 0, 
    label = "text",
    hjust = c(0, 1),
    size = 20
  )

^{Created on 2022-11-05 by the reprex package (v2.0.1)}

[teunbrand]

Hi @clauswilke, might I ask you to review this PR if you have some time to spare? I think you have the most experience with textboxes, so I'd welcome your suggestions.

[clauswilke]

I think the problem you'll have is that if you have one label abc and one label gqj and place them side by side they will have different heights. You have to decide whether that's correct or not. It's probably better than the current situation.

I've always felt that text at a given font size should have a defined ascent and descent regardless of content (and in fact, that's how ascent and descent are defined in typography), but grid doesn't work that way if I recall correctly, and as your examples show.

[clauswilke]

Notice how in a web browser these two labels have the same height: ace Mqj

Screenshot:

[teunbrand]

Thanks that is a keen observation I hadn't thought of myself, so that is useful! I guess I could use ggplot2:::font_descent() instead to calculate the descent. Do you think it is worth it to make a special case for text that doesn't have descenders in any of the labels?

In your example, it also seems like the space above M and space below q/j are unequal, which feels more natural. In this PR, padding is the same in every direction, but this makes me think that perhaps there are better options to display text more naturally. Would it make sense to adapt the label.padding argument to support margin() input?

[thomasp85]

Just to add my own opinion, we should def use the same descent irrespective of the content of the text box, and font_descent() is the way to get it. I think titleGrob() already do a lot of the same work under the hood.

Not sure if supporting margin() in the padding is a feature that is worth the added complexity though, but no strong opinions there

[teunbrand]

Thanks for the thoughts. I'll explore around a bit to see to what degree titleGrob()'s logic can be re-used here, in which case the added complexity for margin() might be ... marginal ...

[teunbrand]

Some updates:

• Using titleGrob() for the labels simplifies the label drawing, as it also supplies appropriate grob widths/heights for the box.
• This made it also easy to implement label.padding = margin(...) input.
• I removed the makeContent method, because I don't think we need it. I've looked at the history of it, but I couldn't figure out what exactly should be calculated at grob drawing time. If there is a good reason for it, I can put it back, but if there isn't, we've reduced the complexity a bit.

Showing the margin argument and descent calculations:

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

data <- data.frame(
  text = c('abc', 'def', 'gjq'),
  angle = c(0, 45, 90),
  x = c(1, 2, 3)
)

ggplot(data, aes(x, label = text, angle = angle)) +
  geom_label(aes(y = 1), label.padding = unit(0, "lines"), size = 10) +
  geom_label(
    aes(y = 2), size = 10,
    label.padding = margin(t = 20, r = 0, b = 5, l = 5)
  ) + 
  xlim(0.5, 3.5) + 
  ylim(0.5, 2.5)

^{Created on 2023-01-16 with reprex v2.0.2}

[thomasp85]

have you tested the impact on performance? Not that geom_label() was ever that performant.

When this is merged it would be good to at least inform ggrepel so they can test whether the new label grobs affect their package

[teunbrand]

have you tested the impact on performance?

It is good that you asked, because I just did, and printing a plot with 100 labels takes 2.5x longer with this PR (695ms vs 268ms). Most of this extra time is spend doing viewport operations. I think I should spend some time trying to make this more efficient.

inform ggrepel

I will, but I ran their unit tests and they all pass with this PR. ggrepel::geom_label_repel() doesn't use labelGrob(), so this makes sense to me.

[thomasp85]

I don't think titleGrob() has ever been subjected to a high degree of performance scrutiny since a plot only ever consist of a few titles. Seeing if this could be improved would be worthwhile

[clauswilke]

If I remember correctly from writing gridtext, I felt that titleGrob was overly complex and convoluted. You don't need a second viewport just to add margins around text. You can just make a bigger box and put the text in the right location so there are margins. So there may be efficiency gains from a refactoring.

ggplot2/R/margins.R

Lines 107 to 156 in f139f40

	add_margins <- function(grob, height, width, margin = NULL,
	gp = gpar(), margin_x = FALSE, margin_y = FALSE) {
	if (is.null(margin)) {
	margin <- margin(0, 0, 0, 0)
	}
	if (margin_x && margin_y) {
	widths <- unit.c(margin[4], width, margin[2])
	heights <- unit.c(margin[1], height, margin[3])
	vp <- viewport(
	layout = grid.layout(3, 3, heights = heights, widths = widths),
	gp = gp
	)
	child_vp <- viewport(layout.pos.row = 2, layout.pos.col = 2)
	} else if (margin_x) {
	widths <- unit.c(margin[4], width, margin[2])
	vp <- viewport(layout = grid.layout(1, 3, widths = widths), gp = gp)
	child_vp <- viewport(layout.pos.col = 2)
	heights <- unit(1, "null")
	} else if (margin_y) {
	heights <- unit.c(margin[1], height, margin[3])
	vp <- viewport(layout = grid.layout(3, 1, heights = heights), gp = gp)
	child_vp <- viewport(layout.pos.row = 2)
	widths <- unit(1, "null")
	} else {
	widths <- width
	heights <- height
	return(
	gTree(
	children = grob,
	widths = widths,
	heights = heights,
	cl = "titleGrob"
	)
	)
	}
	gTree(
	children = grob,
	vp = vpTree(vp, vpList(child_vp)),
	widths = widths,
	heights = heights,
	cl = "titleGrob"
	)
	}

[teunbrand]

Yeah I did some testing and a lot of the performance loss is by using titleGrob and not necessarily the viewport that I use here to rotate the labels. Perhaps it makes more sense to put in another PR where we refactor titleGrob() instead of trying to optimise this implementation of geom_label().

[teunbrand]

Performance hit is now only 26% slower (100 labels, 344ms vs 273ms), which is an improvement over the 160% slower we had earlier. I find this an acceptable trade-off for the features it offers, so I'll merge this.