Title: [294970] trunk
Revision
294970
Author
simon.fra...@apple.com
Date
2022-05-27 17:01:34 -0700 (Fri, 27 May 2022)

Log Message

Optimize setting SVG element transforms
https://bugs.webkit.org/show_bug.cgi?id=240825

Reviewed by Said Abou-Hallawa.

When parseTransformValueGeneric() creates SVGTransformValues, it default-constructed them
and then called setRotate(), setScale() etc, which would reset m_matrix to identity a second time.

Optimize by providing static helpers for creating translate, rotate and scale SVGTransformValues
which initialize the matrix with the final value. Helpers are added to AffineTransform to
create scale, translate and rotate transforms, renaming `translation` to `makeTranslation` so
that the "scale" helper doesn't conflict with the `scale` member function.

This reduces the time spent under the Element::setAttribute() function in the MotionMark
Suits test by about 14%.

* Source/WebCore/page/FrameView.cpp:
(WebCore::FrameView::rectForViewportConstrainedObjects):
* Source/WebCore/platform/graphics/FontCascade.cpp:
(WebCore::GlyphToPathTranslator::GlyphToPathTranslator):
* Source/WebCore/rendering/svg/LegacyRenderSVGRoot.cpp:
(WebCore::LegacyRenderSVGRoot::paintReplaced):
(WebCore::LegacyRenderSVGRoot::localToParentTransform const):
* Source/WebCore/rendering/svg/RenderSVGResourceMarker.cpp:
(WebCore::RenderSVGResourceMarker::localToParentTransform const):
* Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp:
(WebCore::RenderSVGViewportContainer::calculateLocalTransform):
* Source/WebCore/svg/SVGTransform.h:
(WebCore::SVGTransform::create):
(WebCore::SVGTransform::SVGTransform):
* Source/WebCore/svg/SVGTransformList.cpp:
(WebCore::SVGTransformList::parseGeneric):
* Source/WebCore/svg/SVGTransformValue.h:
(WebCore::SVGTransformValue::translateTransformValue):
(WebCore::SVGTransformValue::rotateTransformValue):
(WebCore::SVGTransformValue::scaleTransformValue):
(WebCore::SVGTransformValue::setMatrix):
(WebCore::SVGTransformValue::matrixDidChange):
(WebCore::SVGTransformValue::setTranslate):
(WebCore::SVGTransformValue::setScale):
(WebCore::SVGTransformValue::setRotate):
* Source/WebCore/svg/SVGTransformable.cpp:
(WebCore::parseTransformValueGeneric):
* Source/WebCore/svg/properties/SVGValueProperty.h:
* Tools/TestWebKitAPI/Tests/WebCore/AffineTransform.cpp:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/251074@main

Modified Paths

Diff

Modified: trunk/Source/WebCore/page/FrameView.cpp (294969 => 294970)


--- trunk/Source/WebCore/page/FrameView.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/page/FrameView.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -1955,7 +1955,7 @@
         scaleOrigin.setX(contentRect.x() + sizeDelta.width() > 0 ? contentRect.width() * (viewportRect.x() - contentRect.x()) / sizeDelta.width() : 0);
         scaleOrigin.setY(contentRect.y() + sizeDelta.height() > 0 ? contentRect.height() * (viewportRect.y() - contentRect.y()) / sizeDelta.height() : 0);
         
-        AffineTransform rescaleTransform = AffineTransform::translation(scaleOrigin.x(), scaleOrigin.y());
+        AffineTransform rescaleTransform = AffineTransform::makeTranslation(toFloatSize(scaleOrigin));
         rescaleTransform.scale(frameScaleFactor / maxPostionedObjectsRectScale, frameScaleFactor / maxPostionedObjectsRectScale);
         rescaleTransform = CGAffineTransformTranslate(rescaleTransform, -scaleOrigin.x(), -scaleOrigin.y());
 

Modified: trunk/Source/WebCore/platform/graphics/FontCascade.cpp (294969 => 294970)


--- trunk/Source/WebCore/platform/graphics/FontCascade.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/platform/graphics/FontCascade.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -1603,7 +1603,7 @@
         , m_textRun(textRun)
         , m_glyphBuffer(glyphBuffer)
         , m_fontData(&glyphBuffer.fontAt(m_index))
-        , m_translation(AffineTransform::translation(textOrigin.x(), textOrigin.y()))
+        , m_translation(AffineTransform::makeTranslation(toFloatSize(textOrigin)))
     {
 #if USE(CG)
         m_translation.flipY();

Modified: trunk/Source/WebCore/platform/graphics/transforms/AffineTransform.h (294969 => 294970)


--- trunk/Source/WebCore/platform/graphics/transforms/AffineTransform.h	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/platform/graphics/transforms/AffineTransform.h	2022-05-28 00:01:34 UTC (rev 294970)
@@ -27,6 +27,8 @@
 #pragma once
 
 #include "CompositeOperation.h"
+#include "FloatPoint.h"
+#include "FloatSize.h"
 #include <array>
 #include <optional>
 #include <wtf/FastMalloc.h>
@@ -180,11 +182,24 @@
     WEBCORE_EXPORT operator CGAffineTransform() const;
 #endif
 
-    static AffineTransform translation(double x, double y)
+    static AffineTransform makeTranslation(FloatSize delta)
     {
-        return AffineTransform(1, 0, 0, 1, x, y);
+        return AffineTransform(1, 0, 0, 1, delta.width(), delta.height());
     }
 
+    static AffineTransform makeScale(FloatSize scale)
+    {
+        return AffineTransform(scale.width(), 0, 0, scale.height(), 0, 0);
+    }
+
+    static AffineTransform makeRotation(double angleInDegrees, FloatPoint center = { })
+    {
+        auto matrix = makeTranslation(toFloatSize(center));
+        matrix.rotate(angleInDegrees);
+        matrix.translate(-toFloatSize(center));
+        return matrix;
+    }
+
     // decompose the matrix into its component parts
     typedef struct {
         double scaleX, scaleY;

Modified: trunk/Source/WebCore/rendering/svg/LegacyRenderSVGRoot.cpp (294969 => 294970)


--- trunk/Source/WebCore/rendering/svg/LegacyRenderSVGRoot.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/rendering/svg/LegacyRenderSVGRoot.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -274,7 +274,7 @@
     // Convert from container offsets (html renderers) to a relative transform (svg renderers).
     // Transform from our paint container's coordinate system to our local coords.
     IntPoint adjustedPaintOffset = roundedIntPoint(paintOffset);
-    childPaintInfo.applyTransform(AffineTransform::translation(adjustedPaintOffset.x(), adjustedPaintOffset.y()) * localToBorderBoxTransform());
+    childPaintInfo.applyTransform(AffineTransform::makeTranslation(toFloatSize(adjustedPaintOffset)) * localToBorderBoxTransform());
 
     // SVGRenderingContext must be destroyed before we restore the childPaintInfo.context(), because a filter may have
     // changed the context and it is only reverted when the SVGRenderingContext destructor finishes applying the filter.
@@ -344,7 +344,7 @@
 
 const AffineTransform& LegacyRenderSVGRoot::localToParentTransform() const
 {
-    // Slightly optimized version of m_localToParentTransform = AffineTransform::translation(x(), y()) * m_localToBorderBoxTransform;
+    // Slightly optimized version of m_localToParentTransform = AffineTransform::makeTranslation(x(), y()) * m_localToBorderBoxTransform;
     m_localToParentTransform = m_localToBorderBoxTransform;
     if (x())
         m_localToParentTransform.setE(m_localToParentTransform.e() + roundToInt(x()));

Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceMarker.cpp (294969 => 294970)


--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceMarker.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceMarker.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -80,7 +80,7 @@
 
 const AffineTransform& RenderSVGResourceMarker::localToParentTransform() const
 {
-    m_localToParentTransform = AffineTransform::translation(m_viewport.x(), m_viewport.y()) * viewportTransform();
+    m_localToParentTransform = AffineTransform::makeTranslation(toFloatSize(m_viewport.location())) * viewportTransform();
     return m_localToParentTransform;
     // If this class were ever given a localTransform(), then the above would read:
     // return viewportTranslation * localTransform() * viewportTransform();

Modified: trunk/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp (294969 => 294970)


--- trunk/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -78,7 +78,7 @@
     if (!m_needsTransformUpdate)
         return false;
     
-    m_localToParentTransform = AffineTransform::translation(m_viewport.x(), m_viewport.y()) * viewportTransform();
+    m_localToParentTransform = AffineTransform::makeTranslation(toFloatSize(m_viewport.location())) * viewportTransform();
     m_needsTransformUpdate = false;
     return true;
 }

Modified: trunk/Source/WebCore/svg/SVGTransform.h (294969 => 294970)


--- trunk/Source/WebCore/svg/SVGTransform.h	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/svg/SVGTransform.h	2022-05-28 00:01:34 UTC (rev 294970)
@@ -49,6 +49,11 @@
         return adoptRef(*new SVGTransform(value.type(), value.matrix()->value(), value.angle(), value.rotationCenter()));
     }
 
+    static Ref<SVGTransform> create(SVGTransformValue&& value)
+    {
+        return adoptRef(*new SVGTransform(WTFMove(value)));
+    }
+
     template<typename T>
     static ExceptionOr<Ref<SVGTransform>> create(ExceptionOr<T>&& value)
     {
@@ -166,6 +171,12 @@
     {
     }
 
+    SVGTransform(SVGTransformValue&& value)
+        : Base(WTFMove(value))
+    {
+        m_value.matrix()->attach(this, SVGPropertyAccess::ReadWrite);
+    }
+
     SVGPropertyOwner* owner() const override { return m_owner; }
 
     void commitPropertyChange(SVGProperty* property) override

Modified: trunk/Source/WebCore/svg/SVGTransformList.cpp (294969 => 294970)


--- trunk/Source/WebCore/svg/SVGTransformList.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/svg/SVGTransformList.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -78,7 +78,7 @@
         if (!parsedTransformValue)
             return false;
 
-        append(SVGTransform::create(*parsedTransformValue));
+        append(SVGTransform::create(WTFMove(*parsedTransformValue)));
 
         skipOptionalSVGSpaces(buffer);
 

Modified: trunk/Source/WebCore/svg/SVGTransformValue.h (294969 => 294970)


--- trunk/Source/WebCore/svg/SVGTransformValue.h	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/svg/SVGTransformValue.h	2022-05-28 00:01:34 UTC (rev 294970)
@@ -48,7 +48,25 @@
         ConstructIdentityTransform,
         ConstructZeroTransform
     };
+    
+    static SVGTransformValue translateTransformValue(FloatSize delta)
+    {
+        return SVGTransformValue(SVG_TRANSFORM_TRANSLATE, AffineTransform::makeTranslation(delta));
+    }
 
+    static SVGTransformValue rotateTransformValue(float angleInDegrees, FloatPoint center)
+    {
+        auto result = SVGTransformValue(SVG_TRANSFORM_ROTATE, AffineTransform::makeRotation(angleInDegrees, center));
+        result.m_angle = angleInDegrees;
+        result.m_rotationCenter = center;
+        return result;
+    }
+
+    static SVGTransformValue scaleTransformValue(FloatSize scale)
+    {
+        return SVGTransformValue(SVG_TRANSFORM_SCALE, AffineTransform::makeScale(scale));
+    }
+
     SVGTransformValue(SVGTransformType type = SVG_TRANSFORM_MATRIX, const AffineTransform& transform = { })
         : m_type(type)
         , m_matrix(SVGMatrix::create(transform))
@@ -99,7 +117,7 @@
     {
         m_type = SVG_TRANSFORM_MATRIX;
         m_angle = 0;
-        m_rotationCenter = FloatPoint();
+        m_rotationCenter = { };
         m_matrix->setValue(matrix);
     }
 
@@ -110,7 +128,7 @@
         // then the type of the SVGTransform changes to SVG_TRANSFORM_MATRIX.
         m_type = SVG_TRANSFORM_MATRIX;
         m_angle = 0;
-        m_rotationCenter = FloatPoint();
+        m_rotationCenter = { };
     }
 
     FloatPoint translate() const
@@ -122,10 +140,8 @@
     {
         m_type = SVG_TRANSFORM_TRANSLATE;
         m_angle = 0;
-        m_rotationCenter = FloatPoint();
-        
-        m_matrix->value().makeIdentity();
-        m_matrix->value().translate(tx, ty);
+        m_rotationCenter = { };
+        m_matrix->value() = AffineTransform::makeTranslation({ tx, ty });
     }
 
     FloatSize scale() const
@@ -137,23 +153,16 @@
     {
         m_type = SVG_TRANSFORM_SCALE;
         m_angle = 0;
-        m_rotationCenter = FloatPoint();
-        
-        m_matrix->value().makeIdentity();
-        m_matrix->value().scaleNonUniform(sx, sy);
+        m_rotationCenter = { };
+        m_matrix->value() = AffineTransform::makeScale({ sx, sy });
     }
 
-    void setRotate(float angle, float cx, float cy)
+    void setRotate(float angleInDegrees, float cx, float cy)
     {
         m_type = SVG_TRANSFORM_ROTATE;
-        m_angle = angle;
-        m_rotationCenter = FloatPoint(cx, cy);
-
-        // TODO: toString() implementation, which can show cx, cy (need to be stored?)
-        m_matrix->value().makeIdentity();
-        m_matrix->value().translate(cx, cy);
-        m_matrix->value().rotate(angle);
-        m_matrix->value().translate(-cx, -cy);
+        m_angle = angleInDegrees;
+        m_rotationCenter = { cx, cy };
+        m_matrix->value() = AffineTransform::makeRotation(angleInDegrees, { cx, cy });
     }
 
     void setSkewX(float angle)

Modified: trunk/Source/WebCore/svg/SVGTransformable.cpp (294969 => 294970)


--- trunk/Source/WebCore/svg/SVGTransformable.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/svg/SVGTransformable.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -105,45 +105,51 @@
         return std::nullopt;
 
     int valueCount = 0;
-    float values[] = {0, 0, 0, 0, 0, 0};
+    float values[] = { 0, 0, 0, 0, 0, 0 };
     if ((valueCount = parseTransformParamList(buffer, values, requiredValuesForType[type], optionalValuesForType[type])) < 0)
         return std::nullopt;
 
-    SVGTransformValue transform;
     switch (type) {
     case SVGTransformValue::SVG_TRANSFORM_UNKNOWN:
         ASSERT_NOT_REACHED();
-        break;
-    case SVGTransformValue::SVG_TRANSFORM_SKEWX:
+        return std::nullopt;
+
+    case SVGTransformValue::SVG_TRANSFORM_SKEWX: {
+        SVGTransformValue transform;
         transform.setSkewX(values[0]);
-        break;
-    case SVGTransformValue::SVG_TRANSFORM_SKEWY:
+        return transform;
+    }
+    case SVGTransformValue::SVG_TRANSFORM_SKEWY: {
+        SVGTransformValue transform;
         transform.setSkewY(values[0]);
-        break;
+        return transform;
+    }
     case SVGTransformValue::SVG_TRANSFORM_SCALE:
         if (valueCount == 1) // Spec: if only one param given, assume uniform scaling
-            transform.setScale(values[0], values[0]);
-        else
-            transform.setScale(values[0], values[1]);
-        break;
+            return SVGTransformValue::scaleTransformValue({ values[0], values[0] });
+
+        return SVGTransformValue::scaleTransformValue({ values[0], values[1] });
+
     case SVGTransformValue::SVG_TRANSFORM_TRANSLATE:
         if (valueCount == 1) // Spec: if only one param given, assume 2nd param to be 0
-            transform.setTranslate(values[0], 0);
-        else
-            transform.setTranslate(values[0], values[1]);
-        break;
+            return SVGTransformValue::translateTransformValue({ values[0], 0 });
+
+        return SVGTransformValue::translateTransformValue({ values[0], values[1] });
+
     case SVGTransformValue::SVG_TRANSFORM_ROTATE:
         if (valueCount == 1)
-            transform.setRotate(values[0], 0, 0);
-        else
-            transform.setRotate(values[0], values[1], values[2]);
-        break;
-    case SVGTransformValue::SVG_TRANSFORM_MATRIX:
+            return SVGTransformValue::rotateTransformValue(values[0], { });
+
+        return SVGTransformValue::rotateTransformValue(values[0], { values[1], values[2] });
+
+    case SVGTransformValue::SVG_TRANSFORM_MATRIX: {
+        SVGTransformValue transform;
         transform.setMatrix(AffineTransform(values[0], values[1], values[2], values[3], values[4], values[5]));
-        break;
+        return transform;
     }
+    }
 
-    return transform;
+    return std::nullopt;
 }
 
 std::optional<SVGTransformValue> SVGTransformable::parseTransformValue(SVGTransformValue::SVGTransformType type, StringParsingBuffer<LChar>& buffer)

Modified: trunk/Source/WebCore/svg/properties/SVGValueProperty.h (294969 => 294970)


--- trunk/Source/WebCore/svg/properties/SVGValueProperty.h	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Source/WebCore/svg/properties/SVGValueProperty.h	2022-05-28 00:01:34 UTC (rev 294970)
@@ -57,7 +57,7 @@
     {
     }
 
-    // Needed when value should not be copied, e.g. SVGTransfromValue.
+    // Needed when value should not be copied, e.g. SVGTransformValue.
     SVGValueProperty(PropertyType&& value)
         : m_value(WTFMove(value))
     {

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/AffineTransform.cpp (294969 => 294970)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/AffineTransform.cpp	2022-05-28 00:00:04 UTC (rev 294969)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/AffineTransform.cpp	2022-05-28 00:01:34 UTC (rev 294970)
@@ -925,7 +925,7 @@
 
 TEST(AffineTransform, Translation)
 {
-    auto test = WebCore::AffineTransform::translation(-5.0, -7.0);
+    auto test = WebCore::AffineTransform::makeTranslation({ -5.0, -7.0 });
     EXPECT_DOUBLE_EQ(1.0, test.a());
     EXPECT_DOUBLE_EQ(0.0, test.b());
     EXPECT_DOUBLE_EQ(0.0, test.c());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to