[css-view-transitions-1] Display live image when element is outside the snapshot containing block in old state #10587
Conversation
…de snapshot containing block See w3c#8282 (comment)
|
I'll defer this one to @mattwoodrow |
css-view-transitions-1/Overview.bs
Outdated
| @@ -1172,7 +1172,7 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface; | |||
|
|
|||
| <dl dfn-for="captured element"> | |||
| : <dfn>old image</dfn> | |||
| :: an 2D bitmap or null. Initially null. | |||
| :: a 2D bitmap, "`live`" or null. Initially null. | |||
There was a problem hiding this comment.
Since old image is supposed to be a 2D bitmap data type, making it also "live" is odd. Can we track this state using a separate bool?
There was a problem hiding this comment.
What would old image hold in the live case then? I actually think this is a good way to represent it and it's common in other standards (e.g. fetch callbacks pass failure, a network error, or a response).
There was a problem hiding this comment.
What would old image hold in the live case then?
The old image can stay null, since we didn't capture anything. And a bool on captured element like, "should display new live image for old pseudo".
I find the pattern of strongly typed data types in the spec makes it more readable, could be my C++ brain. 😅 So would prefer it unless you feel strongly otherwise.
There was a problem hiding this comment.
No worries. Actually I need to do this anyway because once we update from the actual live snapshot that live enum is replaced with an image anyway.
There was a problem hiding this comment.
Hmm actually trying to refactor this makes it so that I have to check for this boolean in multiple places (where we check for old image is null). I think the variant approach is a lot cleaner...
There was a problem hiding this comment.
Maybe the bool can be "should generate old pseudo"? That's what old image != null implies which we want to change right. Then we'd only use old image when we need the pixels.
css-view-transitions-1/Overview.bs
Outdated
| 1. Let |originalRect| be [=snapshot containing block=] if |element| is the [=document element=], | ||
| otherwise, the element|'s [=border box=]. | ||
|
|
||
| 1. If |originalRect| does not intersect with [=snapshot containing block=], then set |capture|'s [=old image=] to "`live`". |
There was a problem hiding this comment.
We need to use the ink overflow rect here, instead of border box.
There was a problem hiding this comment.
Was this in the resolution?
There was a problem hiding this comment.
We didn't explicitly state that in the resolution. It's in the notes here:
<TabAtkins> khush: first, how to define offscreen
<TabAtkins> khush: related to the reference rect discussion, we want ink overflow, and i think that's sufficinetly specced now
The last comment was from the discussion here where the conclusion was that ink overflow is sufficiently spec'd to be used by VT or IntersectionObserver.
css-view-transitions-1/Overview.bs
Outdated
|
|
||
| 1. Let |originalRect| be [=snapshot containing block=] if |element| is the [=document element=], | ||
| otherwise, the element|'s [=border box=]. | ||
|
|
||
| 1. Let |boundingBox| be |element|'s [=Element/get the bounding box|bounding box=], with [=ink overflow=] taken into account. |
There was a problem hiding this comment.
nit: The spec uses "element’s ink overflow rectangle" here: https://drafts.csswg.org/css-view-transitions-1/#capture-the-image-algorithm:~:text=Return%20the%20portion%20of%20this%20canvas%20that%20includes%20element%E2%80%99s%20ink%20overflow%20rectangle. Use the same here for consistency?
|
Matt got back to me. Here are my thoughts: Regarding the resolution itself, I think it means we have to display the live image in two places. I'm not quite sure how that'd be implemented, since you'd have to clone layers and keep them in sync. It doesn't quite sound like an optimization to me unless I'm missing something. Could you clarify how this would be an optimization? |
In Chromium we have a handle to the live texture which can be bound to multiple layers. So we can have the layer associated with the old pseudo use the live texture handle instead of a cached texture handle which currently happens.
The desired optimization is to not force rendering and caching of old named elements which are offscreen. Currently as per spec, we have to do this even if the browser's normal rendering flow optimizes out paint/raster/compositing for offscreen content. And in order to do this in the most web compat way, we have the old pseudo use the new live image. The browser can use the knowledge that the old/new pseudos are displaying live images to optimize away the default cross-fade. But if the author has some customization (like old slides to the left as new slides in from right), at least having some content in the old pseudo will not look completely broken. You can read the detailed notes for how we concluded to this resolution here: #8282 (comment) and here: #8282 (comment). Happy to hear any other suggestions! |
I would add that in a large portion of the cases those offscreen elements would remain offscreen also in the new element, so this would optimize them out completely. |
|
WebKit doesn't use textures for View Transitions, so this optimization may just not apply to us. I'd prefer leaving open-ended what the UA may display in place of the old capture. |
How do you preserve the content of the old element (to show in |
It's an ImageBuffer of the old content: The new live snapshot is just the element's graphics layers reparented to the pseudo-element (except for |
|
IIUC ImageBuffer implies you're allocating memory and painting the contents of the old element. This is the cost we want to optimize. I'm assuming if elements are far away Webkit doesn't paint them, but ViewTransition is forcing them to be painted? |
|
I'm not against optimizing, but I think this particular optimization makes assumptions about the implementation. It assumes you can straightforwardly render the live snapshot in two places, which I don't think is the case in WebKit. Maybe @mattwoodrow has other ideas for optimizations of offscreen content? |
|
The main optimization we're after is fundamental for all engines. The browser has to cache some content representation of the old elements and it's very hard to make that cheap. So we want to avoid the spec forcing engines to spend resources for rendering offscreen content. This is not an issue for new images, they are live. The engine can paint them when/if the content comes onscreen. But this is not an option for old elements.
The other option is to not render anything for the old image pseudo, what this PR is doing in the exit animation case. I wouldn't be opposed to trying that in all cases. We just went with reusing the live snapshot for old since it seemed less risky compat wise and we didn't have implementor feedback saying it's hard to do. :) |
Another option is to fade-in from a no-content pseudo element rather than not having an old pseudo at all. |
I think we mean the same thing, just want to confirm. It's this PR except we don't draw the live image into the old pseudo. |
Yes exactly. |
|
@nt1m will you be OK with this PR if the old pseudo had null content in this situation? Basically remove this line: If that seems reasonable we can try to get an amended WG resolution. |
Yes please, that would fit into our architecture much better :) |
|
We've changed the resolution about this, closing. |
See resolution.
Showing the "live" snapshot in the "old" element if the element was offscreen (outside the snapshot containing block) in the old state.
Putting this in css-view-transitions-1 rather than 2 because it changes behavior in a way that's not author-observable. Hope that's OK and happy to consider alternatives (/cc @nt1m)