-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Cherry pick PR826 from Ladybird #26349
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
base: master
Are you sure you want to change the base?
Conversation
(cherry picked from commit 6be559f6394b210ddd3db48fb158d7410adbd2a1)
Manual cherry-pick of e8b7c88 (without Skia and with DisplayListPlayer{CPU,GPU} support).
This change is a preparation for the upcoming changes where display list will be nested and the same display could be owned by multiple display list items. (cherry picked from commit 50ab564)
Before this change, "background-clip: text" was implemented by saving a Vector<Gfx::Path> of all glyphs needed to paint a mask for the background. The issue with this approach was that once glyphs were extracted into vector paths, the glyph rasterization cache could no longer be utilized. With this change, all text required for mask painting is saved in a nested display list and rasterized as a regular text. (cherry picked from commit 67d68ea; amended for DisplayListPlayerCPU compatibility)
|
(6be559f6 is the first commit of LadybirdBrowser/ladybird#5390) |
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.
Very cool, thanks! Just one question:
| } | ||
|
|
||
| auto& command = m_commands[next_command_index++].command; | ||
| auto& command = commands[next_command_index++].command; |
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.
The original PR also touches what's line 149 in this file. Do we not need that? Are we missing an earlier cherry-pick?
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.
So the executor stack mechanism was removed upstream in 0b48c1e (while we still need it). Thus the original commit doesn't have to deal with any nested players, so it can just call would_be_fully_clipped_by_painter and executor_method via this (whereas we need to go through current_executor, so those lines incidentally stay the same).
(and also 6be559f6, because the second commit in this PR needs that).
This was fairly non-trivial (since
DisplayListPlayerCPU.{cpp,h}don't exist upstream), and is almost an NFC, but this should hopefully unblock some other commits should we choose to cherry-pick those.The visual differences in
css-background-clip-text.pnglook similar to the ones upstream in LadybirdBrowser/ladybird#826, so that seems more-or-less fine I guess.