Skip to content

Add support for LargestContentfulPaint PaintMetric #39384

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.

Sign up for GitHub

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

Jump to bottom
Merged
yezhizhen merged 8 commits into from
Oct 31, 2025
Merged

Add support for LargestContentfulPaint PaintMetric #39384

yezhizhen merged 8 commits into from
Oct 31, 2025

Conversation

@shubhamg13
Copy link
Contributor

@shubhamg13 shubhamg13 commented Sep 19, 2025 •
edited
Loading

Steps for calculating LCP:

  1. When the pref for largest_contentful_paint_enabled is enabled, LargestContentfulPaintCandidateCollector is created once per LayoutThread using viewport_size.
  2. While building the display_list, it send the pre-calculated clip_rect and bounds to LCPCollector along with transform and LCPCandidateType for visual_rect calculation for LCPCandidates.
  3. After visual_rect calculation, update the lcp_candidate based on area of visual_rect and send it to compositor.
  4. Then among lcp_candidates, we find out the largest_contentful_paint and send it to paint_metric
  5. In addition to above LCP also uses PaintMetricState to save console from spamming with duplicate entries.

This PR includes following commits better segregation of code

  1. Add LargestContentfulPaintCandidate struct
  2. Add LargestContentfulPaintCandidateCollector
  3. Collect the LargestContentfulPaintCandidateswhen feature is enabled.
  4. Send the LargestContentfulPaintCandidates to compositor
  5. Add LargestContentfulPaintCalculator
  6. Calculates the LargestContentfulPaint
  7. Send LCP to PaintMetrics
  8. Disable LCP feature on user interaction

Elements considered for LCP in this PR:

  • Image element
  • An element with background image

Reference: #38041
Testing: Tested using perf-analysis-tools See attached screenshot.
Fixes: None

Perfetto
Screenshot from 2025-10-16 11-37-41

Console Output of Metric
Screenshot from 2025-10-16 11-33-16

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 19, 2025
@servo-highfive
Copy link

servo-highfive commented Sep 19, 2025

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!

@shubhamg13 shubhamg13 marked this pull request as draft September 19, 2025 06:12
@mrobinson
Copy link
Member

mrobinson commented Sep 19, 2025

How does this change relate to #38041?

@shubhamg13 shubhamg13 force-pushed the lcp branch 2 times, most recently from f3d2f5a to 3e77e77 Compare September 19, 2025 09:16
@shubhamg13
Copy link
Contributor Author

shubhamg13 commented Sep 19, 2025 •
edited
Loading

How does this change relate to #38041?

I'm picking up the PR from where it was left.
I'll cater the unresolved comments and upcoming comments here.

P.S: I can't modify existing PR, so created separate one.

cc: @xiaochengh

@shubhamg13 shubhamg13 changed the title Add LargestContentfultPaint. Add LargestContentfulPaint Sep 19, 2025
@shubhamg13 shubhamg13 force-pushed the lcp branch 2 times, most recently from b9a6e01 to 5bfa356 Compare September 19, 2025 10:17
@mrobinson mrobinson mentioned this pull request Sep 19, 2025
Closed
@shubhamg13 shubhamg13 force-pushed the lcp branch 9 times, most recently from 97278bc to 2679291 Compare September 22, 2025 10:10
@shubhamg13 shubhamg13 marked this pull request as ready for review September 22, 2025 10:10
xiaochengh
xiaochengh reviewed Sep 25, 2025
View reviewed changes
Copy link
Contributor

@xiaochengh xiaochengh left a comment

Choose a reason for hiding this comment

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

Thanks for completing this PR!

I'm still wondering if it's the best choice to tightly couple LCP calculation with display list computation in this way. @mrobinson @Loirooriol could you give your insights?

Otherwise this PR is basically good to me.

@shubhamg13 shubhamg13 force-pushed the lcp branch 2 times, most recently from 7c2832a to 9848b9c Compare September 26, 2025 01:44
@shubhamg13
Copy link
Contributor Author

shubhamg13 commented Oct 31, 2025

I haven't had time to review this in depth, but please fix these nits before landing:

Done with nits.

@shubhamg13 shubhamg13 requested a review from mrobinson October 31, 2025 06:17
@servo-highfive servo-highfive removed the S-tests-failed The changes caused existing tests to fail. label Oct 31, 2025
@yezhizhen yezhizhen added this pull request to the merge queue Oct 31, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 31, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 31, 2025
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 31, 2025
@shubhamg13
Send LCP candidtes to compositor for processing
d161ece 
Signed-off-by: Shubham Gupta <shubham.gupta@chromium.org>
@shubhamg13
Add Largest Contentful Paint Calculator
8b3465c 
Signed-off-by: Shubham Gupta <shubham.gupta@chromium.org>
@shubhamg13
Process LCP and add clean up protocols
b4f113c 
Signed-off-by: Shubham Gupta <shubham.gupta@chromium.org>
@shubhamg13
Send LCP to PaintMetrics
2737521 
Signed-off-by: Shubham Gupta <shubham.gupta@chromium.org>
@shubhamg13
Disable LCP feature on user interaction
84df9fd 
Signed-off-by: Shubham Gupta <shubham.gupta@chromium.org>
@servo-highfive servo-highfive removed the S-tests-failed The changes caused existing tests to fail. label Oct 31, 2025
@yezhizhen yezhizhen added this pull request to the merge queue Oct 31, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 31, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 31, 2025
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 31, 2025
@yezhizhen yezhizhen added this pull request to the merge queue Oct 31, 2025
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 31, 2025
Merged via the queue into servo:main with commit 9eb3a97 Oct 31, 2025
38 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 31, 2025
@shubhamg13 shubhamg13 deleted the lcp branch November 3, 2025 01:52
@shubhamg13 shubhamg13 mentioned this pull request Nov 6, 2025
Merged
@shubhamg13
Copy link
Contributor Author

shubhamg13 commented Nov 6, 2025

Btw, is this PR able to emit LCP performance entries? If so there should be WPT expectation changes.

That's pending, I will implement idl interface in follow-up.

Add basic IDL implementation for LargestContentfulPaint #39714

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

Reviewers

@yezhizhen yezhizhen yezhizhen approved these changes

@Loirooriol Loirooriol Awaiting requested review from Loirooriol Loirooriol is a code owner

@nicoburns nicoburns Awaiting requested review from nicoburns

@gterzian gterzian Awaiting requested review from gterzian gterzian is a code owner

@xiaochengh xiaochengh Awaiting requested review from xiaochengh

@mrobinson mrobinson Awaiting requested review from mrobinson mrobinson is a code owner

Assignees

No one assigned

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@shubhamg13 @servo-highfive @mrobinson @xiaochengh @yezhizhen