WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 211787
Perspective should not be affected by transform-origin
https://bugs.webkit.org/show_bug.cgi?id=211787
Summary
Perspective should not be affected by transform-origin
Simon Fraser (smfr)
Reported
2020-05-12 10:45:02 PDT
Created
attachment 399143
[details]
Test Transform origin affects perspective, and should not.
Attachments
Test
(2.51 KB, text/html)
2020-05-12 10:45 PDT
,
Simon Fraser (smfr)
no flags
Details
Patch, v1
(34.68 KB, patch)
2022-04-04 13:38 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch, v2
(33.21 KB, patch)
2022-04-05 15:00 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch, v3
(32.25 KB, patch)
2022-04-07 16:06 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch, v4
(32.23 KB, patch)
2022-04-08 03:56 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch, v5
(34.56 KB, patch)
2022-04-10 15:57 PDT
,
Nikolas Zimmermann
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch, v6
(35.42 KB, patch)
2022-04-10 16:04 PDT
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch, v7
(35.42 KB, patch)
2022-04-11 05:58 PDT
,
Nikolas Zimmermann
simon.fraser
: review+
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2020-05-12 10:45:21 PDT
<
rdar://problem/63143806
>
Simon Fraser (smfr)
Comment 2
2020-05-12 21:57:26 PDT
This also affects perspective on scrollers with visible scrollbars.
Nikolas Zimmermann
Comment 3
2022-04-04 13:38:19 PDT
Created
attachment 456620
[details]
Patch, v1
Nikolas Zimmermann
Comment 4
2022-04-04 13:40:30 PDT
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
Comment 5
2022-04-04 15:32:46 PDT
- '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
Comment 6
2022-04-05 15:00:51 PDT
Created
attachment 456755
[details]
Patch, v2
Nikolas Zimmermann
Comment 7
2022-04-07 16:06:39 PDT
Created
attachment 456980
[details]
Patch, v3
Nikolas Zimmermann
Comment 8
2022-04-07 16:07:10 PDT
Now that both 237554 and 237701 are in, it's tempting to see what EWS thinks about the patch :-) Let's try.
Nikolas Zimmermann
Comment 9
2022-04-08 03:56:47 PDT
Created
attachment 457041
[details]
Patch, v4
Nikolas Zimmermann
Comment 10
2022-04-10 15:57:37 PDT
Created
attachment 457214
[details]
Patch, v5
Nikolas Zimmermann
Comment 11
2022-04-10 16:04:53 PDT
Created
attachment 457215
[details]
Patch, v6
Nikolas Zimmermann
Comment 12
2022-04-11 05:58:26 PDT
Created
attachment 457251
[details]
Patch, v7
Nikolas Zimmermann
Comment 13
2022-04-20 00:55:02 PDT
Simon, did you have any chance to have a look at it?
Nikolas Zimmermann
Comment 14
2022-05-06 01:18:44 PDT
Yikes, it's pending review since almost a month.... all experts busy? :-)
Simon Fraser (smfr)
Comment 15
2022-05-16 13:29:10 PDT
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
Comment 16
2022-05-18 05:10:01 PDT
(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
Comment 17
2022-05-18 05:13:42 PDT
Pull request:
https://github.com/WebKit/WebKit/pull/720
EWS
Comment 18
2022-05-21 16:17:25 PDT
Committed
r294615
(
250841@main
): <
https://commits.webkit.org/250841@main
> Reviewed commits have been landed. Closing PR #720 and removing active labels.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug