w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 661 forks source link

[css-view-transitions-1] Should isolation and plus-lighter blending be applied conditionally #7814

Closed khushalsagar closed 1 year ago

khushalsagar commented 2 years ago

The default animation in shared element transitions can have either of the following trees per tag:

Cross-fade

::page-transition-container
|_::page-transition-image-wrapper
    |_::page-transition-incoming-image
    |_::page-transition-outgoing-image

There is visual state for an element in both outgoing and incoming DOM. The 2 have a cross-fade and need isolation: isolate on the wrapper and mix-blend-mode: plus-lighter on both the images to the blending right.

Enter animation

::page-transition-container
|_::page-transition-image-wrapper
    |_::page-transition-incoming-image

There is visual state only for an element in the incoming page which fades in.

Exit animation

::page-transition-container
|_::page-transition-image-wrapper
    |_::page-transition-outgoing-image

There is visual state only for an element in the outgoing page which fades out.

It's unclear how best to handle isolation and mix-blend-mode for the 3 cases:

  1. Technically we don't need the isolation and mix-blend-mode properties in enter/exit animations. These were also causing sub-optimal rendering (in Blink) because the fact that they are no-op isn't always optimized out. So we could omit adding them in these 2 modes.
  2. Always add them to these elements to ensure developers see a consistent style. Optimize out their effect in the implementation.

Option 1 is always better perf wise though it can be optimized if needed. I'd err for that unless there is a good reason to have option 2.

khushalsagar commented 2 years ago

Always having isolation/mix-blend-mode can be a footgun since any content generated by these 2 nodes (like the developer adding background to the wrapper) will force us off the fast path.

jakearchibald commented 1 year ago

Summary for CSSWG:

We use mix-blend-mode: plus-lighter on the old & new views, plus isolation: isolate on the image-pair, in order to create a true cross-fade.

Isolation isn't free performance-wise.

Some transition groups only have an old view, or only new view. This happens when an element with a particular view-transition-name only exists on one side of the transition. Isolation is unnecessary in these cases.

Option 1: Apply isolation & blend mode styles only when both old & new views are present

This would be observable in getComputedStyle.

It would also result in a different rendering if the developer gives the image-pair a background color, since some contents will plus-lighter blend with that background, and others wouldn't. However, if you set a background on the image-pair, it like that you'd want to set blending to normal anyway.

We already apply lots of styles dynamically in a view transition, such as width, height, transform etc of transition groups.

Option 2: Optimise internally for cases where isolation and blending is unnecessary

getComputedStyle would still see isolation and plus-lighter, but the rendering engine would avoid rendering to another buffer in cases where doing so would have no visual effect.

It's unclear how tricky this would be.

khushalsagar commented 1 year ago

Discussed this offline again, the summary above is great. Adding what came up at the last discussion.

Option 2 will need to be done anyway for cases like this. In this case the author is adding view-transition-name for elements in both the old and new DOM, since they want a size/transform animation. But they're setting opacity: 0 on the old image because the content hasn't changed. In fact they want the live animating new image instead of a cross-fade. In this case the engine will set up all the isolation and mix-blend-mode styles for cross-fade anyway and would have to detect that only image is actually being rendered so this blending can be optimized.

The worry is still that while the UA can optimize the default entry/exit transitions, authors might add styles (like background) which will require the expensive blending without realizing it. Since it comes from rules in the UA CSS. On the flip side, if normal blending is incorrect for them, they would discover that and add the requisite blend-mode/isolation (same as the regular DOM rendering).

So the proposal is to go with option 1. This is also in the spec already here. Adding this for async resolution.

astearns commented 1 year ago

I am reluctant to resolve on this without some agreement from others who have more expertise in animations and transitions than I have. Could you solicit opinions from outside your team before we try for an async resolution?

khushalsagar commented 1 year ago

@flackr was present in the discussion I summarized above for animations/transitions expertise. @rcabanier for compositing/blending expertise since that's what we're trying to decide here: what should be the default isolation/blending on the pseudo-DOM generated by the UA if we don't need a cross-fade as described here.

css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [css-view-transitions-1] Should isolation and plus-lighter blending be applied conditionally, and agreed to the following:

The full IRC log of that discussion <TabAtkins> khush: There's some styles in the UA CSS that are applied when you have both old and new image
<TabAtkins> khush: One is isolation on the image pair, th eother is blend mode on the images, plus-lighter
<TabAtkins> khush: This means if the two pixels you're blending are same value it's a no-op
<TabAtkins> khush: You only really need the isolation to get this two-image crossfade tho
<TabAtkins> khush: If you have only an incoming or outgoing, just fading in/out, you don't need this complex blending
<TabAtkins> khush: As an engine you can detect this and optimize
<TabAtkins> khush: We weren't sure - authors might add a background to things and force us to the offscreen-blending like with two images, authors might not realize they're falling off to a slow path
<TabAtkins> khush: So proposal is only apply these styles when both images are present. When only one is, don't apply isolation or blending
<TabAtkins> khush: Just like in a custom-authored webpage, you'd only add what you need. If authors do need some complex blending they can add it themselves.
<fantasai> This seems fine...
<emeyer> TabAtkins: My only potential concern is isolation has containing-block effects
<emeyer> …But I’m not certain if that’s an important effect or not
<TabAtkins> khush: the image pair element already has abspos, it'll have the same CB effect
<emeyer> TabAtkins: In most cases, you aren’t positioning the images themselves
<emeyer> …If there’s an iossue, it’s in a funky corner case I’m not concerned with
<TabAtkins> khush: The size flows thru the group, the image pair uses the group as its CB so it sees that size. That's why we added abspos to make sure the CB hierarchy happens regardless of isolation
<emeyer> TabAtkins: There’s a few things that will skip abspos but won’t skip isolation
<emeyer> …so I think that’s fine
<TabAtkins> astearns: So are we reoslving to just accept the current spec?
<TabAtkins> khush: Yes.
<TabAtkins> astearns: Objections to accepting Khushal's edits?
<TabAtkins> RESOLVED: isolation/blending-mode properties are added conditionally, as stated in current spec
khushalsagar commented 1 year ago

Resolution aligns with current spec, no work needed.