w3c / largest-contentful-paint

Specification for the LargestContentfulPaint API
https://w3c.github.io/largest-contentful-paint/
Other
88 stars 17 forks source link

Expose LCP renderTime in non-TAO cross-origin images, when it doesn't reflect the image decoding time #91

Open yoavweiss opened 2 years ago

yoavweiss commented 2 years ago

Currently Chromium has unspecified heuristics that ignore LCP candidates when the document's opacity is 0, and only expose those candidates when the opacity becomes non zero.

Once we've specified those, it may make sense to expose the renderTime for such LCP candidates, even without a TAO opt-in. Not doing that can result in situations where LCP's startTime is lower than FCP.

The rational for that is that the renderTime in such cases doesn't reveal anything about the image's decoding speed. One caveat to that is that it can reveal that the image finished decoding before the opacity change, so may make sense to include some fixed timeout where renderTime will be expoed only if the opacity change happened X milliseconds after the image's load time.

Thoughts?

mmocny commented 2 years ago

Generally, makes sense to me.

My one concern is: is anyone is using the existence of renderTime property as some form of feature detection? I've done this before... but, only to detect when I should use a fallback LCP approach. So this proposal would have been preferred there.

I think the new semantics would be

Are there other cases where renderTime is decoupled from decodeTime? Could this be applied more broadly?

yoavweiss commented 2 years ago

is anyone is using the existence of renderTime property as some form of feature detection?

Can you expand on that?

mmocny commented 2 years ago

Can you expand on that?

We discussed this issue at web perf WG meeting. We had a few interesting points raised which we should summarize here, and I think it's worth summarizing the notes here. (But I am not doing so right now, sorry).


I just wanted to add one more case which is similar: prerendering, where paint timings are also adjusted due to explicit visibility hiding.

It may be nice if we had a solution where we can make it more obvious that the renderTime was due to some arbitrary gap of time where the element was done load and decode, but wasn't even attempting to present since it was hidden.


And, with that in mind, an idea came to mind. First, some background:

So, could we solve multiple problems at once by adding consistency to some of these time points?

With all this, you could probably already expose renderingStart (and maybe even renderingEnd) even for cases of cross-origin images without TAO. That would be a more accurate timing that is unrelated to the image, and would capture these arbitrary time gaps.

You perhaps could possibly also expose a presentationTime, if the image was already decoded prior to renderingStart (e.g. the loaded but explicitly hidden -> shown cases).

You can then explicitly compare the loadTime vs renderingStart times as a clue to know if the LCP was artificially late (e.g. due to content hiding, or due to unrelated script delays). This would be very useful for LCP attribution even outside of this issue here.

Note: this proposes a literal swap of how startTime and duration are currently used for paint timing, which may make this specific suggestion untenable. It also basically suggests deprecating renderTime value since its not used very consistently. We may want something more reasonable :)

mmocny commented 2 years ago

83 seems related here as well (where we suggest exposing the first frame of an animated image as a unique time point distinct from loadTme and final renderTime/presentationTime)

nicjansma commented 2 years ago

May 12 W3C WebPerf Meeting minutes and summary:

@mmocny am I missing anything else that you recall?

mmocny commented 2 years ago

I think the one large-ish question that was raised was: what happens is there is still some rendering time left that is still specific to the image? One example given was: what if the image finishes loading, but doesn't start decoding? Thats a case where there is a large "idle" time gap, but exposing renderTime can still leak information about the image presumably?

Another, more minor point: the proposal is that whenever loadTime and renderTime are separated by some idle time (perhaps +gap), its safe to expose because idle time dominates... but thats really only easy to detect when the image is attached to DOM and hidden with CSS. But what happens when you use JS, and something like a vdom, to prefetch the image but then choose not attach it to DOM until some time later... Its hard to know if thats some sort of long-task related delay, but the image is still racing... vs induced idle time. It's possible we can report for this case... but then maybe that opens the door for even more reporting?

yoavweiss commented 2 years ago

If image decoding work doesn't start before we add the image to the DOM, then we cannot expose render time in those cases. If it's done independently, then we can. I'm not sure which of the above is the current behavior in Chromium and other engines. I also believe those parts of implementations are not specified, and hence I'd be reluctant to web expose them here, unless we also want to specify them.

yoavweiss commented 2 years ago

In discussions elsewhere, @mmocny suggested that we can potentially expose the renderTime of non-TAO LCP images when it equals the FCP time, which would tackle the opacity change case, foregrounding case and maybe others. It won't cover all cases in which we can expose the render time without revealing details about the image, but it'd be significantly simpler to do so. We could expand on that in the future if needed.

yoavweiss commented 1 year ago

Assuming there's agreement on the above direction, I think the next step would be to whip up a PR, and tests to go along with it.

mmocny commented 3 months ago

This issue hasn't been touched for a while, so here's a quick update:


Chromium has added a feature flag called ExposeRenderTimeNonTaoDelayedImage.

Despite that name, it doesn't actually expose renderTime. Instead, it adds the FCP time as a final fallback for LCP. This behaviour is somewhat equivalent to a developer using const lcp_time = Math.max(FCP.startTime, LCP.startTime) in their own rum reporting.

Besides being ergonomically useful, it also addresses issues that come up in developer feedback such as LCP time being 0, or even "appearing to be negative", whenever you have an activation time that is non-0. See how web-vitals.js library expects lcp.startTime to be at least as large as page activation time.

I think we should update the LCP spec to clamp the LCP candidate startTime to at least the FCP.startTime.

Then, Chromium can consider shipping this feature.


As a distinct change, @noamr has been poking away at the paint timing interop issue: w3c/paint-timing/issues/62.

I am not certain, but I think that one opens a pathway by which even for non-TAO images which might still be able to report the animation frame's paint time, or raf time, or something that isn't falling back to loadTime.

If that work lands, it is possible that LCP.startTime will no longer be 0, but it might still be less than FCP.startTime (if FCP does measure presentation time and LCP does not).

It is not clear to me what should be the right behaviour in that use case, but it may be that this feature here entirely goes away. I suspect we will always want to use some sort of minimum possible paint time, and that FCP.startTime is a good default.


@yoavweiss @noamr @nicjansma WDYT?

mmocny commented 3 months ago

One risk with the above: today FCP.startTime is always either renderTime or loadTime and now we are adding a third option.

If any scripts specifically test for renderTime == 0 or startTime == renderTime, and then assume certain things based on that test... it could be possible that adding a new case (LCP.startTime as FCP.startTime) might break some expectations. Theoretically.

But, I tried to search github for any snippets that might do something like this, and I have not successfully done so. The very small number of cases where snippets actually read renderTime is usually just for diagnostics, it seems.

mmocny commented 2 weeks ago

Update: I believe @noamr is working on this.

I think that: