Skip to content

Conversation

@implicitfield
Copy link
Contributor

(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.png look similar to the ones upstream in LadybirdBrowser/ladybird#826, so that seems more-or-less fine I guess.

kalenikaliaksandr and others added 4 commits October 31, 2025 21:57
(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)
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Oct 31, 2025
@nico
Copy link
Contributor

nico commented Nov 2, 2025

(6be559f6 is the first commit of LadybirdBrowser/ladybird#5390)

Copy link
Contributor

@nico nico left a 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;
Copy link
Contributor

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?

Copy link
Contributor Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👀 pr-needs-review PR needs review from a maintainer or community member

3 participants