layout: Fix `first-paint` detection by shubhamg13 · Pull Request #42975 · servo/servo

shubhamg13 · 2026-03-03T03:59:41Z

This PR comprises

Segregate paintable and contentful.
Adds Checking of non-default background for eligibility to mark first-paint.
Adds a bool is_paintable to display_list and use same for marking first-paint.

TODO: This doesn't consider iframes. Will add PR after fixing WPT tests.

Raised an issue for some incomplete definitions: w3c/paint-timing#122

Testing:

wpt/tests/paint-timing/first-paint-only/first-paint-bg-color.html
Updated WPT tests expectations.
- wpt/paint-timing/first-image-child.html
- wpt/paint-timing/first-paint-only/sibling-painting-first-image.html
Unit Test: servo/tests/performance_paint_timings.rs. Following Tests Cases are covered:

Background Pref	Root Background	FP	TC
Default	Unset	❌	`test_default_pref_with_unset_background`
Non-Default	Unset	❌	`test_non_default_pref_with_unset_background`
Default	Transparent	❌	`test_default_pref_with_transparent_background`
Non-Default	Unset	❌	`test_non_default_pref_with_transparent_background`
Default	Set	✅	`test_default_pref_with_background`
Non-Default	Set	✅	`test_non_default_pref_with_background`
Non-Default	Set (same as pref)	❌	`test_non_default_pref_with_same_background`

Fixes: #42148

mrobinson · 2026-03-03T08:37:59Z

components/layout/display_list/stacking_context.rs

+
+            // From <https://www.w3.org/TR/paint-timing/#sec-terminology>:
+            // First paint ... includes non-default background paint and the enclosing box of an iframe.
+            if background_color != AbsoluteColor::WHITE {


I do not understand this code. White is not the default background color in style. You should be checking the value from the style itself, not the final color. In addition, the specification text says "First paint excludes the default background paint, but includes non-default background paint." This isn't talking about the color, but the background itself, so my reading is that it should include all kinds of background paints, including images.

IMO it is about color. We should mark default color in prefs or somewhere.

If I don't check for white presumably default, It will result in failing of first-contentful-paint.html

Can you justify this interpretation of the specification somehow? A "default background color" of <div>s does not sound like the kind of feature that the web platform usually contains. Also does "default background paint" here refer to the background of individual fragments or the page canvas background of the HTML <body> element (https://drafts.csswg.org/css-backgrounds/#body-background)?

I understand now that this code is only about the page canvas background.

If you observe the behavior of this code in Chrome you can see that background images should trigger first-paint. I am pretty sure that "non-default" here should mean when any background that eventually propagates to be the HTML page canvas is not transparent and does not match the backdrop color (pref!(shell_background_color_rgba)) including images.

<!doctype html> <html> <head> <style> html { background-image: url(data:img/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQIW2Ng+P//PwAF/wL+wMZ2tgAAAABJRU5ErkJggg==); } </style> </head> <body> <script> setTimeout(() => { console.log(performance.getEntriesByType('paint')); }, 2000); </script> </body> </html>

Also observe that this works the same way with a 1 pixel by 1 pixel transparent or white image

<!doctype html> <html> <head> <style> html { /* background-image: url(data:img/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQIW2Ng+P//PwAF/wL+wMZ2tgAAAABJRU5ErkJggg==);*/ background-image: url(data:img/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVQIW2P4DwQACfsD/Z8fLAAAAAAASUVORK5CYII=); } </style> </head> <body> <script> setTimeout(() => { console.log(performance.getEntriesByType('paint')); }, 2000); </script> </body> </html>

All the other background like background-image, border-image are already considered for FP.

Only non-default background color is not considered. So, i marked it here.

Still, you will need to consider the backdrop color from the preferences. Also, it's unclear what should happen here when the page is loading in the iframe. I suspect the behavior might end up being different depending on where the content is loading (main frame versus iframe).

Still, you will need to consider the backdrop color from the preferences. Also, it's unclear what should happen here when the page is loading in the iframe. I suspect the behavior might end up being different depending on where the content is loading (main frame versus iframe).

Considered the backdrop color from preference. And consolidated all the changes here.

Yeah, for iframe, it is not considered here. Since, I'm fixing the WPT tests, I will submit it in a separate PR when WPT tests are updated after weekly sync. Added a TODO in this PR description for same.

Please review

The latest commit (4a6f9c4) looks correct to me in handling non-default background color of the root.

The spec is vague on this. We should provide some test cases to ensure that our behavior is correct in at least common cases.

mrobinson

This doesn't look to me like what the specification is saying and I think we need to work out what is actually meant here by "non-default background paint" before landing.

mrobinson · 2026-03-03T12:04:42Z

components/layout/display_list/stacking_context.rs

+
+            // From <https://www.w3.org/TR/paint-timing/#sec-terminology>:
+            // First paint ... includes non-default background paint and the enclosing box of an iframe.
+            if background_color != AbsoluteColor::WHITE {


If you observe the behavior of this code in Chrome you can see that background images should trigger first-paint. I am pretty sure that "non-default" here should mean when any background that eventually propagates to be the HTML page canvas is not transparent and does not match the backdrop color (pref!(shell_background_color_rgba)) including images.

<!doctype html> <html> <head> <style> html { background-image: url(data:img/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQIW2Ng+P//PwAF/wL+wMZ2tgAAAABJRU5ErkJggg==); } </style> </head> <body> <script> setTimeout(() => { console.log(performance.getEntriesByType('paint')); }, 2000); </script> </body> </html>

Also observe that this works the same way with a 1 pixel by 1 pixel transparent or white image

<!doctype html> <html> <head> <style> html { /* background-image: url(data:img/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQIW2Ng+P//PwAF/wL+wMZ2tgAAAABJRU5ErkJggg==);*/ background-image: url(data:img/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVQIW2P4DwQACfsD/Z8fLAAAAAAASUVORK5CYII=); } </style> </head> <body> <script> setTimeout(() => { console.log(performance.getEntriesByType('paint')); }, 2000); </script> </body> </html>

mrobinson · 2026-03-03T14:06:02Z

components/layout/display_list/mod.rs

        // An element el is paintable when all of the following apply:
+        // > el is being rendered.
+        // > el’s used visibility is visible.
+        // This is sufficient to mark the first paint


I know this is from the other PR, but I think these two should be combined into a single PR.

Why is this sufficient? Please provide specification text and a link so that those reading the code can understand why this is the case.

Updated please take a look and review.

mrobinson · 2026-03-03T14:07:58Z

components/layout/display_list/stacking_context.rs

+
+            // From <https://www.w3.org/TR/paint-timing/#sec-terminology>:
+            // First paint ... includes non-default background paint and the enclosing box of an iframe.
+            if background_color != AbsoluteColor::WHITE {


Still, you will need to consider the backdrop color from the preferences. Also, it's unclear what should happen here when the page is loading in the iframe. I suspect the behavior might end up being different depending on where the content is loading (main frame versus iframe).

shubhamg13 · 2026-03-06T06:54:42Z

This doesn't look to me like what the specification is saying and I think we need to work out what is actually meant here by "non-default background paint" before landing.

Yeah, As I understand anything other than default color is "non-default background paint". Whether it is border-image, background-image, background color or even gradient also.

xiaochengh · 2026-03-13T03:47:25Z

components/shared/paint/display_list.rs

+    /// First paint
+    /// See <https://w3c.github.io/paint-timing/#first-paint>.
+    pub first_paint: bool,


IIUC this is marking whether anything is painted. If so, it seems better to change it into:

Suggested change

/// First paint

/// See <https://w3c.github.io/paint-timing/#first-paint>.

pub first_paint: bool,

/// Whether anything has been painted. Used to mark first paint.

/// See <https://w3c.github.io/paint-timing/#first-paint>.

pub has_paint: bool,

All the other relevant variables and methods should also be renamed.

Sure, I will update.

xiaochengh · 2026-03-13T03:50:21Z

components/layout/display_list/mod.rs

Might be irrelevant to this PR but this doesn't match the definition of contentful. Something seems wrong here.

Do we really have a genuine implementation of FCP, or are we simply reporting FP as FCP?

Yeah, It looks like that.

But FCP is quite consistent.

https://www.w3.org/TR/paint-timing/#should-report-first-contentful-paint

If document contains at least one element that is both paintable and contentful, then return true.

We are checking paintable for sure, and most elements from contentful list as we only call this API if element is of that relevant type.

And FP needed some extra addition, which are being handled in this one.

But we can reach here for non-contentful paints, like the background paint of the root context, or an element with a non-image background.

Discussed offline:

PaintDisplayListInfo should maintain two independent booleans, is_paintable and is_contentful, following exactly the definition in spec

check_if_paintable should mark paintable only

When painting a fragment, callers should never bypass check_if_paintable as otherwise some necessary checks will be bypassed

When painting an image or text fragment, callers should mark contentful

xiaochengh · 2026-03-13T03:52:54Z

components/layout/display_list/mod.rs

+        // > > From <https://w3c.github.io/paint-timing/#first-paint>:
+        // > > First paint entry contains a DOMHighResTimeStamp reporting the time when the user agent first rendered after navigation.
+        // > > Hence, This is sufficient to mark the first paint.
+        self.mark_first_paint();


Per spec, first paint should be marked only after all of visibility, opacity and bounding rect intersection have been checked and passed.

Aaah right. Updating

xiaochengh · 2026-03-13T04:00:04Z

components/layout/display_list/stacking_context.rs

+
+            // From <https://www.w3.org/TR/paint-timing/#sec-terminology>:
+            // First paint ... includes non-default background paint and the enclosing box of an iframe.
+            if background_color != AbsoluteColor::WHITE {


The latest commit (4a6f9c4) looks correct to me in handling non-default background color of the root.

xiaochengh · 2026-03-13T05:05:32Z

components/layout/display_list/mod.rs

Discussed offline:

PaintDisplayListInfo should maintain two independent booleans, is_paintable and is_contentful, following exactly the definition in spec

check_if_paintable should mark paintable only

When painting a fragment, callers should never bypass check_if_paintable as otherwise some necessary checks will be bypassed

When painting an image or text fragment, callers should mark contentful

components/paint/painter.rs

servo-wpt-sync · 2026-03-17T08:31:21Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#58544).

jdm

This looks reasonable!

components/servo/tests/performance_paint_timing.rs

Signed-off-by: Shubham Gupta <[email protected]>

servo-wpt-sync · 2026-03-18T04:58:35Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#58544).

servo-wpt-sync · 2026-03-18T05:19:45Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#58544) title and body.

Comments have been addressed

`iframe` should be considered for `first-paint`. TODO of: #42975 Synced: web-platform-tests/wpt#58243 Refer for some history: #42498 Testing: - `wpt/paint-timing/first-paint-only/child-painting-first-image.html` - `wpt/paint-timing/first-paint-only/sibling-painting-first-image.html` Fixes: #42455 --------- Signed-off-by: Shubham Gupta <[email protected]>

shubhamg13 requested review from Loirooriol and mrobinson as code owners March 3, 2026 03:59

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 3, 2026

shubhamg13 requested a review from jdm March 3, 2026 04:00

shubhamg13 force-pushed the fix_fb_bg branch from 0de4cf1 to a287259 Compare March 3, 2026 04:27

shubhamg13 changed the title ~~layout: Consider non-default background color for first-paint only~~ layout: Consider non-default background color for first-paint Mar 3, 2026

shubhamg13 mentioned this pull request Mar 3, 2026

layout: Consider iframe for first-paint #42149

Merged

mrobinson requested changes Mar 3, 2026

View reviewed changes

servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 3, 2026

shubhamg13 requested a review from mrobinson March 3, 2026 09:29

mrobinson requested changes Mar 3, 2026

View reviewed changes

mrobinson previously requested changes Mar 3, 2026

View reviewed changes

shubhamg13 marked this pull request as draft March 4, 2026 03:09

shubhamg13 force-pushed the fix_fb_bg branch from a287259 to f4d02d6 Compare March 4, 2026 03:38

shubhamg13 changed the title ~~layout: Consider non-default background color for first-paint~~ layout: Fixfirst-paint detection Mar 4, 2026

shubhamg13 changed the title ~~layout: Fixfirst-paint detection~~ layout: Fix first-paint detection Mar 4, 2026

shubhamg13 requested a review from mrobinson March 4, 2026 03:58

shubhamg13 marked this pull request as ready for review March 4, 2026 03:58

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Mar 4, 2026

shubhamg13 requested a review from xiaochengh March 12, 2026 07:57

xiaochengh reviewed Mar 13, 2026

View reviewed changes

shubhamg13 force-pushed the fix_fb_bg branch from 4a6f9c4 to 5d3aad7 Compare March 13, 2026 04:38

xiaochengh previously requested changes Mar 13, 2026

View reviewed changes

servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 13, 2026

shubhamg13 force-pushed the fix_fb_bg branch from 5d3aad7 to caad2f2 Compare March 13, 2026 06:46

shubhamg13 marked this pull request as draft March 17, 2026 08:22

shubhamg13 force-pushed the fix_fb_bg branch from 30b1741 to cee7684 Compare March 17, 2026 08:27

shubhamg13 marked this pull request as ready for review March 17, 2026 08:34

shubhamg13 mentioned this pull request Mar 18, 2026

layout: Do not consider broken images as LCP candidates #42833

Merged

jdm approved these changes Mar 18, 2026

View reviewed changes

components/servo/tests/performance_paint_timing.rs Show resolved Hide resolved

components/servo/tests/performance_paint_timing.rs Outdated Show resolved Hide resolved

components/servo/tests/performance_paint_timing.rs Outdated Show resolved Hide resolved

servo-highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 18, 2026

shubhamg13 added 4 commits March 18, 2026 12:52

Segregate paintable and contentful

9f931d3

Signed-off-by: Shubham Gupta <[email protected]>

Update WPT tests expectations

6286c9d

Signed-off-by: Shubham Gupta <[email protected]>

Make FP background test more versatile

ba0d49e

Signed-off-by: Shubham Gupta <[email protected]>

Add unittest with different shell background colors.

d513731

Signed-off-by: Shubham Gupta <[email protected]>

shubhamg13 force-pushed the fix_fb_bg branch from cee7684 to d513731 Compare March 18, 2026 04:54

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-rebase There are merge conflict errors. labels Mar 18, 2026

shubhamg13 requested a review from jdm March 18, 2026 04:54

TimvdLippe added this pull request to the merge queue Mar 20, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 20, 2026

Merged via the queue into servo:main with commit a7d8f54 Mar 20, 2026
34 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 20, 2026

shubhamg13 deleted the fix_fb_bg branch March 20, 2026 07:57

yezhizhen mentioned this pull request Mar 20, 2026

Paint Timing unit tests are flaky #43484

Closed

shubhamg13 mentioned this pull request Mar 22, 2026

Feature: PaintTiming #42623

Open

Uh oh!

Conversation

shubhamg13 commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrobinson Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

servo-wpt-sync commented Mar 17, 2026

Uh oh!

jdm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

servo-wpt-sync commented Mar 18, 2026

Uh oh!

shubhamg13 commented Mar 3, 2026 •

edited

Loading

mrobinson Mar 3, 2026 •

edited

Loading

shubhamg13 Mar 4, 2026 •

edited

Loading

shubhamg13 Mar 4, 2026 •

edited

Loading

shubhamg13 commented Mar 6, 2026 •

edited

Loading

shubhamg13 Mar 13, 2026 •

edited

Loading

shubhamg13 Mar 13, 2026 •

edited

Loading