Skip to content

Commit

Permalink
[web-animations] don't normalize progress for visibility and `conte…
Browse files Browse the repository at this point in the history 
…nt-visibility` while animating

https://bugs.webkit.org/show_bug.cgi?id=268366

Reviewed by Tim Nguyen.

For CSS properties using a "discrete" animation type, we normalize the animated value to be either
0 or 1 depending on which side of the 0.5 progress value we stand. However, the `visibility` and
`content-visibility` properties have a special type of discrete animation support that requires
to not perform this value normalization, as will the `display` property when we add animation
support for it (see bug 267762).

We introduce a new wrapper type `NonNormalizedDiscretePropertyWrapper` to do just that and use
it for the two properties listed above. This addresses the last remaining failure for the
`content-visibility` interpolation test.

* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-interpolation-expected.txt:
* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::AnimationPropertyWrapperBase::normalizesProgressForDiscreteInterpolation const):
(WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap):
(WebCore::blendStandardProperty):

Canonical link: https://commits.webkit.org/273742@main
  • Loading branch information
@graouts
graouts committed Jan 30, 2024
1 parent 2a0d30d commit e4117be
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 10 deletions.
12 changes: 6 additions & 6 deletions ...rm-tests/css/css-contain/content-visibility/content-visibility-interpolation-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ PASS Web Animations: property <content-visibility> from [hidden] to [visible] at
PASS Web Animations: property <content-visibility> from [hidden] to [visible] at (0.9) should be [visible]
PASS Web Animations: property <content-visibility> from [hidden] to [visible] at (1) should be [visible]
PASS Web Animations: property <content-visibility> from [hidden] to [visible] at (1.5) should be [visible]
FAIL CSS Transitions: property <content-visibility> from [auto] to [visible] at (-0.3) should be [visible] assert_equals: expected "visible " but got "auto "
FAIL CSS Transitions: property <content-visibility> from [auto] to [visible] at (0) should be [visible] assert_equals: expected "visible " but got "auto "
FAIL CSS Transitions: property <content-visibility> from [auto] to [visible] at (0.3) should be [visible] assert_equals: expected "visible " but got "auto "
PASS CSS Transitions: property <content-visibility> from [auto] to [visible] at (-0.3) should be [visible]
PASS CSS Transitions: property <content-visibility> from [auto] to [visible] at (0) should be [visible]
PASS CSS Transitions: property <content-visibility> from [auto] to [visible] at (0.3) should be [visible]
PASS CSS Transitions: property <content-visibility> from [auto] to [visible] at (0.5) should be [visible]
PASS CSS Transitions: property <content-visibility> from [auto] to [visible] at (0.6) should be [visible]
PASS CSS Transitions: property <content-visibility> from [auto] to [visible] at (1) should be [visible]
PASS CSS Transitions: property <content-visibility> from [auto] to [visible] at (1.5) should be [visible]
FAIL CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (-0.3) should be [visible] assert_equals: expected "visible " but got "auto "
FAIL CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (0) should be [visible] assert_equals: expected "visible " but got "auto "
FAIL CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (0.3) should be [visible] assert_equals: expected "visible " but got "auto "
PASS CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (-0.3) should be [visible]
PASS CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (0) should be [visible]
PASS CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (0.3) should be [visible]
PASS CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (0.5) should be [visible]
PASS CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (0.6) should be [visible]
PASS CSS Transitions with transition: all: property <content-visibility> from [auto] to [visible] at (1) should be [visible]
Expand Down
23 changes: 19 additions & 4 deletions Source/WebCore/animation/CSSPropertyAnimation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,9 @@ class AnimationPropertyWrapperBase {
virtual bool requiresBlendingForAccumulativeIteration(const RenderStyle&, const RenderStyle&) const { return false; }
virtual bool equals(const RenderStyle&, const RenderStyle&) const = 0;
virtual bool canInterpolate(const RenderStyle&, const RenderStyle&, CompositeOperation) const { return true; }
virtual bool normalizesProgressForDiscreteInterpolation() const { return true; }
virtual void blend(RenderStyle&, const RenderStyle&, const RenderStyle&, const CSSPropertyBlendingContext&) const = 0;

#if !LOG_DISABLED
virtual void logBlend(const RenderStyle&, const RenderStyle&, const RenderStyle&, double) const = 0;
#endif
Expand Down Expand Up @@ -3597,6 +3598,20 @@ class CSSPropertyAnimationWrapperMap final {
};
DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(CSSPropertyAnimationWrapperMap);

template <typename T>
class NonNormalizedDiscretePropertyWrapper final : public PropertyWrapper<T> {
WTF_MAKE_FAST_ALLOCATED_WITH_HEAP_IDENTIFIER(NonNormalizedDiscretePropertyWrapper);
public:
NonNormalizedDiscretePropertyWrapper(CSSPropertyID property, T (RenderStyle::*getter)() const, void (RenderStyle::*setter)(T))
: PropertyWrapper<T>(property, getter, setter)
{
}

private:
bool canInterpolate(const RenderStyle&, const RenderStyle&, CompositeOperation) const final { return false; }
bool normalizesProgressForDiscreteInterpolation() const final { return false; }
};

CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
{
// build the list of property wrappers to do the comparisons and blends
Expand Down Expand Up @@ -3699,7 +3714,7 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
new LengthVariantPropertyWrapper<LengthSize>(CSSPropertyBorderTopRightRadius, &RenderStyle::borderTopRightRadius, &RenderStyle::setBorderTopRightRadius),
new LengthVariantPropertyWrapper<LengthSize>(CSSPropertyBorderBottomLeftRadius, &RenderStyle::borderBottomLeftRadius, &RenderStyle::setBorderBottomLeftRadius),
new LengthVariantPropertyWrapper<LengthSize>(CSSPropertyBorderBottomRightRadius, &RenderStyle::borderBottomRightRadius, &RenderStyle::setBorderBottomRightRadius),
new PropertyWrapper<Visibility>(CSSPropertyVisibility, &RenderStyle::visibility, &RenderStyle::setVisibility),
new NonNormalizedDiscretePropertyWrapper<Visibility>(CSSPropertyVisibility, &RenderStyle::visibility, &RenderStyle::setVisibility),

new ClipWrapper,

Expand Down Expand Up @@ -3814,7 +3829,7 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
new DiscretePropertyWrapper<PrintColorAdjust>(CSSPropertyPrintColorAdjust, &RenderStyle::printColorAdjust, &RenderStyle::setPrintColorAdjust),
new DiscretePropertyWrapper<ColumnFill>(CSSPropertyColumnFill, &RenderStyle::columnFill, &RenderStyle::setColumnFill),
new DiscretePropertyWrapper<BorderStyle>(CSSPropertyColumnRuleStyle, &RenderStyle::columnRuleStyle, &RenderStyle::setColumnRuleStyle),
new PropertyWrapper<ContentVisibility>(CSSPropertyContentVisibility, &RenderStyle::contentVisibility, &RenderStyle::setContentVisibility),
new NonNormalizedDiscretePropertyWrapper<ContentVisibility>(CSSPropertyContentVisibility, &RenderStyle::contentVisibility, &RenderStyle::setContentVisibility),
new DiscretePropertyWrapper<CursorType>(CSSPropertyCursor, &RenderStyle::cursor, &RenderStyle::setCursor),
new DiscretePropertyWrapper<EmptyCell>(CSSPropertyEmptyCells, &RenderStyle::emptyCells, &RenderStyle::setEmptyCells),
new DiscretePropertyWrapper<FlexDirection>(CSSPropertyFlexDirection, &RenderStyle::flexDirection, &RenderStyle::setFlexDirection),
Expand Down Expand Up @@ -4237,7 +4252,7 @@ static void blendStandardProperty(const CSSPropertyBlendingClient& client, CSSPr
// The property's values cannot be meaningfully combined, thus it is not additive and
// interpolation swaps from Va to Vb at 50% (p=0.5).
auto isDiscrete = !wrapper->canInterpolate(from, to, compositeOperation);
if (isDiscrete) {
if (isDiscrete && wrapper->normalizesProgressForDiscreteInterpolation()) {
// If we want additive, we should specify progress at 0 actually and return from.
progress = progress < 0.5 ? 0 : 1;
compositeOperation = CompositeOperation::Replace;
Expand Down

0 comments on commit e4117be

Please sign in to comment.