Fix perspective interpolation test to allow 0 as value #28036
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…orms spec](https://drafts.csswg.org/css-transforms-2/#perspective-property): > As very small <length> values can produce bizarre rendering results and stress the numerical accuracy of transform calculations, values less than 1px must be treated as 1px for rendering purposes. (This clamping does not affect the underlying value, so perspective: 0; in a stylesheet will still serialize back as 0.)
graouts
requested review from
birtles and
flackr
and removed request for
smfr,
grorg,
dbaron,
dirkschulze and
plinss
|
Firefox has the correct behavior I believe, while Chrome has the incorrect behavior where |
webkit-commit-queue
pushed a commit
to WebKit/WebKit
that referenced
this pull request
Mar 12, 2021
https://bugs.webkit.org/show_bug.cgi?id=223111 Reviewed by Antti Koivisto. LayoutTests/imported/w3c: Add an extra 59 PASS results, although we also have 18 new FAIL results. Of those new failures, 16 are due to what I believe to be an issue in the WPT test and I filed web-platform-tests/wpt#28036 to address this. With this change in the test, we pass all perspective interpolation tests. * web-platform-tests/css/css-transforms/animation/perspective-composition-expected.txt: * web-platform-tests/css/css-transforms/animation/perspective-interpolation-expected.txt: Source/WebCore: In order to correctly interplate the "perspective" CSS property, we must not interpolate between "none" values and lengths. To do this, we add a new wrapper for this property with a canInterpolate() implementation that uses RenderStyle::hasPerspective() to determine whether we're dealing with a "none" value. We also had to make a change to the way the "none" value is represented internally, since it used to be 0 although the spec (https://drafts.csswg.org/css-transforms-2/#perspective-property) says "perspective: 0 in a stylesheet will still serialize back as 0". So we now change the initial value to be -1, which is fine since negative values are otherwise not allowed. To correctly support this, we must also change consumePerspective() to no longer disallow the 0 value at parse time. * animation/CSSPropertyAnimation.cpp: (WebCore::PerspectiveWrapper::PerspectiveWrapper): (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap): * css/parser/CSSPropertyParser.cpp: (WebCore::consumePerspective): * rendering/style/RenderStyle.h: (WebCore::RenderStyle::hasPerspective const): (WebCore::RenderStyle::initialPerspective): * style/StyleBuilderConverter.h: (WebCore::Style::BuilderConverter::convertPerspective): Canonical link: https://commits.webkit.org/235217@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@274329 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bertogg
pushed a commit
to Igalia/webkit
that referenced
this pull request
Mar 12, 2021
https://bugs.webkit.org/show_bug.cgi?id=223111 Reviewed by Antti Koivisto. LayoutTests/imported/w3c: Add an extra 59 PASS results, although we also have 18 new FAIL results. Of those new failures, 16 are due to what I believe to be an issue in the WPT test and I filed web-platform-tests/wpt#28036 to address this. With this change in the test, we pass all perspective interpolation tests. * web-platform-tests/css/css-transforms/animation/perspective-composition-expected.txt: * web-platform-tests/css/css-transforms/animation/perspective-interpolation-expected.txt: Source/WebCore: In order to correctly interplate the "perspective" CSS property, we must not interpolate between "none" values and lengths. To do this, we add a new wrapper for this property with a canInterpolate() implementation that uses RenderStyle::hasPerspective() to determine whether we're dealing with a "none" value. We also had to make a change to the way the "none" value is represented internally, since it used to be 0 although the spec (https://drafts.csswg.org/css-transforms-2/#perspective-property) says "perspective: 0 in a stylesheet will still serialize back as 0". So we now change the initial value to be -1, which is fine since negative values are otherwise not allowed. To correctly support this, we must also change consumePerspective() to no longer disallow the 0 value at parse time. * animation/CSSPropertyAnimation.cpp: (WebCore::PerspectiveWrapper::PerspectiveWrapper): (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap): * css/parser/CSSPropertyParser.cpp: (WebCore::consumePerspective): * rendering/style/RenderStyle.h: (WebCore::RenderStyle::hasPerspective const): (WebCore::RenderStyle::initialPerspective): * style/StyleBuilderConverter.h: (WebCore::Style::BuilderConverter::convertPerspective): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@274329 268f45cc-cd09-0410-ab3c-d52691b4dbfc
|
So having -1 be a (still valid) 0px looks good, but -20 would be -190px, right? Is there defined behavior for clamping to the valid range when an extended transition exceeds the normal bounds? |
The existing tests expects the value to be clipped to 0, but the spec doesn't mention anything like other specs do about the value being non-negative. |
|
Okay, after combing thru several specs, as far as I can tell it is not defined what happens if an interpolation results in a value that is outside the valid range. I've opened w3c/csswg-drafts#6097 to address this. That said, I think there's an obvious correct answer (clamp to the valid range, like math functions do), and this test conforms to that, so I'm gonna go ahead and approve this ahead of the WG discussion. |
tabatkins
approved these changes
Mar 13, 2021
webkit-commit-queue
pushed a commit
to WebKit/WebKit
that referenced
this pull request
Mar 13, 2021
…hat resulted from web-platform-tests/wpt#28036 and update the expectations now that we pass this test entirely with 16 new PASS results. * web-platform-tests/css/css-transforms/animation/perspective-interpolation-expected.txt: * web-platform-tests/css/css-transforms/animation/perspective-interpolation.html: Canonical link: https://commits.webkit.org/235251@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@274384 268f45cc-cd09-0410-ab3c-d52691b4dbfc
philn
pushed a commit
to philn/old-webkit
that referenced
this pull request
Mar 14, 2021
…hat resulted from web-platform-tests/wpt#28036 and update the expectations now that we pass this test entirely with 16 new PASS results. * web-platform-tests/css/css-transforms/animation/perspective-interpolation-expected.txt: * web-platform-tests/css/css-transforms/animation/perspective-interpolation.html: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@274384 268f45cc-cd09-0410-ab3c-d52691b4dbfc
|
Following up: I confirmed in w3c/csswg-drafts#6097 that clamping is indeed the desired effect, and it was present in the spec previously before getting accidentally removed by unrelated edits. It's fixed now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
perspectiveproperty allows0as a value, per the CSS Transforms spec: