From 86edf7332417d0d7c80123f074f3e12a9128776d Mon Sep 17 00:00:00 2001
From: "vandebo@chromium.org"
 <vandebo@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Date: Tue, 17 Apr 2012 22:31:52 +0000
Subject: [PATCH] [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/src@3715 2bbb7eff-a529-9590-31e7-b0007b416f81
---
 pdf/SkPDFDevice.cpp      |  4 +++-
 pdf/SkPDFFormXObject.cpp |  2 ++
 pdf/SkPDFShader.cpp      | 35 ++++++++++++++++++++++++-----------
 pdf/SkPDFShader.h        |  2 ++
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/pdf/SkPDFDevice.cpp b/pdf/SkPDFDevice.cpp
index 7e85afd..733a612 100644
--- a/pdf/SkPDFDevice.cpp
+++ b/pdf/SkPDFDevice.cpp
@@ -491,6 +491,8 @@ static inline SkBitmap makeContentBitmap(const SkISize& contentSize,
         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 @@ void SkPDFDevice::internalDrawPaint(const SkPaint& paint,
     totalTransform.preConcat(contentEntry->fState.fMatrix);
     SkMatrix inverse;
     if (!totalTransform.invert(&inverse)) {
-        inverse.reset();
+        return;
     }
     inverse.mapRect(&bbox);
 
diff --git a/pdf/SkPDFFormXObject.cpp b/pdf/SkPDFFormXObject.cpp
index f368834..c32ea44 100644
--- a/pdf/SkPDFFormXObject.cpp
+++ b/pdf/SkPDFFormXObject.cpp
@@ -37,6 +37,8 @@ SkPDFFormXObject::SkPDFFormXObject(SkPDFDevice* device) {
     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();
diff --git a/pdf/SkPDFShader.cpp b/pdf/SkPDFShader.cpp
index 2c3bfd0..c3c3cd0 100644
--- a/pdf/SkPDFShader.cpp
+++ b/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 @@ class SkPDFFunctionShader : public SkPDFDict, public SkPDFShader {
         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 @@ class SkPDFImageShader : public SkPDFStream, public SkPDFShader {
         fResources.unrefAll();
     }
 
+    virtual bool isValid() { return size() > 0; }
+
     void getResources(SkTDArray<SkPDFObject*>* resourceList) {
         GetResourcesHelper(&fResources, resourceList);
     }
@@ -367,18 +370,24 @@ SkPDFObject* SkPDFShader::GetPDFShader(const SkShader& shader,
         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 @@ SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
     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 @@ SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
         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 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
     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()));
diff --git a/pdf/SkPDFShader.h b/pdf/SkPDFShader.h
index 439d83b..afa63e7 100644
--- a/pdf/SkPDFShader.h
+++ b/pdf/SkPDFShader.h
@@ -60,6 +60,8 @@ class SkPDFShader {
     static void RemoveShader(SkPDFObject* shader);
 
     SkPDFShader();
+
+    virtual bool isValid() = 0;
 };
 
 #endif