Remove SkCanvas matrix ops return value.

src/core/SkMatrix.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkMatrix.h"
 #include "SkFloatBits.h"
 #include "SkOnce.h"
@@ skipping 229 matching lines @@
         fMat[kMScaleX] = fMat[kMScaleY] = fMat[kMPersp2] = 1;
         fMat[kMSkewX]  = fMat[kMSkewY] =
         fMat[kMPersp0] = fMat[kMPersp1] = 0;
 
         this->setTypeMask(kTranslate_Mask | kRectStaysRect_Mask);
     } else {
         this->reset();
     }
 }
 
-bool SkMatrix::preTranslate(SkScalar dx, SkScalar dy) {
+void SkMatrix::preTranslate(SkScalar dx, SkScalar dy) {
+    if (!dx && !dy) {
+        return;
+    }
+
     if (this->hasPerspective()) {
         SkMatrix    m;
         m.setTranslate(dx, dy);
-        return this->preConcat(m);
+        this->preConcat(m);
+    } else {
+        fMat[kMTransX] += sdot(fMat[kMScaleX], dx, fMat[kMSkewX], dy);
+        fMat[kMTransY] += sdot(fMat[kMSkewY], dx, fMat[kMScaleY], dy);
+        this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
+    }
+}
+
+void SkMatrix::postTranslate(SkScalar dx, SkScalar dy) {
+    if (!dx && !dy) {
+        return;
     }
 
-    if (dx || dy) {
-        fMat[kMTransX] += sdot(fMat[kMScaleX], dx, fMat[kMSkewX], dy);
-        fMat[kMTransY] += sdot(fMat[kMSkewY], dx, fMat[kMScaleY], dy);
-
-        this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
-    }
-    return true;
-}
-
-bool SkMatrix::postTranslate(SkScalar dx, SkScalar dy) {
     if (this->hasPerspective()) {
         SkMatrix    m;
         m.setTranslate(dx, dy);
-        return this->postConcat(m);
-    }
-
-    if (dx || dy) {
+        this->postConcat(m);
+    } else {
         fMat[kMTransX] += dx;
         fMat[kMTransY] += dy;
         this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
     }
-    return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkMatrix::setScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
     if (1 == sx && 1 == sy) {
         this->reset();
     } else {
         fMat[kMScaleX] = sx;
         fMat[kMScaleY] = sy;
@@ skipping 25 matching lines @@
 }
 
 bool SkMatrix::setIDiv(int divx, int divy) {
     if (!divx || !divy) {
         return false;
     }
     this->setScale(SkScalarInvert(divx), SkScalarInvert(divy));
     return true;
 }
 
-bool SkMatrix::preScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
+void SkMatrix::preScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
+    if (1 == sx && 1 == sy) {
+        return;
+    }
+
     SkMatrix    m;
     m.setScale(sx, sy, px, py);
-    return this->preConcat(m);
+    this->preConcat(m);
 }
 
-bool SkMatrix::preScale(SkScalar sx, SkScalar sy) {
+void SkMatrix::preScale(SkScalar sx, SkScalar sy) {
     if (1 == sx && 1 == sy) {
-        return true;
+        return;
     }
 
     // the assumption is that these multiplies are very cheap, and that
     // a full concat and/or just computing the matrix type is more expensive.
     // Also, the fixed-point case checks for overflow, but the float doesn't,
     // so we can get away with these blind multiplies.
 
     fMat[kMScaleX] *= sx;
     fMat[kMSkewY]  *= sx;
     fMat[kMPersp0] *= sx;
 
     fMat[kMSkewX]  *= sy;
     fMat[kMScaleY] *= sy;
     fMat[kMPersp1] *= sy;
 
     this->orTypeMask(kScale_Mask);
-    return true;
 }
 
-bool SkMatrix::postScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
+void SkMatrix::postScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
     if (1 == sx && 1 == sy) {
-        return true;
+        return;
     }
     SkMatrix    m;
     m.setScale(sx, sy, px, py);
-    return this->postConcat(m);
+    this->postConcat(m);
 }
 
-bool SkMatrix::postScale(SkScalar sx, SkScalar sy) {
+void SkMatrix::postScale(SkScalar sx, SkScalar sy) {
     if (1 == sx && 1 == sy) {
-        return true;
+        return;
     }
     SkMatrix    m;
     m.setScale(sx, sy);
-    return this->postConcat(m);
+    this->postConcat(m);
 }
 
 // this guy perhaps can go away, if we have a fract/high-precision way to
 // scale matrices
 bool SkMatrix::postIDiv(int divx, int divy) {
     if (divx == 0 || divy == 0) {
         return false;
     }
 
     const float invX = 1.f / divx;
@@ skipping 51 matching lines @@
     sinV = SkScalarSinCos(SkDegreesToRadians(degrees), &cosV);
     this->setSinCos(sinV, cosV, px, py);
 }
 
 void SkMatrix::setRotate(SkScalar degrees) {
     SkScalar sinV, cosV;
     sinV = SkScalarSinCos(SkDegreesToRadians(degrees), &cosV);
     this->setSinCos(sinV, cosV);
 }
 
-bool SkMatrix::preRotate(SkScalar degrees, SkScalar px, SkScalar py) {
+void SkMatrix::preRotate(SkScalar degrees, SkScalar px, SkScalar py) {
     SkMatrix    m;
     m.setRotate(degrees, px, py);
-    return this->preConcat(m);
+    this->preConcat(m);
 }
 
-bool SkMatrix::preRotate(SkScalar degrees) {
+void SkMatrix::preRotate(SkScalar degrees) {
     SkMatrix    m;
     m.setRotate(degrees);
-    return this->preConcat(m);
+    this->preConcat(m);
 }
 
-bool SkMatrix::postRotate(SkScalar degrees, SkScalar px, SkScalar py) {
+void SkMatrix::postRotate(SkScalar degrees, SkScalar px, SkScalar py) {
     SkMatrix    m;
     m.setRotate(degrees, px, py);
-    return this->postConcat(m);
+    this->postConcat(m);
 }
 
-bool SkMatrix::postRotate(SkScalar degrees) {
+void SkMatrix::postRotate(SkScalar degrees) {
     SkMatrix    m;
     m.setRotate(degrees);
-    return this->postConcat(m);
+    this->postConcat(m);
 }
 
 ////////////////////////////////////////////////////////////////////////////////////
 
 void SkMatrix::setSkew(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
     fMat[kMScaleX]  = 1;
     fMat[kMSkewX]   = sx;
     fMat[kMTransX]  = -sx * py;
 
     fMat[kMSkewY]   = sy;
@@ skipping 14 matching lines @@
     fMat[kMSkewY]   = sy;
     fMat[kMScaleY]  = 1;
     fMat[kMTransY]  = 0;
 
     fMat[kMPersp0] = fMat[kMPersp1] = 0;
     fMat[kMPersp2] = 1;
 
     this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
 }
 
-bool SkMatrix::preSkew(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
+void SkMatrix::preSkew(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
     SkMatrix    m;
     m.setSkew(sx, sy, px, py);
-    return this->preConcat(m);
+    this->preConcat(m);
 }
 
-bool SkMatrix::preSkew(SkScalar sx, SkScalar sy) {
+void SkMatrix::preSkew(SkScalar sx, SkScalar sy) {
     SkMatrix    m;
     m.setSkew(sx, sy);
-    return this->preConcat(m);
+    this->preConcat(m);
 }
 
-bool SkMatrix::postSkew(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
+void SkMatrix::postSkew(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
     SkMatrix    m;
     m.setSkew(sx, sy, px, py);
-    return this->postConcat(m);
+    this->postConcat(m);
 }
 
-bool SkMatrix::postSkew(SkScalar sx, SkScalar sy) {
+void SkMatrix::postSkew(SkScalar sx, SkScalar sy) {
     SkMatrix    m;
     m.setSkew(sx, sy);
-    return this->postConcat(m);
+    this->postConcat(m);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 bool SkMatrix::setRectToRect(const SkRect& src, const SkRect& dst,
                              ScaleToFit align)
 {
     if (src.isEmpty()) {
         this->reset();
         return false;
@@ skipping 54 matching lines @@
         }
         this->setTypeMask(mask);
     }
     // shared cleanup
     fMat[kMPersp2] = 1;
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
-static inline int fixmuladdmul(float a, float b, float c, float d,
-                               float* result) {
-    *result = SkDoubleToFloat((double)a * b + (double)c * d);
-    return true;
+static inline float muladdmul(float a, float b, float c, float d) {
+    return SkDoubleToFloat((double)a * b + (double)c * d);

[scroggo, 2014/04/02 14:53:36]

I know this is not a change in behavior, but is there a reason we decided not to
consider the possibility that these computations might overflow float?

 }
 
-static inline bool rowcol3(const float row[], const float col[],
-                           float* result) {
-    *result = row[0] * col[0] + row[1] * col[3] + row[2] * col[6];
-    return true;
-}
-
-static inline int negifaddoverflows(float& result, float a, float b) {
-    result = a + b;
-    return 0;
+static inline float rowcol3(const float row[], const float col[]) {
+    return row[0] * col[0] + row[1] * col[3] + row[2] * col[6];

[scroggo, 2014/04/02 14:53:36]

Same question.

 }
 
 static void normalize_perspective(SkScalar mat[9]) {
     if (SkScalarAbs(mat[SkMatrix::kMPersp2]) > 1) {
         for (int i = 0; i < 9; i++)
             mat[i] = SkScalarHalf(mat[i]);
     }
 }
 
-bool SkMatrix::setConcat(const SkMatrix& a, const SkMatrix& b) {
+void SkMatrix::setConcat(const SkMatrix& a, const SkMatrix& b) {
     TypeMask aType = a.getPerspectiveTypeMaskOnly();
     TypeMask bType = b.getPerspectiveTypeMaskOnly();
 
     if (a.isTriviallyIdentity()) {
         *this = b;
     } else if (b.isTriviallyIdentity()) {
         *this = a;
     } else {
         SkMatrix tmp;
 
         if ((aType | bType) & kPerspective_Mask) {
-            if (!rowcol3(&a.fMat[0], &b.fMat[0], &tmp.fMat[kMScaleX])) {
-                return false;
-            }
-            if (!rowcol3(&a.fMat[0], &b.fMat[1], &tmp.fMat[kMSkewX])) {
-                return false;
-            }
-            if (!rowcol3(&a.fMat[0], &b.fMat[2], &tmp.fMat[kMTransX])) {
-                return false;
-            }
-
-            if (!rowcol3(&a.fMat[3], &b.fMat[0], &tmp.fMat[kMSkewY])) {
-                return false;
-            }
-            if (!rowcol3(&a.fMat[3], &b.fMat[1], &tmp.fMat[kMScaleY])) {
-                return false;
-            }
-            if (!rowcol3(&a.fMat[3], &b.fMat[2], &tmp.fMat[kMTransY])) {
-                return false;
-            }
-
-            if (!rowcol3(&a.fMat[6], &b.fMat[0], &tmp.fMat[kMPersp0])) {
-                return false;
-            }
-            if (!rowcol3(&a.fMat[6], &b.fMat[1], &tmp.fMat[kMPersp1])) {
-                return false;
-            }
-            if (!rowcol3(&a.fMat[6], &b.fMat[2], &tmp.fMat[kMPersp2])) {
-                return false;
-            }
+            tmp.fMat[kMScaleX] = rowcol3(&a.fMat[0], &b.fMat[0]);
+            tmp.fMat[kMSkewX]  = rowcol3(&a.fMat[0], &b.fMat[1]);
+            tmp.fMat[kMTransX] = rowcol3(&a.fMat[0], &b.fMat[2]);
+            tmp.fMat[kMSkewY]  = rowcol3(&a.fMat[3], &b.fMat[0]);
+            tmp.fMat[kMScaleY] = rowcol3(&a.fMat[3], &b.fMat[1]);
+            tmp.fMat[kMTransY] = rowcol3(&a.fMat[3], &b.fMat[2]);
+            tmp.fMat[kMPersp0] = rowcol3(&a.fMat[6], &b.fMat[0]);
+            tmp.fMat[kMPersp1] = rowcol3(&a.fMat[6], &b.fMat[1]);
+            tmp.fMat[kMPersp2] = rowcol3(&a.fMat[6], &b.fMat[2]);
 
             normalize_perspective(tmp.fMat);
             tmp.setTypeMask(kUnknown_Mask);
         } else {    // not perspective
-            if (!fixmuladdmul(a.fMat[kMScaleX], b.fMat[kMScaleX],
-                    a.fMat[kMSkewX], b.fMat[kMSkewY], &tmp.fMat[kMScaleX])) {
-                return false;
-            }
-            if (!fixmuladdmul(a.fMat[kMScaleX], b.fMat[kMSkewX],
-                      a.fMat[kMSkewX], b.fMat[kMScaleY], &tmp.fMat[kMSkewX])) {
-                return false;
-            }
-            if (!fixmuladdmul(a.fMat[kMScaleX], b.fMat[kMTransX],
-                      a.fMat[kMSkewX], b.fMat[kMTransY], &tmp.fMat[kMTransX])) {
-                return false;
-            }
-            if (negifaddoverflows(tmp.fMat[kMTransX], tmp.fMat[kMTransX],
-                                  a.fMat[kMTransX]) < 0) {
-                return false;
-            }
+            tmp.fMat[kMScaleX] = muladdmul(a.fMat[kMScaleX],
+                                           b.fMat[kMScaleX],
+                                           a.fMat[kMSkewX],
+                                           b.fMat[kMSkewY]);
 
-            if (!fixmuladdmul(a.fMat[kMSkewY], b.fMat[kMScaleX],
-                      a.fMat[kMScaleY], b.fMat[kMSkewY], &tmp.fMat[kMSkewY])) {
-                return false;
-            }
-            if (!fixmuladdmul(a.fMat[kMSkewY], b.fMat[kMSkewX],
-                    a.fMat[kMScaleY], b.fMat[kMScaleY], &tmp.fMat[kMScaleY])) {
-                return false;
-            }
-            if (!fixmuladdmul(a.fMat[kMSkewY], b.fMat[kMTransX],
-                     a.fMat[kMScaleY], b.fMat[kMTransY], &tmp.fMat[kMTransY])) {
-                return false;
-            }
-            if (negifaddoverflows(tmp.fMat[kMTransY], tmp.fMat[kMTransY],
-                                  a.fMat[kMTransY]) < 0) {
-                return false;
-            }
+            tmp.fMat[kMSkewX]  = muladdmul(a.fMat[kMScaleX],
+                                           b.fMat[kMSkewX],
+                                           a.fMat[kMSkewX],
+                                           b.fMat[kMScaleY]);
 
+            tmp.fMat[kMTransX] = muladdmul(a.fMat[kMScaleX],
+                                           b.fMat[kMTransX],
+                                           a.fMat[kMSkewX],
+                                           b.fMat[kMTransY]);
+
+            tmp.fMat[kMTransX] += a.fMat[kMTransX];
+
+            tmp.fMat[kMSkewY]  = muladdmul(a.fMat[kMSkewY],
+                                           b.fMat[kMScaleX],
+                                           a.fMat[kMScaleY],
+                                           b.fMat[kMSkewY]);
+
+            tmp.fMat[kMScaleY] = muladdmul(a.fMat[kMSkewY],
+                                           b.fMat[kMSkewX],
+                                           a.fMat[kMScaleY],
+                                           b.fMat[kMScaleY]);
+
+            tmp.fMat[kMTransY] = muladdmul(a.fMat[kMSkewY],
+                                           b.fMat[kMTransX],
+                                           a.fMat[kMScaleY],
+                                           b.fMat[kMTransY]);
+
+            tmp.fMat[kMTransY] += a.fMat[kMTransY];
             tmp.fMat[kMPersp0] = tmp.fMat[kMPersp1] = 0;
             tmp.fMat[kMPersp2] = 1;
             //SkDebugf("Concat mat non-persp type: %d\n", tmp.getType());
             //SkASSERT(!(tmp.getType() & kPerspective_Mask));
             tmp.setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
         }
         *this = tmp;
     }
-    return true;
 }
 
-bool SkMatrix::preConcat(const SkMatrix& mat) {
+void SkMatrix::preConcat(const SkMatrix& mat) {
     // check for identity first, so we don't do a needless copy of ourselves
     // to ourselves inside setConcat()
-    return mat.isIdentity() || this->setConcat(*this, mat);
+    if(!mat.isIdentity()) {
+        this->setConcat(*this, mat);
+    }
 }
 
-bool SkMatrix::postConcat(const SkMatrix& mat) {
+void SkMatrix::postConcat(const SkMatrix& mat) {
     // check for identity first, so we don't do a needless copy of ourselves
     // to ourselves inside setConcat()
-    return mat.isIdentity() || this->setConcat(mat, *this);
+    if (!mat.isIdentity()) {
+        this->setConcat(mat, *this);
+    }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 /*  Matrix inversion is very expensive, but also the place where keeping
     precision may be most important (here and matrix concat). Hence to avoid
     bitmap blitting artifacts when walking the inverse, we use doubles for
     the intermediate math, even though we know that is more expensive.
  */
 
@@ skipping 736 matching lines @@
 
     if (!proc(src, &tempMap, scale)) {
         return false;
     }
     if (!tempMap.invert(&result)) {
         return false;
     }
     if (!proc(dst, &tempMap, scale)) {
         return false;
     }
-    if (!result.setConcat(tempMap, result)) {
-        return false;
-    }
-    *this = result;
+    this->setConcat(tempMap, result);
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 enum MinOrMax {
     kMin_MinOrMax,
     kMax_MinOrMax
 };
 
@@ skipping 269 matching lines @@
         rotation1->fX = cos1;
         rotation1->fY = sin1;
     }
     if (NULL != rotation2) {
         rotation2->fX = cos2;
         rotation2->fY = sin2;
     }
 
     return true;
 }