[webkit-changes] [273977] trunk/Source/WebCore

Fri, 05 Mar 2021 09:43:57 -0800

Title: [273977] trunk/Source/WebCore
Revision
273977
Author
[email protected]
Date
2021-03-05 09:43:16 -0800 (Fri, 05 Mar 2021)

Log Message

Refactor CSSPropertyAnimation to specify fewer wrappers and use value() functions
https://bugs.webkit.org/show_bug.cgi?id=222751

Reviewed by Dean Jackson.

We clean up CSSPropertyAnimation by removing a few dedicated wrappers which added little value
over passing in parameters to existing wrappers: PropertyWrapperColor, PropertyWrapperAcceleratedOpacity,
PropertyWrapperAcceleratedTransform, PropertyWrapperScale, PropertyWrapperRotate and PropertyWrapperTranslate
are out. For most of those we introduce the new AcceleratedPropertyWrapper to replace them.

Additionally, there is a value() method that can be used instead of using m_getter in lots of places,
which makes the code much more readable.

* animation/CSSPropertyAnimation.cpp:
(WebCore::AcceleratedPropertyWrapper::AcceleratedPropertyWrapper):
(WebCore::PropertyWrapperVisitedAffectedColor::PropertyWrapperVisitedAffectedColor):
(WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap):
(WebCore::PropertyWrapperColor::PropertyWrapperColor): Deleted.
(WebCore::PropertyWrapperAcceleratedOpacity::PropertyWrapperAcceleratedOpacity): Deleted.
(WebCore::PropertyWrapperAcceleratedTransform::PropertyWrapperAcceleratedTransform): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (273976 => 273977)


--- trunk/Source/WebCore/ChangeLog	2021-03-05 17:38:47 UTC (rev 273976)
+++ trunk/Source/WebCore/ChangeLog	2021-03-05 17:43:16 UTC (rev 273977)
@@ -1,3 +1,26 @@
+2021-03-04  Antoine Quint  <[email protected]>
+
+        Refactor CSSPropertyAnimation to specify fewer wrappers and use value() functions
+        https://bugs.webkit.org/show_bug.cgi?id=222751
+
+        Reviewed by Dean Jackson.
+
+        We clean up CSSPropertyAnimation by removing a few dedicated wrappers which added little value
+        over passing in parameters to existing wrappers: PropertyWrapperColor, PropertyWrapperAcceleratedOpacity,
+        PropertyWrapperAcceleratedTransform, PropertyWrapperScale, PropertyWrapperRotate and PropertyWrapperTranslate
+        are out. For most of those we introduce the new AcceleratedPropertyWrapper to replace them.
+
+        Additionally, there is a value() method that can be used instead of using m_getter in lots of places,
+        which makes the code much more readable.
+
+        * animation/CSSPropertyAnimation.cpp:
+        (WebCore::AcceleratedPropertyWrapper::AcceleratedPropertyWrapper):
+        (WebCore::PropertyWrapperVisitedAffectedColor::PropertyWrapperVisitedAffectedColor):
+        (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap):
+        (WebCore::PropertyWrapperColor::PropertyWrapperColor): Deleted.
+        (WebCore::PropertyWrapperAcceleratedOpacity::PropertyWrapperAcceleratedOpacity): Deleted.
+        (WebCore::PropertyWrapperAcceleratedTransform::PropertyWrapperAcceleratedTransform): Deleted.
+
 2021-03-05  Zalan Bujtas  <[email protected]>
 
         [LFC][IFC] Transition LineBox::InlineLevelBoxList from a list of std::unique_ptr<InlineLevelBox> to a list of InlineLevelBox objects

Modified: trunk/Source/WebCore/animation/CSSPropertyAnimation.cpp (273976 => 273977)


--- trunk/Source/WebCore/animation/CSSPropertyAnimation.cpp	2021-03-05 17:38:47 UTC (rev 273976)
+++ trunk/Source/WebCore/animation/CSSPropertyAnimation.cpp	2021-03-05 17:43:16 UTC (rev 273977)
@@ -593,7 +593,7 @@
             return true;
         if (!a || !b)
             return false;
-        return (a->*m_getter)() == (b->*m_getter)();
+        return this->value(a) == this->value(b);
     }
 
     T value(const RenderStyle* a) const
@@ -624,7 +624,7 @@
 
     void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*PropertyWrapperGetter<T>::m_getter)(), (b->*PropertyWrapperGetter<T>::m_getter)(), progress));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress));
     }
 
 protected:
@@ -665,7 +665,7 @@
 
     void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*PropertyWrapperGetter<T*>::m_getter)(), (b->*PropertyWrapperGetter<T*>::m_getter)(), progress));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress));
     }
 
 protected:
@@ -683,12 +683,12 @@
 
     bool canInterpolate(const RenderStyle* a, const RenderStyle* b) const override
     {
-        return !(a->*PropertyWrapperGetter<const Length&>::m_getter)().isAuto() && !(b->*PropertyWrapperGetter<const Length&>::m_getter)().isAuto();
+        return !this->value(a).isAuto() && !this->value(b).isAuto();
     }
 
     void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*PropertyWrapperGetter<const Length&>::m_getter)(), (b->*PropertyWrapperGetter<const Length&>::m_getter)(), progress));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress));
     }
 
 protected:
@@ -705,7 +705,7 @@
 
     void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*PropertyWrapperGetter<const Length&>::m_getter)(), (b->*PropertyWrapperGetter<const Length&>::m_getter)(), progress, ValueRangeNonNegative));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress, ValueRangeNonNegative));
     }
 };
 
@@ -721,7 +721,7 @@
 
     void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*PropertyWrapperGetter<const T&>::m_getter)(), (b->*PropertyWrapperGetter<const T&>::m_getter)(), progress));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress));
     }
 
 protected:
@@ -761,8 +761,8 @@
             return false;
         };
 
-        auto& aLengthBox = (a->*PropertyWrapperGetter<const LengthBox&>::m_getter)();
-        auto& bLengthBox = (b->*PropertyWrapperGetter<const LengthBox&>::m_getter)();
+        auto& aLengthBox = this->value(a);
+        auto& bLengthBox = this->value(b);
         return canInterpolateBetweenLengths(aLengthBox.top(), bLengthBox.top())
             && canInterpolateBetweenLengths(aLengthBox.right(), bLengthBox.right())
             && canInterpolateBetweenLengths(aLengthBox.bottom(), bLengthBox.bottom())
@@ -800,8 +800,8 @@
         if (!a || !b)
             return false;
 
-        ClipPathOperation* clipPathA = (a->*m_getter)();
-        ClipPathOperation* clipPathB = (b->*m_getter)();
+        auto* clipPathA = this->value(a);
+        auto* clipPathB = this->value(b);
         if (clipPathA == clipPathB)
             return true;
         if (!clipPathA || !clipPathB)
@@ -827,10 +827,7 @@
             return true;
         if (!a || !b)
             return false;
-
-        const FontVariationSettings& variationSettingsA = (a->*m_getter)();
-        const FontVariationSettings& variationSettingsB = (b->*m_getter)();
-        return variationSettingsA == variationSettingsB;
+        return this->value(a) == this->value(b);
     }
 };
 #endif
@@ -852,8 +849,8 @@
         if (!a || !b)
             return false;
 
-        ShapeValue* shapeA = (a->*m_getter)();
-        ShapeValue* shapeB = (b->*m_getter)();
+        auto* shapeA = this->value(a);
+        auto* shapeB = this->value(b);
         if (shapeA == shapeB)
             return true;
         if (!shapeA || !shapeB)
@@ -877,113 +874,43 @@
         if (!a || !b)
             return false;
 
-        StyleImage* imageA = (a->*m_getter)();
-        StyleImage* imageB = (b->*m_getter)();
+        auto* imageA = this->value(a);
+        auto* imageB = this->value(b);
         return arePointingToEqualData(imageA, imageB);
     }
 };
 
-class PropertyWrapperColor : public PropertyWrapperGetter<const Color&> {
+template <typename T>
+class AcceleratedPropertyWrapper : public PropertyWrapper<T> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    PropertyWrapperColor(CSSPropertyID prop, const Color& (RenderStyle::*getter)() const, void (RenderStyle::*setter)(const Color&))
-        : PropertyWrapperGetter<const Color&>(prop, getter)
-        , m_setter(setter)
+    AcceleratedPropertyWrapper(CSSPropertyID prop, T (RenderStyle::*getter)() const, void (RenderStyle::*setter)(T))
+        : PropertyWrapper<T>(prop, getter, setter)
     {
     }
 
-    void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
-    {
-        (dst->*m_setter)(blendFunc(anim, (a->*PropertyWrapperGetter<const Color&>::m_getter)(), (b->*PropertyWrapperGetter<const Color&>::m_getter)(), progress));
-    }
-
-protected:
-    void (RenderStyle::*m_setter)(const Color&);
-};
-
-class PropertyWrapperAcceleratedOpacity : public PropertyWrapper<float> {
-    WTF_MAKE_FAST_ALLOCATED;
-public:
-    PropertyWrapperAcceleratedOpacity()
-        : PropertyWrapper<float>(CSSPropertyOpacity, &RenderStyle::opacity, &RenderStyle::setOpacity)
-    {
-    }
-
+private:
     bool animationIsAccelerated() const override { return true; }
-
-    void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
-    {
-        dst->setOpacity(blendFunc(anim, a->opacity(), b->opacity(), progress));
-    }
 };
 
-class PropertyWrapperAcceleratedTransform : public PropertyWrapper<const TransformOperations&> {
+template <typename T>
+class AcceleratedIndividualTransformPropertyWrapper final : public RefCountedPropertyWrapper<T> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    PropertyWrapperAcceleratedTransform()
-        : PropertyWrapper<const TransformOperations&>(CSSPropertyTransform, &RenderStyle::transform, &RenderStyle::setTransform)
+    AcceleratedIndividualTransformPropertyWrapper(CSSPropertyID prop, T* (RenderStyle::*getter)() const, void (RenderStyle::*setter)(RefPtr<T>&&))
+        : RefCountedPropertyWrapper<T>(prop, getter, setter)
     {
     }
 
-    bool animationIsAccelerated() const override { return true; }
-
-    void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
-    {
-        dst->setTransform(blendFunc(anim, a->transform(), b->transform(), progress));
-    }
-};
-
-class PropertyWrapperScale final : public RefCountedPropertyWrapper<ScaleTransformOperation> {
-    WTF_MAKE_FAST_ALLOCATED;
-public:
-    PropertyWrapperScale()
-        : RefCountedPropertyWrapper<ScaleTransformOperation>(CSSPropertyScale, &RenderStyle::scale, &RenderStyle::setScale)
-    {
-    }
-
 private:
     bool animationIsAccelerated() const final { return true; }
 
     bool equals(const RenderStyle* a, const RenderStyle* b) const final
     {
-        return arePointingToEqualData((a->*m_getter)(), (b->*m_getter)());
+        return arePointingToEqualData(this->value(a), this->value(b));
     }
 };
 
-class PropertyWrapperRotate final : public RefCountedPropertyWrapper<RotateTransformOperation> {
-    WTF_MAKE_FAST_ALLOCATED;
-public:
-    PropertyWrapperRotate()
-        : RefCountedPropertyWrapper<RotateTransformOperation>(CSSPropertyRotate, &RenderStyle::rotate, &RenderStyle::setRotate)
-    {
-    }
-
-private:
-    bool animationIsAccelerated() const final { return true; }
-
-    bool equals(const RenderStyle* a, const RenderStyle* b) const final
-    {
-        return arePointingToEqualData((a->*m_getter)(), (b->*m_getter)());
-    }
-};
-
-class PropertyWrapperTranslate final : public RefCountedPropertyWrapper<TranslateTransformOperation> {
-    WTF_MAKE_FAST_ALLOCATED;
-public:
-    PropertyWrapperTranslate()
-        : RefCountedPropertyWrapper<TranslateTransformOperation>(CSSPropertyTranslate, &RenderStyle::translate, &RenderStyle::setTranslate)
-    {
-    }
-
-private:
-    bool animationIsAccelerated() const final { return true; }
-
-    bool equals(const RenderStyle* a, const RenderStyle* b) const final
-    {
-        return arePointingToEqualData((a->*m_getter)(), (b->*m_getter)());
-    }
-};
-
 class PropertyWrapperFilter : public PropertyWrapper<const FilterOperations&> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -1003,7 +930,7 @@
 
     void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*PropertyWrapperGetter<const FilterOperations&>::m_getter)(), (b->*PropertyWrapperGetter<const FilterOperations&>::m_getter)(), progress, property()));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress, property()));
     }
 };
 
@@ -1231,8 +1158,8 @@
 public:
     PropertyWrapperVisitedAffectedColor(CSSPropertyID prop, const Color& (RenderStyle::*getter)() const, void (RenderStyle::*setter)(const Color&), const Color& (RenderStyle::*visitedGetter)() const, void (RenderStyle::*visitedSetter)(const Color&))
         : AnimationPropertyWrapperBase(prop)
-        , m_wrapper(makeUnique<PropertyWrapperColor>(prop, getter, setter))
-        , m_visitedWrapper(makeUnique<PropertyWrapperColor>(prop, visitedGetter, visitedSetter))
+        , m_wrapper(makeUnique<PropertyWrapper<const Color&>>(prop, getter, setter))
+        , m_visitedWrapper(makeUnique<PropertyWrapper<const Color&>>(prop, visitedGetter, visitedSetter))
     {
     }
     PropertyWrapperVisitedAffectedColor(CSSPropertyID prop, MaybeInvalidColorTag, const Color& (RenderStyle::*getter)() const, void (RenderStyle::*setter)(const Color&), const Color& (RenderStyle::*visitedGetter)() const, void (RenderStyle::*visitedSetter)(const Color&))
@@ -1303,7 +1230,7 @@
             return true;
         if (!a || !b)
             return false;
-        return (a->*m_getter)() == (b->*m_getter)();
+        return this->value(a) == this->value(b);
     }
 
     T value(const FillLayer* layer) const
@@ -1334,7 +1261,7 @@
 
     void blend(const CSSPropertyBlendingClient* anim, FillLayer* dst, const FillLayer* a, const FillLayer* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*FillLayerPropertyWrapperGetter<const T&>::m_getter)(), (b->*FillLayerPropertyWrapperGetter<const T&>::m_getter)(), progress));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress));
     }
 
 #if !LOG_DISABLED
@@ -1370,8 +1297,8 @@
         if (!a || !b)
             return false;
 
-        Length fromLength = (a->*FillLayerPropertyWrapperGetter<const Length&>::m_getter)();
-        Length toLength = (b->*FillLayerPropertyWrapperGetter<const Length&>::m_getter)();
+        auto fromLength = this->value(a);
+        auto toLength = this->value(b);
         
         Edge fromEdge = (a->*m_originGetter)();
         Edge toEdge = (b->*m_originGetter)();
@@ -1381,8 +1308,8 @@
 
     void blend(const CSSPropertyBlendingClient* anim, FillLayer* dst, const FillLayer* a, const FillLayer* b, double progress) const override
     {
-        Length fromLength = (a->*FillLayerPropertyWrapperGetter<const Length&>::m_getter)();
-        Length toLength = (b->*FillLayerPropertyWrapperGetter<const Length&>::m_getter)();
+        auto fromLength = this->value(a);
+        auto toLength = this->value(b);
         
         Edge fromEdge = (a->*m_originGetter)();
         Edge toEdge = (b->*m_originGetter)();
@@ -1426,7 +1353,7 @@
 
     void blend(const CSSPropertyBlendingClient* anim, FillLayer* dst, const FillLayer* a, const FillLayer* b, double progress) const override
     {
-        (dst->*m_setter)(blendFunc(anim, (a->*FillLayerPropertyWrapperGetter<T*>::m_getter)(), (b->*FillLayerPropertyWrapperGetter<T*>::m_getter)(), progress));
+        (dst->*m_setter)(blendFunc(anim, this->value(a), this->value(b), progress));
     }
 
 #if !LOG_DISABLED
@@ -1457,10 +1384,7 @@
             return true;
         if (!a || !b)
             return false;
-
-        StyleImage* imageA = (a->*m_getter)();
-        StyleImage* imageB = (b->*m_getter)();
-        return arePointingToEqualData(imageA, imageB);
+        return arePointingToEqualData(this->value(a), this->value(b));
     }
 
 #if !LOG_DISABLED
@@ -1893,12 +1817,12 @@
 
         new LengthBoxPropertyWrapper(CSSPropertyClip, &RenderStyle::clip, &RenderStyle::setClip),
 
-        new PropertyWrapperAcceleratedOpacity(),
-        new PropertyWrapperAcceleratedTransform(),
+        new AcceleratedPropertyWrapper<float>(CSSPropertyOpacity, &RenderStyle::opacity, &RenderStyle::setOpacity),
+        new AcceleratedPropertyWrapper<const TransformOperations&>(CSSPropertyTransform, &RenderStyle::transform, &RenderStyle::setTransform),
 
-        new PropertyWrapperScale,
-        new PropertyWrapperRotate,
-        new PropertyWrapperTranslate,
+        new AcceleratedIndividualTransformPropertyWrapper<ScaleTransformOperation>(CSSPropertyScale, &RenderStyle::scale, &RenderStyle::setScale),
+        new AcceleratedIndividualTransformPropertyWrapper<RotateTransformOperation>(CSSPropertyRotate, &RenderStyle::rotate, &RenderStyle::setRotate),
+        new AcceleratedIndividualTransformPropertyWrapper<TranslateTransformOperation>(CSSPropertyTranslate, &RenderStyle::translate, &RenderStyle::setTranslate),
 
         new PropertyWrapperFilter(CSSPropertyFilter, &RenderStyle::filter, &RenderStyle::setFilter),
 #if ENABLE(FILTERS_LEVEL_2)

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to