Refactor facet panel drawing code

teunbrand commented 1 month ago

This PR aims to fix #5482.

Briefly, the panel drawing code is now divided over 3 Facet ggproto methods: init_gtable(), attach_axes() and attach_strips().

My main gripe with the previous code was that it was hard to follow along with what it was doing. I think reasons why it was hard for me to follow are the following:

The code was alternating between the three tasks, for example facet_grid() was first setting up axes, then setting up strips, then initialising the table, then adding the axes and finally adding the strips.
Some pieces of code were 4 if blocks deep which is hard to keep track of.

The primary benefit is that it should now be easier to 'just' tweak the axes, or 'just' tweak the strips in extensions. Quick benchmark showed no speed difference between current main and this PR.

teunbrand commented 1 month ago

TODO: try unifying draw_panels()

teunbrand commented 1 month ago

The draw_panels() method is now part of the base Facet class, as well as the init_gtable() method. FacetWrap still has to ggproto_parent() the panel drawing code due to interactions with CoordFlip that need to be adressed. I've left FacetNull as-is, to keep using its highly simplified method.

teunbrand commented 2 weeks ago

CI is a bit out of whack because non-MacOS builds use most recent {cli} 3.6.3 whil I presume MacOS still uses an older version. The new {cli} affects some error message snapshots, which are fine and unrelated to this PR. I'm going to go ahead and merge this anyway, as I trust that at some point MacOS will also be able to use the new {cli}.

tidyverse / ggplot2

Refactor facet panel drawing code #5917