Changeset 76537 in webkit


Ignore:
Timestamp:
Jan 24, 2011 11:42:08 AM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-01-24 Shane Stephens <​shanestephens@google.com>

Reviewed by Chris Marrin.

TransformationMatrix multiply operations apply operands in wrong order.
​https://bugs.webkit.org/show_bug.cgi?id=52780

Rename TranformationMatrix::multLeft into multiply (the method does a multRight,
not a multLeft).

Remove TransformationMatrix::multiply, which was actually doing a multLeft.

Fix TransformationMatrix::operator* and operator*= such that the operand is
applied to the right-hand side of the matrix that the method is called on.
i.e., previously "a * b" used to compute "b * a", and "a *= b" used to store
"b * a" in "a". This has now been fixed so "a * b" computes "a * b" and
"a *= b" stores "a * b" in "a".

Convert all call sites for these methods to provide operands in the correct order.

No new tests as patch adds no new functionality.

  • css/WebKitCSSMatrix.cpp: (WebCore::WebKitCSSMatrix::multiply):
  • platform/graphics/transforms/Matrix3DTransformOperation.h: (WebCore::Matrix3DTransformOperation::apply):
  • platform/graphics/transforms/MatrixTransformOperation.h: (WebCore::MatrixTransformOperation::apply):
  • platform/graphics/transforms/TransformationMatrix.cpp: (WebCore::TransformationMatrix::scaleNonUniform): (WebCore::TransformationMatrix::scale3d): (WebCore::TransformationMatrix::rotate3d): (WebCore::TransformationMatrix::skew): (WebCore::TransformationMatrix::applyPerspective): (WebCore::TransformationMatrix::multiply): (WebCore::TransformationMatrix::recompose):
  • platform/graphics/transforms/TransformationMatrix.h: (WebCore::TransformationMatrix::operator*=): (WebCore::TransformationMatrix::operator*):
  • rendering/RenderLayer.cpp: (WebCore::transparencyClipBox):
  • rendering/RenderObject.cpp: (WebCore::RenderObject::getTransformFromContainer):
  • rendering/TransformState.cpp: (WebCore::TransformState::applyTransform): (WebCore::HitTestingTransformState::applyTransform):
Location:
trunk/Source/WebCore
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r76531 r76537  
     12011-01-24  Shane Stephens  <shanestephens@google.com>
     2
     3        Reviewed by Chris Marrin.
     4
     5        TransformationMatrix multiply operations apply operands in wrong order.
     6        https://bugs.webkit.org/show_bug.cgi?id=52780
     7
     8        Rename TranformationMatrix::multLeft into multiply (the method does a multRight,
     9        not a multLeft).
     10
     11        Remove TransformationMatrix::multiply, which was actually doing a multLeft.
     12
     13        Fix TransformationMatrix::operator* and operator*= such that the operand is
     14        applied to the right-hand side of the matrix that the method is called on.
     15        i.e., previously "a * b" used to compute "b * a", and "a *= b" used to store
     16        "b * a" in "a".  This has now been fixed so "a * b" computes "a * b" and
     17        "a *= b" stores "a * b" in "a".
     18
     19        Convert all call sites for these methods to provide operands in the correct order.
     20
     21        No new tests as patch adds no new functionality.
     22
     23        * css/WebKitCSSMatrix.cpp:
     24        (WebCore::WebKitCSSMatrix::multiply):
     25        * platform/graphics/transforms/Matrix3DTransformOperation.h:
     26        (WebCore::Matrix3DTransformOperation::apply):
     27        * platform/graphics/transforms/MatrixTransformOperation.h:
     28        (WebCore::MatrixTransformOperation::apply):
     29        * platform/graphics/transforms/TransformationMatrix.cpp:
     30        (WebCore::TransformationMatrix::scaleNonUniform):
     31        (WebCore::TransformationMatrix::scale3d):
     32        (WebCore::TransformationMatrix::rotate3d):
     33        (WebCore::TransformationMatrix::skew):
     34        (WebCore::TransformationMatrix::applyPerspective):
     35        (WebCore::TransformationMatrix::multiply):
     36        (WebCore::TransformationMatrix::recompose):
     37        * platform/graphics/transforms/TransformationMatrix.h:
     38        (WebCore::TransformationMatrix::operator*=):
     39        (WebCore::TransformationMatrix::operator*):
     40        * rendering/RenderLayer.cpp:
     41        (WebCore::transparencyClipBox):
     42        * rendering/RenderObject.cpp:
     43        (WebCore::RenderObject::getTransformFromContainer):
     44        * rendering/TransformState.cpp:
     45        (WebCore::TransformState::applyTransform):
     46        (WebCore::HitTestingTransformState::applyTransform):
     47
    1482011-01-24  Andrei Popescu  <andreip@google.com>
    249

  • trunk/Source/WebCore/css/WebKitCSSMatrix.cpp

    r76170 r76537  
    9292        return 0;
    9393
    94     TransformationMatrix tmp(secondMatrix->m_matrix);
    95     tmp.multiply(m_matrix);
    96     return WebKitCSSMatrix::create(tmp);
     94    return WebKitCSSMatrix::create(TransformationMatrix(m_matrix).multiply(secondMatrix->m_matrix));
    9795}
    9896

  • trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp

    r76475 r76537  
    435435
    436436    // Apply the projection matrix before sending the transform over to the shader.
    437     renderMatrix.multiply(projectionMatrix);
    438 
    439     toGLMatrix(&glMatrix[0], renderMatrix);
     437    toGLMatrix(&glMatrix[0], projectionMatrix * renderMatrix);
    440438
    441439    GLC(context, context->uniformMatrix4fv(matrixLocation, false, &glMatrix[0], 1));
    … …  
    459457    TransformationMatrix renderMatrix = drawTransform();
    460458    renderMatrix.scale3d(bounds().width(), bounds().height(), 1);
    461     renderMatrix.multiply(layerRenderer()->projectionMatrix());
    462     toGLMatrix(&glMatrix[0], renderMatrix);
     459    toGLMatrix(&glMatrix[0], layerRenderer()->projectionMatrix() * renderMatrix);
    463460    GraphicsContext3D* context = layerRendererContext();
    464461    GLC(context, context->uniformMatrix4fv(sv->borderShaderMatrixLocation(), false, &glMatrix[0], 1));

  • trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp

    r76475 r76537  
    388388    TransformationMatrix renderMatrix = matrix;
    389389    renderMatrix.scale3d(layer->bounds().width(), layer->bounds().height(), 1);
    390     renderMatrix.multiply(m_projectionMatrix);
     390    renderMatrix = m_projectionMatrix * renderMatrix;
    391391
    392392    FloatRect layerRect(-0.5, -0.5, 1, 1);
    … …  
    434434    layerLocalTransform.translate3d(position.x(), position.y(), layer->anchorPointZ());
    435435    // LT = Tr[l] * M[l]
    436     layerLocalTransform.multLeft(layer->transform());
     436    layerLocalTransform.multiply(layer->transform());
    437437    // LT = Tr[l] * M[l] * Tr[c]
    438438    layerLocalTransform.translate3d(centerOffsetX, centerOffsetY, -layer->anchorPointZ());
    439439
    440440    TransformationMatrix combinedTransform = parentMatrix;
    441     combinedTransform = combinedTransform.multLeft(layerLocalTransform);
     441    combinedTransform = combinedTransform.multiply(layerLocalTransform);
    442442
    443443    FloatRect layerRect(-0.5 * layer->bounds().width(), -0.5 * layer->bounds().height(), layer->bounds().width(), layer->bounds().height());
    … …  
    549549
    550550    // Apply the sublayer transform at the center of the layer.
    551     sublayerMatrix.multLeft(layer->sublayerTransform());
     551    sublayerMatrix.multiply(layer->sublayerTransform());
    552552
    553553    // The origin of the sublayers is the top left corner of the layer, not the

  • trunk/Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp

    r71538 r76537  
    391391    GL_CMD(glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 0, unitRect))
    392392
    393     TransformationMatrix matrix = TransformationMatrix(data().projectionMatrix).multLeft(modelViewMatrix).multLeft(TransformationMatrix(
     393    TransformationMatrix matrix = TransformationMatrix(data().projectionMatrix).multiply(modelViewMatrix).multiply(TransformationMatrix(
    394394            targetRect.width(), 0, 0, 0,
    395395            0, targetRect.height(), 0, 0,
    … …  
    607607
    608608    // Create the model-view-projection matrix to display on screen.
    609     TransformationMatrix matrix = createProjectionMatrix(surfaceSize, true).multLeft(transform).multLeft(
     609    TransformationMatrix matrix = createProjectionMatrix(surfaceSize, true).multiply(transform).multiply(
    610610                TransformationMatrix(
    611611                        surface.m_actualSize.width(), 0, 0, 0,

  • trunk/Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp

    r75870 r76537  
    530530    localTransform
    531531            .translate3d(originX + m_state.pos.x(), originY + m_state.pos.y(), m_state.anchorPoint.z())
    532             .multLeft(m_baseTransform)
     532            .multiply(m_baseTransform)
    533533            .translate3d(-originX, -originY, -m_state.anchorPoint.z());
    534534
    535535    // This is the actual 3D transform of this item, with the ancestors' transform baked in.
    536536    m_transformRelativeToRootLayer = TransformationMatrix(parent ? parent->m_transformRelativeToRootLayer : TransformationMatrix())
    537                                          .multLeft(localTransform);
     537                                         .multiply(localTransform);
    538538
    539539    // Now we have enough information to determine if the layer is facing backwards.
    … …  
    563563        m_transformRelativeToRootLayer
    564564            .translate(m_size.width() / 2, m_size.height() /2)
    565             .multLeft(m_state.childrenTransform)
     565            .multiply(m_state.childrenTransform)
    566566            .translate(-m_size.width() / 2, -m_size.height() /2);
    567567    }

  • trunk/Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp

    r71538 r76537  
    319319        TransformationMatrix()
    320320        .translate3d(originX + m_state.pos.x(), originY + m_state.pos.y(), m_state.anchorPoint.z())
    321         .multLeft(m_state.transform)
     321        .multiply(m_state.transform)
    322322        .translate3d(-originX, -originY, -m_state.anchorPoint.z());
    323323    m_transforms.localDirty = false;
    … …  
    353353
    354354    if (m_layerType != TransparencyLayer) {
    355         m_transforms.replica = TransformationMatrix(m_transforms.target).multLeft(m_state.replicaLayer->m_transforms.local);
     355        m_transforms.replica = TransformationMatrix(m_transforms.target).multiply(m_state.replicaLayer->m_transforms.local);
    356356        return;
    357357    }
    … …  
    362362            TransformationMatrix()
    363363                .translate(originX, originY)
    364                 .multLeft(m_state.replicaLayer->m_transforms.local)
     364                .multiply(m_state.replicaLayer->m_transforms.local)
    365365                .translate(-originX, -originY);
    366366}
    … …  
    378378    computeLocalTransform();
    379379
    380     m_transforms.target = TransformationMatrix(parent ? parent->m_transforms.forDescendants : TransformationMatrix()).multLeft(m_transforms.local);
     380    m_transforms.target = TransformationMatrix(parent ? parent->m_transforms.forDescendants : TransformationMatrix()).multiply(m_transforms.local);
    381381    m_transforms.forDescendants = (m_layerType == ClipLayer ? TransformationMatrix() : m_transforms.target);
    382382
    … …  
    409409        m_transforms.perspective = TransformationMatrix()
    410410            .translate(centerPoint.x(), centerPoint.y())
    411             .multLeft(m_state.childrenTransform)
     411            .multiply(m_state.childrenTransform)
    412412            .translate(-centerPoint.x(), -centerPoint.y());
    413413    m_transforms.perspectiveDirty = false;
    414     m_transforms.forDescendants.multLeft(m_transforms.perspective);
     414    m_transforms.forDescendants.multiply(m_transforms.perspective);
    415415}
    416416

  • trunk/Source/WebCore/platform/graphics/transforms/Matrix3DTransformOperation.h

    r69884 r76537  
    5656    virtual bool apply(TransformationMatrix& transform, const IntSize&) const
    5757    {
    58         transform.multLeft(TransformationMatrix(m_matrix));
     58        transform.multiply(TransformationMatrix(m_matrix));
    5959        return false;
    6060    }

  • trunk/Source/WebCore/platform/graphics/transforms/MatrixTransformOperation.h

    r69884 r76537  
    6363    {
    6464        TransformationMatrix matrix(m_a, m_b, m_c, m_d, m_e, m_f);
    65         transform.multLeft(TransformationMatrix(matrix));
     65        transform.multiply(matrix);
    6666        return false;
    6767    }

  • trunk/Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp

    r76170 r76537  
    5656// with no guarantee.
    5757
    58 // A Note About row-major vs. column major matrixes
     58// A clarification about the storage of matrix elements
    5959//
    60 // The clients of this class (CSSMatrix and SVGMatrix) assume a column-major ordering.
    61 // That means that when the matrix is initialized with 16 values, the first 4 values
    62 // go in the 4 rows of the first column, etc. And in the dereferencing calls, the first
    63 // digit is the column (e.g., m23() is column 2 row 3). Because C++ uses row-major arrays
    64 // the internal matrix is stored in row-major order, so m[2][0] means row 2, column 0. This
    65 // has no bearing on how the matrix is viewed on the outside, since all access is done
    66 // with function calls. But it does help make the code more clear if you know that.
     60// This class uses a 2 dimensional array internally to store the elements of the matrix.  The first index into
     61// the array refers to the column that the element lies in; the second index refers to the row.
    6762//
    68 // FIXME: Multiply calls are named for what they do in the internal, row-major world.
    69 // multLeft is actually a multRight in a column-major world, and multiply is a multLeft
    70 // in a column-major world. For now I've left it that way to avoid too many confusing
    71 // changes to the code. In particular AffineTransform uses these same terms for the
    72 // opposite operations. So we have to be VERY careful when we change them.
     63// In other words, this is the layout of the matrix:
     64//
     65// | m_matrix[0][0] m_matrix[1][0] m_matrix[2][0] m_matrix[3][0] |
     66// | m_matrix[0][1] m_matrix[1][1] m_matrix[2][1] m_matrix[3][1] |
     67// | m_matrix[0][2] m_matrix[1][2] m_matrix[2][2] m_matrix[3][2] |
     68// | m_matrix[0][3] m_matrix[1][3] m_matrix[2][3] m_matrix[3][3] |
    7369
    7470typedef double Vector4[4];
    … …  
    635631    mat.m_matrix[1][1] = sy;
    636632
    637     multLeft(mat);
     633    multiply(mat);
    638634    return *this;
    639635}
    … …  
    646642    mat.m_matrix[2][2] = sz;
    647643
    648     multLeft(mat);
     644    multiply(mat);
    649645    return *this;
    650646}
    … …  
    733729        mat.m_matrix[3][3] = 1.0f;
    734730    }
    735     multLeft(mat);
     731    multiply(mat);
    736732    return *this;
    737733}
    … …  
    784780    mat.m_matrix[3][3] = 1.0f;
    785781   
    786     rmat.multLeft(mat);
     782    rmat.multiply(mat);
    787783
    788784    rx /= 2.0f;
    … …  
    804800    mat.m_matrix[3][3] = 1.0f;
    805801   
    806     rmat.multLeft(mat);
    807 
    808     multLeft(rmat);
     802    rmat.multiply(mat);
     803
     804    multiply(rmat);
    809805    return *this;
    810806}
    … …  
    870866    mat.m_matrix[1][0] = tan(sx); // and the x shear in the second row
    871867
    872     multLeft(mat);
     868    multiply(mat);
    873869    return *this;
    874870}
    … …  
    880876        mat.m_matrix[2][3] = -1/p;
    881877
    882     multLeft(mat);
     878    multiply(mat);
    883879    return *this;
    884880}
    … …  
    897893// *this = mat * *this
    898894//
    899 TransformationMatrix& TransformationMatrix::multLeft(const TransformationMatrix& mat)
     895TransformationMatrix& TransformationMatrix::multiply(const TransformationMatrix& mat)
    900896{
    901897    Matrix4 tmp;
    … …  
    11061102                           0, 0, 0, 1);
    11071103   
    1108     multLeft(rotationMatrix);
     1104    multiply(rotationMatrix);
    11091105   
    11101106    // now apply skew
    … …  
    11121108        TransformationMatrix tmp;
    11131109        tmp.setM32((float) decomp.skewYZ);
    1114         multLeft(tmp);
     1110        multiply(tmp);
    11151111    }
    11161112   
    … …  
    11181114        TransformationMatrix tmp;
    11191115        tmp.setM31((float) decomp.skewXZ);
    1120         multLeft(tmp);
     1116        multiply(tmp);
    11211117    }
    11221118   
    … …  
    11241120        TransformationMatrix tmp;
    11251121        tmp.setM21((float) decomp.skewXY);
    1126         multLeft(tmp);
     1122        multiply(tmp);
    11271123    }
    11281124   

  • trunk/Source/WebCore/platform/graphics/transforms/TransformationMatrix.h

    r76248 r76537  
    209209
    210210    // this = this * mat
    211     TransformationMatrix& multiply(const TransformationMatrix& t) { return *this *= t; }
    212 
    213     // this = mat * this
    214     TransformationMatrix& multLeft(const TransformationMatrix& mat);
    215    
     211    TransformationMatrix& multiply(const TransformationMatrix&);
     212
    216213    TransformationMatrix& scale(double);
    217214    TransformationMatrix& scaleNonUniform(double sx, double sy);
    … …  
    297294
    298295    bool operator!=(const TransformationMatrix& other) const { return !(*this == other); }
    299    
    300     // *this = *this * t (i.e., a multRight)
     296
     297    // *this = *this * t
    301298    TransformationMatrix& operator*=(const TransformationMatrix& t)
    302299    {
    303         *this = *this * t;
    304         return *this;
    305     }
    306    
    307     // result = *this * t (i.e., a multRight)
     300        return multiply(t);
     301    }
     302   
     303    // result = *this * t
    308304    TransformationMatrix operator*(const TransformationMatrix& t) const
    309305    {
    310         TransformationMatrix result = t;
    311         result.multLeft(*this);
     306        TransformationMatrix result = *this;
     307        result.multiply(t);
    312308        return result;
    313309    }

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r76378 r76537  
    946946        TransformationMatrix transform;
    947947        transform.translate(x, y);
    948         transform = *l->transform() * transform;
     948        transform = transform * *l->transform();
    949949
    950950        IntRect clipRect = l->boundingBox(l);

  • trunk/Source/WebCore/rendering/RenderObject.cpp

    r76083 r76537  
    19871987    RenderLayer* layer;
    19881988    if (hasLayer() && (layer = toRenderBoxModelObject(this)->layer()) && layer->transform())
    1989         transform.multLeft(layer->currentTransform());
     1989        transform.multiply(layer->currentTransform());
    19901990   
    19911991#if ENABLE(3D_RENDERING)
    … …  
    19991999       
    20002000        transform.translateRight3d(-perspectiveOrigin.x(), -perspectiveOrigin.y(), 0);
    2001         transform.multiply(perspectiveMatrix);
     2001        transform = perspectiveMatrix * transform;
    20022002        transform.translateRight3d(perspectiveOrigin.x(), perspectiveOrigin.y(), 0);
    20032003    }

  • trunk/Source/WebCore/rendering/TransformState.cpp

    r54503 r76537  
    6161    if (m_accumulatedTransform) {
    6262        if (m_direction == ApplyTransformDirection)
     63            m_accumulatedTransform.set(new TransformationMatrix(transformFromContainer * *m_accumulatedTransform));
     64        else
    6365            m_accumulatedTransform->multiply(transformFromContainer);
    64         else
    65             m_accumulatedTransform->multLeft(transformFromContainer);
    6666    } else if (accumulate == AccumulateTransform) {
    6767        // Make one if we started to accumulate
    … …  
    141141void HitTestingTransformState::applyTransform(const TransformationMatrix& transformFromContainer, TransformAccumulation accumulate)
    142142{
    143     m_accumulatedTransform.multLeft(transformFromContainer);   
     143    m_accumulatedTransform.multiply(transformFromContainer);
    144144    if (accumulate == FlattenTransform)
    145145        flattenWithTransform(m_accumulatedTransform);
Note: See TracChangeset for help on using the changeset viewer.