Skip to content
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

LibWeb: Draw floating replaced elements more correctly #3430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

InvalidUsernameException
Copy link
Contributor

@InvalidUsernameException InvalidUsernameException commented Feb 2, 2025

Previously floating replaced elements were drawn incorrectly and also twice.

This was originally discovered because of the css/css-images/object-fit-* set of tests from the WPT suite. Those use dashed borders to draw floating replaced elements. The dashed borders use subpixel rendering, so rendering them twice made the borders darker than they were supposed to be. The tests themselves cannot be imported though because a) they either fail for other, unrelated reasons, or b) they only pass because both test and ref page don't render the intended content yet.

@InvalidUsernameException InvalidUsernameException force-pushed the floats-as-own-stacking-context branch from 94432f1 to 7a76914 Compare February 3, 2025 22:01
@InvalidUsernameException InvalidUsernameException force-pushed the floats-as-own-stacking-context branch 4 times, most recently from 43dbd91 to 4870cf7 Compare February 5, 2025 13:04
@InvalidUsernameException
Copy link
Contributor Author

Rebased on master to fix CI.

@InvalidUsernameException InvalidUsernameException force-pushed the floats-as-own-stacking-context branch from 4870cf7 to 3468fca Compare February 15, 2025 12:38
@@ -0,0 +1,27 @@
<!DOCTYPE html>
Copy link
Member

Choose a reason for hiding this comment

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

I compared this page on build from master vs other browsers and it seems to visually already be painted correctly. could we add a test that demoes correctness improvement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess something changed then in the commits on master that I rebased over. It rendered incorrectly on master when I first opened the PR. I'll take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test still reproduces the original problem for me as of 29d7463.

Master This PR/other browser
wrong correct

I've rebased on that commit now.

Previously floating replaced elements were drawn incorrectly and also
twice.
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.

3 participants