commit 386dfc049baa400c13e4e98727d4c04d0242b7b8
author vandebo@chromium.org <vandebo@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Tue Apr 17 22:31:52 2012 +0000
committer vandebo@chromium.org <vandebo@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Tue Apr 17 22:31:52 2012 +0000
tree b874dcd52dbce252abf9a1fdcea596c31e68cdcc
parent c39c8674c94dba8b1ffe938cd99c825320cc1475

[PDF] Handle failures of matrix inversion

Previously reviewed in https://codereview.appspot.com/6033047.  Rolled back
because of unrelated fixed-point bugs.

Review URL: https://codereview.appspot.com/6052051

git-svn-id: http://skia.googlecode.com/svn/trunk@3715 2bbb7eff-a529-9590-31e7-b0007b416f81

src/pdf/SkPDFDevice.cpp

diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 7e85afd..733a612 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -491,6 +491,8 @@
         drawingSize.set(SkIntToScalar(contentSize.fWidth),
                         SkIntToScalar(contentSize.fHeight));
         if (!initialTransform->invert(&inverse)) {
+            // This shouldn't happen, initial transform should be invertible.
+            SkASSERT(false);
             inverse.reset();
         }
         inverse.mapVectors(&drawingSize, 1);
@@ -603,7 +605,7 @@
     totalTransform.preConcat(contentEntry->fState.fMatrix);
     SkMatrix inverse;
     if (!totalTransform.invert(&inverse)) {
-        inverse.reset();
+        return;
     }
     inverse.mapRect(&bbox);
 

src/pdf/SkPDFFormXObject.cpp

diff --git a/src/pdf/SkPDFFormXObject.cpp b/src/pdf/SkPDFFormXObject.cpp
index f368834..c32ea44 100644
--- a/src/pdf/SkPDFFormXObject.cpp
+++ b/src/pdf/SkPDFFormXObject.cpp
@@ -37,6 +37,8 @@
     if (!device->initialTransform().isIdentity()) {
         SkMatrix inverse;
         if (!device->initialTransform().invert(&inverse)) {
+            // The initial transform should be invertible.
+            SkASSERT(false);
             inverse.reset();
         }
         insert("Matrix", SkPDFUtils::MatrixToArray(inverse))->unref();

src/pdf/SkPDFShader.cpp

diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp
index 2c3bfd0..c3c3cd0 100644
--- a/src/pdf/SkPDFShader.cpp
+++ b/src/pdf/SkPDFShader.cpp
@@ -21,12 +21,13 @@
 #include "SkThread.h"
 #include "SkTypes.h"
 
-static void transformBBox(const SkMatrix& matrix, SkRect* bbox) {
+static bool transformBBox(const SkMatrix& matrix, SkRect* bbox) {
     SkMatrix inverse;
     if (!matrix.invert(&inverse)) {
-        inverse.reset();
+        return false;
     }
     inverse.mapRect(bbox);
+    return true;
 }
 
 static void unitToPointsMatrix(const SkPoint pts[2], SkMatrix* matrix) {
@@ -309,7 +310,7 @@
         fResources.unrefAll();
     }
 
-    bool isValid() { return fResources.count() > 0; }
+    virtual bool isValid() { return fResources.count() > 0; }
 
     void getResources(SkTDArray<SkPDFObject*>* resourceList) {
         GetResourcesHelper(&fResources, resourceList);
@@ -332,6 +333,8 @@
         fResources.unrefAll();
     }
 
+    virtual bool isValid() { return size() > 0; }
+
     void getResources(SkTDArray<SkPDFObject*>* resourceList) {
         GetResourcesHelper(&fResources, resourceList);
     }
@@ -367,18 +370,24 @@
         result->ref();
         return result;
     }
+
+    bool valid = false;
     // The PDFShader takes ownership of the shaderSate.
     if (shaderState.get()->fType == SkShader::kNone_GradientType) {
-        result = new SkPDFImageShader(shaderState.detach());
+        SkPDFImageShader* imageShader =
+            new SkPDFImageShader(shaderState.detach());
+        valid = imageShader->isValid();
+        result = imageShader;
     } else {
         SkPDFFunctionShader* functionShader =
             new SkPDFFunctionShader(shaderState.detach());
-        if (!functionShader->isValid()) {
-            delete functionShader;
-            return NULL;
-        }
+        valid = functionShader->isValid();
         result = functionShader;
     }
+    if (!valid) {
+        delete result;
+        return NULL;
+    }
     entry.fPDFShader = result;
     CanonicalShaders().push(entry);
     return result;  // return the reference that came from new.
@@ -472,7 +481,9 @@
     finalMatrix.preConcat(fState.get()->fShaderTransform);
     SkRect bbox;
     bbox.set(fState.get()->fBBox);
-    transformBBox(finalMatrix, &bbox);
+    if (!transformBBox(finalMatrix, &bbox)) {
+        return;
+    }
 
     SkRefPtr<SkPDFArray> domain = new SkPDFArray;
     domain->unref();  // SkRefPtr and new both took a reference.
@@ -490,7 +501,7 @@
         SkShader::GradientInfo twoPointRadialInfo = *info;
         SkMatrix inverseMapperMatrix;
         if (!mapperMatrix.invert(&inverseMapperMatrix)) {
-            inverseMapperMatrix.reset();
+            return;
         }
         inverseMapperMatrix.mapPoints(twoPointRadialInfo.fPoint, 2);
         twoPointRadialInfo.fRadius[0] =
@@ -525,7 +536,9 @@
     finalMatrix.preConcat(fState.get()->fShaderTransform);
     SkRect surfaceBBox;
     surfaceBBox.set(fState.get()->fBBox);
-    transformBBox(finalMatrix, &surfaceBBox);
+    if (!transformBBox(finalMatrix, &surfaceBBox)) {
+        return;
+    }
 
     SkMatrix unflip;
     unflip.setTranslate(0, SkScalarRoundToScalar(surfaceBBox.height()));

src/pdf/SkPDFShader.h

diff --git a/src/pdf/SkPDFShader.h b/src/pdf/SkPDFShader.h
index 439d83b..afa63e7 100644
--- a/src/pdf/SkPDFShader.h
+++ b/src/pdf/SkPDFShader.h
@@ -60,6 +60,8 @@
     static void RemoveShader(SkPDFObject* shader);
 
     SkPDFShader();
+
+    virtual bool isValid() = 0;
 };
 
 #endif