-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Refactor titleGrob()
#5273
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
Refactor titleGrob()
#5273
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM... Can I get you to run the visual tests on a non-arm64 computer (assuming you are working on a M1 Mac). I've found the chip architecture influences these weird rounding issues and all the large theme tests never matches on my machine
These visual tests were run on a windows machine with an x64-based processor. Bit weird that CPU architecture matters for this rounding. |
This PR aims to fix a performance bottleneck for #5030 as identified in #5030 (comment).
Briefly, it introduces the following changes:
titleGrob
grobs no longer have viewports.textGrob
is adjusted to get the same effect.titleGrob()
constructor now does the job of what previously was done bytitle_spec()
andadd_margins()
.assemble_strip()
no longer needs to awkwardly hassle with all these viewports.title_spec()
,add_margins()
,margin_width()
,margin_height()
have become obsolete. I've removed them, but we might expect this to bite us during revdepchecks because they're missing.Benchmark below shows a ~4 fold reduction in drawing time for 574 titleGrobs.
Created on 2023-04-17 with reprex v2.0.2