Skip to content

Conversation

@InvalidUsernameException
Copy link
Contributor

@InvalidUsernameException InvalidUsernameException commented May 29, 2025

This fixes rendering bugs on https://www.fangamer.com/, https://null.com/games/bleakwood and https://t-shaped.nl/, among others. For a demonstration of the change, see the videos in #3646.

This is the third attempt at fixing this problem, the prior ones are in the stale PRs #3646 and #4312. However, this PR does not built on top of those changes. The approach used there was not correct. The original approach was to apply the scroll-offset of the mask-images while recording the display list. This does not work because a single display list may be rendered multiple times with different scroll offsets if the page does not cause any invalidation during scrolling.

As it turns out, applying of scroll-offsets to mask-images during display list replay is already present. It simply did not work because the scroll-frame ids were not set up correctly. For more details on the new approach see the commit message.

When recording the display list for a stacking context, the following
operations (relevant to this bug) happened:
* push a stacking context
  * as part of that push a None-value to the scroll frame id stack
* apply filters
* apply masking
* paint recursively

This meant that mask-images were always recorded without scroll frame
id, causing them to be painted without any scroll offset. As a result
mask-images would break as soon as the website using them was scrolled.

Instead, push to the scroll frame id stack later to solve the problem:
* push a stacking context
* apply filters
* apply masking
* push a None-value to the scroll frame id stack
* paint recursively
Copy link
Member

@kalenikaliaksandr kalenikaliaksandr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fix looks very reasonable. thanks for figuring that out!

@kalenikaliaksandr kalenikaliaksandr enabled auto-merge (rebase) May 29, 2025 19:13
@kalenikaliaksandr kalenikaliaksandr merged commit 164afdc into LadybirdBrowser:master May 29, 2025
7 checks passed
@InvalidUsernameException InvalidUsernameException deleted the mask-image-with-scroll-offset branch May 29, 2025 20:27
@trflynn89
Copy link
Contributor

The test added here is flaking in CI:
https://github.com/LadybirdBrowser/ladybird/actions/runs/15332846648/job/43143535195

The result of the test seems to be a white blank page:
libweb-test-artifacts-Linux-Sanitizer_CI-Clang-.zip

@InvalidUsernameException
Copy link
Contributor Author

The test added here is flaking in CI: https://github.com/LadybirdBrowser/ladybird/actions/runs/15332846648/job/43143535195

Unfortunate, I even commented on that in #3646, but then couldn't reproduce it locally, so I thought it was fine.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants