Bug 211787 – Perspective should not be affected by transform-origin

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

Created attachment 399143 [details]
Test

Transform origin affects perspective, and should not.

[Radar WebKit Bug Importer]

<rdar://problem/63143806>

[Simon Fraser (smfr)]

This also affects perspective on scrollers with visible scrollbars.

[Nikolas Zimmermann]

Created attachment 456620 [details]
Patch, v1

[Nikolas Zimmermann]

For LBSE I need proper transform-box/origin support for both composited & non-composited element. Therefore I gave this a shot: looks promising, perspective is no longer dependent on the transform-origin/transform-box.

This depends on the earlier cleanup patches from bugs 237554, 237701. I can mark this for review, once they were landed.

[Nikolas Zimmermann]

- 'perspective-origin' <length> values are now resolved against the 'transform reference box' (== the reference box according to 'transform-box', e.g. content-box / view-box / border-box / ...). Until now we've always used 'border-box'.

- Teach RenderLayerBacking about 'transform-box': composited elements ignored 'transform-box' so far, when computing the anchor points that encode 'transform-origin' information as relative values.

- Update the GraphicsLayer geometry upon 'transform-box' changes (fixes 
'transform-box' animations, covered by a new WPT test).

- The computed perspective transformation, that is applied to children of an element that defines a CSS 'perspective', is now invariant under 'transform-origin' / 'transform-box' changes. This now matches Blink/Gecko behaviour.

Blink/Gecko currently do not support e.g. 'content-box' as value for 'transform-box' -- other than that our compositing + perspective/scrolling/transform-box testcases now match between Safari/Firefox/Chromium, however only WebKit provides proper 'content-box' support.

This aligns our support for composited / non-composited elements (assigning 'will-change: transform' to elements that specify a non-default 'transform-box' no longer leads to surprises as in 'why does the element jump from its nominal position when compositing is active')

...


I think we should re-sync /css/css-transforms WPT tests first, to assess the current WPT CSS transforms state and how this patch affects WPT results -- I have had issues with a fresh WPT sync (hundreds of failures without this patch applied, so that needs to be checked first, I did not investigate yet).

[Nikolas Zimmermann]

Created attachment 456755 [details]
Patch, v2

[Nikolas Zimmermann]

Created attachment 456980 [details]
Patch, v3

[Nikolas Zimmermann]

Now that both 237554 and 237701 are in, it's tempting to see what EWS thinks about the patch :-) Let's try.

[Nikolas Zimmermann]

Created attachment 457041 [details]
Patch, v4

[Nikolas Zimmermann]

Created attachment 457214 [details]
Patch, v5

[Nikolas Zimmermann]

Created attachment 457215 [details]
Patch, v6

[Nikolas Zimmermann]

Created attachment 457251 [details]
Patch, v7

[Nikolas Zimmermann]

Simon, did you have any chance to have a look at it?

[Nikolas Zimmermann]

Yikes, it's pending review since almost a month.... all experts busy? :-)

[Simon Fraser (smfr)]

Comment on attachment 457251 [details]
Patch, v7

View in context: https://bugs.webkit.org/attachment.cgi?id=457251&action=review

> Source/WebCore/rendering/RenderLayer.cpp:1815
> +    // To circumvent this, explicitely remove the transform-origin dependency in the perspective matrix.
> +    auto transformOrigin = transformOriginForPainting();

If this function isn't returning the "perspectiveTransform", but rather something that makes assumptions about platform-specific rendering, perhaps it needs to be renamed?

> Source/WebCore/rendering/RenderLayer.cpp:1824
> +FloatPoint3D RenderLayer::transformOriginForPainting() const

Unclear how this relates to the other kind of "transform origin". Is it just about snapping?

> Source/WebCore/rendering/RenderLayer.h:743
>      // Note that this transform has the perspective-origin baked in.

Is this comment still accurate?

[Nikolas Zimmermann]

(In reply to Simon Fraser (smfr) from comment #15)
> Comment on attachment 457251 [details]
> Patch, v7
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457251&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:1815
> > +    // To circumvent this, explicitely remove the transform-origin dependency in the perspective matrix.
> > +    auto transformOrigin = transformOriginForPainting();
> 
> If this function isn't returning the "perspectiveTransform", but rather
> something that makes assumptions about platform-specific rendering, perhaps
> it needs to be renamed?
It's still the "vanilla" perspective transform matrix, plus an additional component that embeds it into the parent coordinate system, and that is indeed platform-dependent, as you pointed out -- however consistent for all our platforms at present.

How about re-using the GraphicsLayer terminology and call it "childrenTransform()"? 
I'll try that.

> 
> > Source/WebCore/rendering/RenderLayer.cpp:1824
> > +FloatPoint3D RenderLayer::transformOriginForPainting() const
> 
> Unclear how this relates to the other kind of "transform origin". Is it just
> about snapping?
Yeah, it snaps, if necessary.
How about "transformOriginPixelSnappedIfNeeded"? I'll go for that for now.

> 
> > Source/WebCore/rendering/RenderLayer.h:743
> >      // Note that this transform has the perspective-origin baked in.
> 
> Is this comment still accurate?
Yeah, we removed the transform-origin dependency, but perspective-origin is still included (and that's ok).

I will upload the new iteration of this patch as GitHub PR.

[Nikolas Zimmermann]

Pull request: https://github.com/WebKit/WebKit/pull/720

[EWS]

Committed r294615 (250841@main): <https://commits.webkit.org/250841@main>

Reviewed commits have been landed. Closing PR #720 and removing active labels.