2011-01-19 Shane Stephens <shanestephens@google.com>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jan 2011 20:10:07 +0000 (20:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jan 2011 20:10:07 +0000 (20:10 +0000)
        Reviewed by Chris Marrin.

        Make AffineTransform and TransformationMatrix do matrix multiplication in the correct order (Column Major)
        https://bugs.webkit.org/show_bug.cgi?id=48031

        Fixed reversal, renamed matrix methods to be clearer, found all examples of operator* and
        operator*= usage and switched operand order.

        No new tests as this refactor doesn't add functionality.

        * html/HTMLCanvasElement.cpp:
        (WebCore::HTMLCanvasElement::baseTransform):
        * html/canvas/CanvasRenderingContext2D.cpp:
        (WebCore::CanvasRenderingContext2D::transform):
        (WebCore::CanvasRenderingContext2D::setTransform):
        * platform/graphics/cg/PatternCG.cpp:
        (WebCore::Pattern::createPlatformPattern):
        * platform/graphics/chromium/GLES2Canvas.cpp:
        (WebCore::GLES2Canvas::fillRect):
        (WebCore::GLES2Canvas::concatCTM):
        (WebCore::GLES2Canvas::drawQuad):
        * platform/graphics/chromium/TransparencyWin.cpp:
        (WebCore::TransparencyWin::setupLayerForOpaqueCompositeLayer):
        (WebCore::TransparencyWin::setupTransformForKeepTransform):
        * platform/graphics/transforms/AffineTransform.cpp:
        (WebCore::AffineTransform::multiply):
        (WebCore::AffineTransform::rotate):
        * platform/graphics/transforms/AffineTransform.h:
        (WebCore::AffineTransform::operator*=):
        (WebCore::AffineTransform::operator*):
        * platform/graphics/wince/GraphicsContextWinCE.cpp:
        (WebCore::GraphicsContextPlatformPrivate::concatCTM):
        * platform/graphics/wince/SharedBitmap.cpp:
        (WebCore::SharedBitmap::drawPattern):
        * rendering/svg/RenderSVGResourceContainer.cpp:
        (WebCore::RenderSVGResourceContainer::transformOnNonScalingStroke):
        * rendering/svg/RenderSVGResourceGradient.cpp:
        (WebCore::clipToTextMask):
        (WebCore::RenderSVGResourceGradient::applyResource):
        * rendering/svg/RenderSVGResourcePattern.cpp:
        (WebCore::RenderSVGResourcePattern::applyResource):
        * rendering/svg/RenderSVGRoot.cpp:
        (WebCore::RenderSVGRoot::localToBorderBoxTransform):
        * rendering/svg/SVGImageBufferTools.cpp:
        (WebCore::SVGImageBufferTools::calculateTransformationToOutermostSVGCoordinateSystem):
        (WebCore::SVGImageBufferTools::renderSubtreeToImageBuffer):
        * rendering/svg/SVGTextLayoutEngine.cpp:
        (WebCore::SVGTextLayoutEngine::finalizeTransformMatrices):
        * svg/SVGLocatable.cpp:
        (WebCore::SVGLocatable::computeCTM):
        (WebCore::SVGLocatable::getTransformToElement):
        * svg/SVGMatrix.h:
        (WebCore::SVGMatrix::multiply):
        * svg/SVGSVGElement.cpp:
        (WebCore::SVGSVGElement::localCoordinateSpaceTransform):
        (WebCore::SVGSVGElement::viewBoxToViewTransform):
        * svg/SVGStyledTransformableElement.cpp:
        (WebCore::SVGStyledTransformableElement::animatedLocalTransform):
        * svg/SVGTextElement.cpp:
        (WebCore::SVGTextElement::animatedLocalTransform):
        * svg/SVGTransformList.cpp:
        (WebCore::SVGTransformList::concatenate):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@76146 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLCanvasElement.cpp
Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
Source/WebCore/platform/graphics/cg/PatternCG.cpp
Source/WebCore/platform/graphics/chromium/GLES2Canvas.cpp
Source/WebCore/platform/graphics/chromium/TransparencyWin.cpp
Source/WebCore/platform/graphics/transforms/AffineTransform.cpp
Source/WebCore/platform/graphics/transforms/AffineTransform.h
Source/WebCore/platform/graphics/wince/GraphicsContextWinCE.cpp
Source/WebCore/platform/graphics/wince/SharedBitmap.cpp
Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
Source/WebCore/rendering/svg/RenderSVGResourceGradient.cpp
Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp
Source/WebCore/rendering/svg/RenderSVGRoot.cpp
Source/WebCore/rendering/svg/SVGImageBufferTools.cpp
Source/WebCore/rendering/svg/SVGTextLayoutEngine.cpp
Source/WebCore/svg/SVGLocatable.cpp
Source/WebCore/svg/SVGMatrix.h
Source/WebCore/svg/SVGSVGElement.cpp
Source/WebCore/svg/SVGStyledTransformableElement.cpp
Source/WebCore/svg/SVGTextElement.cpp
Source/WebCore/svg/SVGTransformList.cpp

index 623876c..787ac96 100644 (file)
@@ -1,3 +1,68 @@
+2011-01-19  Shane Stephens  <shanestephens@google.com>
+
+        Reviewed by Chris Marrin.
+
+        Make AffineTransform and TransformationMatrix do matrix multiplication in the correct order (Column Major)
+        https://bugs.webkit.org/show_bug.cgi?id=48031
+
+        Fixed reversal, renamed matrix methods to be clearer, found all examples of operator* and
+        operator*= usage and switched operand order.
+
+        No new tests as this refactor doesn't add functionality.
+
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::baseTransform):
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::transform):
+        (WebCore::CanvasRenderingContext2D::setTransform):
+        * platform/graphics/cg/PatternCG.cpp:
+        (WebCore::Pattern::createPlatformPattern):
+        * platform/graphics/chromium/GLES2Canvas.cpp:
+        (WebCore::GLES2Canvas::fillRect):
+        (WebCore::GLES2Canvas::concatCTM):
+        (WebCore::GLES2Canvas::drawQuad):
+        * platform/graphics/chromium/TransparencyWin.cpp:
+        (WebCore::TransparencyWin::setupLayerForOpaqueCompositeLayer):
+        (WebCore::TransparencyWin::setupTransformForKeepTransform):
+        * platform/graphics/transforms/AffineTransform.cpp:
+        (WebCore::AffineTransform::multiply):
+        (WebCore::AffineTransform::rotate):
+        * platform/graphics/transforms/AffineTransform.h:
+        (WebCore::AffineTransform::operator*=):
+        (WebCore::AffineTransform::operator*):
+        * platform/graphics/wince/GraphicsContextWinCE.cpp:
+        (WebCore::GraphicsContextPlatformPrivate::concatCTM):
+        * platform/graphics/wince/SharedBitmap.cpp:
+        (WebCore::SharedBitmap::drawPattern):
+        * rendering/svg/RenderSVGResourceContainer.cpp:
+        (WebCore::RenderSVGResourceContainer::transformOnNonScalingStroke):
+        * rendering/svg/RenderSVGResourceGradient.cpp:
+        (WebCore::clipToTextMask):
+        (WebCore::RenderSVGResourceGradient::applyResource):
+        * rendering/svg/RenderSVGResourcePattern.cpp:
+        (WebCore::RenderSVGResourcePattern::applyResource):
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::localToBorderBoxTransform):
+        * rendering/svg/SVGImageBufferTools.cpp:
+        (WebCore::SVGImageBufferTools::calculateTransformationToOutermostSVGCoordinateSystem):
+        (WebCore::SVGImageBufferTools::renderSubtreeToImageBuffer):
+        * rendering/svg/SVGTextLayoutEngine.cpp:
+        (WebCore::SVGTextLayoutEngine::finalizeTransformMatrices):
+        * svg/SVGLocatable.cpp:
+        (WebCore::SVGLocatable::computeCTM):
+        (WebCore::SVGLocatable::getTransformToElement):
+        * svg/SVGMatrix.h:
+        (WebCore::SVGMatrix::multiply):
+        * svg/SVGSVGElement.cpp:
+        (WebCore::SVGSVGElement::localCoordinateSpaceTransform):
+        (WebCore::SVGSVGElement::viewBoxToViewTransform):
+        * svg/SVGStyledTransformableElement.cpp:
+        (WebCore::SVGStyledTransformableElement::animatedLocalTransform):
+        * svg/SVGTextElement.cpp:
+        (WebCore::SVGTextElement::animatedLocalTransform):
+        * svg/SVGTransformList.cpp:
+        (WebCore::SVGTransformList::concatenate):
+
 2011-01-19  Chang Shu  <chang.shu@nokia.com>
 
         Reviewed by Darin Adler.
index f286749..9d15e46 100644 (file)
@@ -446,8 +446,7 @@ AffineTransform HTMLCanvasElement::baseTransform() const
     AffineTransform transform;
     if (size.width() && size.height())
         transform.scaleNonUniform(size.width() / unscaledSize.width(), size.height() / unscaledSize.height());
-    transform.multiply(m_imageBuffer->baseTransform());
-    return transform;
+    return m_imageBuffer->baseTransform() * transform;
 }
 
 }
index 62c5793..d4cbb17 100644 (file)
@@ -537,7 +537,7 @@ void CanvasRenderingContext2D::transform(float m11, float m12, float m21, float
         return;
 
     AffineTransform transform(m11, m12, m21, m22, dx, dy);
-    AffineTransform newTransform = transform * state().m_transform;
+    AffineTransform newTransform = state().m_transform * transform;
     if (!newTransform.isInvertible()) {
         state().m_invertibleCTM = false;
         return;
@@ -562,7 +562,7 @@ void CanvasRenderingContext2D::setTransform(float m11, float m12, float m21, flo
         return;
     c->concatCTM(c->getCTM().inverse());
     c->concatCTM(canvas()->baseTransform());
-    state().m_transform.multiply(ctm.inverse());
+    state().m_transform = ctm.inverse() * state().m_transform;
     m_path.transform(ctm);
 
     state().m_invertibleCTM = true;
index 94f37b2..cfac15c 100644 (file)
@@ -54,8 +54,7 @@ CGPatternRef Pattern::createPlatformPattern(const AffineTransform& userSpaceTran
 {
     IntRect tileRect = tileImage()->rect();
 
-    AffineTransform patternTransform = m_patternSpaceTransformation;
-    patternTransform.multiply(userSpaceTransformation);
+    AffineTransform patternTransform = userSpaceTransformation * m_patternSpaceTransformation;
     patternTransform.scaleNonUniform(1, -1);
     patternTransform.translate(0, -tileRect.height());
 
index 697cf5e..ae017de 100644 (file)
@@ -108,7 +108,7 @@ void GLES2Canvas::fillRect(const FloatRect& rect, const Color& color, ColorSpace
     m_context->useQuadVertices();
 
     AffineTransform matrix(m_flipMatrix);
-    matrix.multLeft(m_state->m_ctm);
+    matrix *= m_state->m_ctm;
     matrix.translate(rect.x(), rect.y());
     matrix.scale(rect.width(), rect.height());
 
@@ -150,7 +150,7 @@ void GLES2Canvas::scale(const FloatSize& size)
 
 void GLES2Canvas::concatCTM(const AffineTransform& affine)
 {
-    m_state->m_ctm.multLeft(affine);
+    m_state->m_ctm *= affine;
 }
 
 void GLES2Canvas::save()
@@ -220,7 +220,7 @@ void GLES2Canvas::drawTexturedRectTile(Texture* texture, int tile, const FloatRe
 void GLES2Canvas::drawQuad(const IntSize& textureSize, const FloatRect& srcRect, const FloatRect& dstRect, const AffineTransform& transform, float alpha)
 {
     AffineTransform matrix(m_flipMatrix);
-    matrix.multLeft(transform);
+    matrix *= transform;
     matrix.translate(dstRect.x(), dstRect.y());
     matrix.scale(dstRect.width(), dstRect.height());
 
index 4dc2157..ba66eae 100644 (file)
@@ -252,7 +252,7 @@ void TransparencyWin::setupLayerForOpaqueCompositeLayer()
     if (m_transformMode == Untransform){ 
         // Compute the inverse mapping from the canvas space to the
         // coordinate space of our bitmap.
-        mapping = m_orgTransform.inverse() * mapping;
+        mapping *= m_orgTransform.inverse();
     }
     compositeToCopy(*m_destContext, *m_drawContext, mapping);
 
@@ -313,7 +313,7 @@ void TransparencyWin::setupTransformForKeepTransform(const IntRect& region)
         // We're making a layer, so apply the old transform to the new one
         // so it's maintained. We know the new layer has the identity
         // transform now, we we can just multiply it.
-        xform = m_orgTransform * xform;
+        xform *= m_orgTransform;
         m_drawContext->concatCTM(xform);
     }
     m_drawRect = m_sourceRect;
index f275526..3f88140 100644 (file)
@@ -164,12 +164,10 @@ AffineTransform AffineTransform::inverse() const
     return result;
 }
 
-AffineTransform& AffineTransform::multiply(const AffineTransform& other)
-{
-    return (*this) *= other;
-}
 
-AffineTransform& AffineTransform::multLeft(const AffineTransform& other)
+// Multiplies this AffineTransform by the provided AffineTransform - i.e.
+// this = this * other;
+AffineTransform& AffineTransform::multiply(const AffineTransform& other)
 {
     AffineTransform trans;
     
@@ -192,7 +190,7 @@ AffineTransform& AffineTransform::rotate(double a)
     double sinAngle = sin(a);
     AffineTransform rot(cosAngle, sinAngle, -sinAngle, cosAngle, 0, 0);
 
-    multLeft(rot);
+    multiply(rot);
     return *this;
 }
 
index baee102..2adbb39 100644 (file)
@@ -95,8 +95,7 @@ public:
 
     void makeIdentity();
 
-    AffineTransform& multiply(const AffineTransform&);
-    AffineTransform& multLeft(const AffineTransform&);
+    AffineTransform& multiply(const AffineTransform& other);
     AffineTransform& scale(double); 
     AffineTransform& scale(double sx, double sy); 
     AffineTransform& scaleNonUniform(double sx, double sy);
@@ -147,15 +146,14 @@ public:
     // *this = *this * t (i.e., a multRight)
     AffineTransform& operator*=(const AffineTransform& t)
     {
-        *this = *this * t;
-        return *this;
+        return multiply(t);
     }
     
     // result = *this * t (i.e., a multRight)
     AffineTransform operator*(const AffineTransform& t) const
     {
-        AffineTransform result = t;
-        result.multLeft(*this);
+        AffineTransform result = *this;
+        result *= t;
         return result;
     }
 
index 2def6ab..1b8a704 100644 (file)
@@ -209,7 +209,7 @@ public:
 
     void  concatCTM(const AffineTransform& transform)
     {
-        m_transform = transform * m_transform;
+        m_transform *= transform;
     }
 
     IntRect mapRect(const IntRect& rect) const
index 05d1535..168a5e2 100644 (file)
@@ -445,8 +445,7 @@ void SharedBitmap::drawPattern(HDC hdc, const AffineTransform& transform, const
         bmpHeight = tileRect.height();
     }
 
-    AffineTransform tf = transform;
-    tf *= patternTransform;
+    AffineTransform tf = patternTransform * transform;
 
     FloatRect trRect = tf.mapRect(destRect);
 
index 2a8a47f..0215e8e 100644 (file)
@@ -182,8 +182,8 @@ AffineTransform RenderSVGResourceContainer::transformOnNonScalingStroke(RenderOb
         return resourceTransform;
 
     SVGStyledTransformableElement* element = static_cast<SVGStyledTransformableElement*>(object->node());
-    AffineTransform transform = resourceTransform;
-    transform.multiply(element->getScreenCTM(SVGLocatable::DisallowStyleUpdate));
+    AffineTransform transform = element->getScreenCTM(SVGLocatable::DisallowStyleUpdate);
+    transform *= resourceTransform;
     return transform;
 }
 
index 5ad5d84..9f2bb8d 100644 (file)
@@ -133,7 +133,7 @@ static inline AffineTransform clipToTextMask(GraphicsContext* context,
         matrix.translate(maskBoundingBox.x(), maskBoundingBox.y());
         matrix.scaleNonUniform(maskBoundingBox.width(), maskBoundingBox.height());
     }
-    matrix.multLeft(gradientTransform);
+    matrix *= gradientTransform;
     return matrix;
 }
 #endif
@@ -190,7 +190,7 @@ bool RenderSVGResourceGradient::applyResource(RenderObject* object, RenderStyle*
         AffineTransform gradientTransform;
         calculateGradientTransform(gradientTransform);
 
-        gradientData->userspaceTransform.multLeft(gradientTransform);
+        gradientData->userspaceTransform *= gradientTransform;
         gradientData->gradient->setGradientSpaceTransform(gradientData->userspaceTransform);
     }
 
index 330a21a..2113a25 100644 (file)
@@ -139,7 +139,7 @@ bool RenderSVGResourcePattern::applyResource(RenderObject* object, RenderStyle*
 
         AffineTransform patternTransform = m_attributes.patternTransform();
         if (!patternTransform.isIdentity())
-            patternData->transform.multiply(patternTransform);
+            patternData->transform = patternTransform * patternData->transform;
 
         patternData->pattern->setPatternSpaceTransform(patternData->transform);
     }
index aa87b09..3a8dce9 100644 (file)
@@ -246,7 +246,7 @@ AffineTransform RenderSVGRoot::localToBorderBoxTransform() const
     float scale = svg->currentScale();
     FloatPoint translate = svg->currentTranslate();
     AffineTransform ctm(scale, 0, 0, scale, borderAndPadding.width() + translate.x(), borderAndPadding.height() + translate.y());
-    return svg->viewBoxToViewTransform(width() / scale, height() / scale) * ctm;
+    return ctm * svg->viewBoxToViewTransform(width() / scale, height() / scale);
 }
 
 IntSize RenderSVGRoot::parentOriginToBorderBox() const
index f0657af..6726694 100644 (file)
@@ -43,7 +43,7 @@ void SVGImageBufferTools::calculateTransformationToOutermostSVGCoordinateSystem(
 
     absoluteTransform = currentContentTransformation();
     while (current) {
-        absoluteTransform.multiply(current->localToParentTransform());
+        absoluteTransform = current->localToParentTransform() * absoluteTransform;
         if (current->isSVGRoot())
             break;
         current = current->parent();
@@ -83,7 +83,7 @@ void SVGImageBufferTools::renderSubtreeToImageBuffer(ImageBuffer* image, RenderO
 
     AffineTransform& contentTransformation = currentContentTransformation();
     AffineTransform savedContentTransformation = contentTransformation;
-    contentTransformation.multiply(subtreeContentTransformation);
+    contentTransformation = subtreeContentTransformation * contentTransformation;
 
     item->layoutIfNeeded();
     item->paint(info, 0, 0);
index 7eefad6..3b28d2b 100644 (file)
@@ -321,7 +321,7 @@ void SVGTextLayoutEngine::finalizeTransformMatrices(Vector<SVGInlineTextBox*>& b
             if (transform.isIdentity())
                 transform = textBoxTransformation;
             else
-                transform.multiply(textBoxTransformation);
+                transform = textBoxTransformation * transform;
         }
     }
 
index 43961d9..2a7c9be 100644 (file)
@@ -91,7 +91,8 @@ AffineTransform SVGLocatable::computeCTM(const SVGElement* element, CTMScope mod
     while (current && current->isSVGElement()) {
         SVGElement* currentElement = static_cast<SVGElement*>(current);
         if (currentElement->isStyled())
-            ctm = static_cast<SVGStyledElement*>(currentElement)->localCoordinateSpaceTransform(mode).multLeft(ctm);
+            // note that this modifies the AffineTransform returned by localCoordinateSpaceTransform(mode) too.
+            ctm = static_cast<SVGStyledElement*>(currentElement)->localCoordinateSpaceTransform(mode).multiply(ctm);
 
         // For getCTM() computation, stop at the nearest viewport element
         if (currentElement == stopAtElement)
@@ -113,7 +114,7 @@ AffineTransform SVGLocatable::getTransformToElement(SVGElement* target, Exceptio
             ec = SVGException::SVG_MATRIX_NOT_INVERTABLE;
             return ctm;
         }
-        ctm *= targetCTM.inverse();
+        ctm = targetCTM.inverse() * ctm;
     }
 
     return ctm;
index 807b703..077eb03 100644 (file)
@@ -96,11 +96,10 @@ public:
         return static_cast<SVGMatrix>(copy);
     }
 
-    // SVGMatrix::multiply needs to call SVGMatrix::multLeft.
     SVGMatrix multiply(const SVGMatrix& other)
     {
         AffineTransform copy = *this;
-        copy.multLeft(static_cast<const AffineTransform&>(other));
+        copy *= static_cast<const AffineTransform&>(other);
         return static_cast<SVGMatrix>(copy);
     }
 
index 7e04f5d..483e45f 100644 (file)
@@ -502,7 +502,7 @@ AffineTransform SVGSVGElement::localCoordinateSpaceTransform(SVGLocatable::CTMSc
         }
     }
 
-    return transform.multLeft(viewBoxTransform);
+    return transform.multiply(viewBoxTransform);
 }
 
 RenderObject* SVGSVGElement::createRenderer(RenderArena* arena, RenderStyle*)
@@ -587,12 +587,11 @@ AffineTransform SVGSVGElement::viewBoxToViewTransform(float viewWidth, float vie
         viewBoxRect = viewBox();
 
     AffineTransform ctm = SVGFitToViewBox::viewBoxToViewTransform(viewBoxRect, preserveAspectRatio(), viewWidth, viewHeight);
+
     if (useCurrentView() && currentView()) {
         AffineTransform transform;
-        if (!currentView()->transform().concatenate(transform))
-            return ctm;
-
-        return transform * ctm;
+        if (currentView()->transform().concatenate(transform))
+            ctm *= transform;
     }
 
     return ctm;
index 2334943..15528e0 100644 (file)
@@ -56,7 +56,9 @@ AffineTransform SVGStyledTransformableElement::animatedLocalTransform() const
 {
     AffineTransform matrix;
     transform().concatenate(matrix);
-    return m_supplementalTransform ? *m_supplementalTransform * matrix : matrix;
+    if (m_supplementalTransform)
+        matrix *= *m_supplementalTransform;
+    return matrix;
 }
 
 AffineTransform* SVGStyledTransformableElement::supplementalTransform()
index 7229824..b6094c9 100644 (file)
@@ -89,7 +89,9 @@ AffineTransform SVGTextElement::animatedLocalTransform() const
 {
     AffineTransform matrix;
     transform().concatenate(matrix);
-    return m_supplementalTransform ? *m_supplementalTransform * matrix : matrix;
+    if (m_supplementalTransform)
+        matrix *= *m_supplementalTransform;
+    return matrix;
 }
 
 AffineTransform* SVGTextElement::supplementalTransform()
index 52bfc85..1800b1e 100644 (file)
@@ -56,7 +56,7 @@ bool SVGTransformList::concatenate(AffineTransform& result) const
         return false;
 
     for (unsigned i = 0; i < size; ++i)
-        result = at(i).matrix() * result;
+        result *= at(i).matrix();
 
     return true;
 }