Created attachment 399143 [details] Test Transform origin affects perspective, and should not.
<rdar://problem/63143806>
This also affects perspective on scrollers with visible scrollbars.
Created attachment 456620 [details] Patch, v1
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.
- '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).
Created attachment 456755 [details] Patch, v2
Created attachment 456980 [details] Patch, v3
Now that both 237554 and 237701 are in, it's tempting to see what EWS thinks about the patch :-) Let's try.
Created attachment 457041 [details] Patch, v4
Created attachment 457214 [details] Patch, v5
Created attachment 457215 [details] Patch, v6
Created attachment 457251 [details] Patch, v7
Simon, did you have any chance to have a look at it?
Yikes, it's pending review since almost a month.... all experts busy? :-)
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?
(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.
Pull request: https://github.com/WebKit/WebKit/pull/720
Committed r294615 (250841@main): <https://commits.webkit.org/250841@main> Reviewed commits have been landed. Closing PR #720 and removing active labels.