commit b88cfe58e117ffe781e4ce2cba73cc4f7a795de7
author vandebo@chromium.org <vandebo@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Mon Jul 18 18:40:32 2011 +0000
committer vandebo@chromium.org <vandebo@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Mon Jul 18 18:40:32 2011 +0000
tree 5aa8cdd2417c77f2d50bd93699d2214a21da31db
parent f204d68c39bfcaaef892bd568d18de2d4ee662d2

[PDF] Make color shaders work correctly.

Make SkPDFShader correctly bail out for color shaders.
Fix the bail out handling code.

Review URL: http://codereview.appspot.com/4750045

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

src/pdf/SkPDFDevice.cpp

diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 75782d2..81cfdaa 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -1409,9 +1409,20 @@
         pdfShader = SkPDFShader::getPDFShader(*shader, transform, bounds);
         SkSafeUnref(pdfShader.get());  // getShader and SkRefPtr both took a ref
 
-        // A color shader is treated as an invalid shader so we don't have
-        // to set a shader just for a color.
-        if (pdfShader.get() == NULL) {
+        if (pdfShader.get()) {
+            // pdfShader has been canonicalized so we can directly compare
+            // pointers.
+            int resourceIndex = fShaderResources.find(pdfShader.get());
+            if (resourceIndex < 0) {
+                resourceIndex = fShaderResources.count();
+                fShaderResources.push(pdfShader.get());
+                pdfShader->ref();
+            }
+            entry->fShaderIndex = resourceIndex;
+        } else {
+            // A color shader is treated as an invalid shader so we don't have
+            // to set a shader just for a color.
+            entry->fShaderIndex = -1;
             entry->fColor = 0;
             color = 0;
 
@@ -1427,18 +1438,6 @@
                 color = gradientColor;
             }
         }
-    }
-
-    if (pdfShader) {
-        // pdfShader has been canonicalized so we can directly compare
-        // pointers.
-        int resourceIndex = fShaderResources.find(pdfShader.get());
-        if (resourceIndex < 0) {
-            resourceIndex = fShaderResources.count();
-            fShaderResources.push(pdfShader.get());
-            pdfShader->ref();
-        }
-        entry->fShaderIndex = resourceIndex;
     } else {
         entry->fShaderIndex = -1;
         entry->fColor = SkColorSetA(paint.getColor(), 0xFF);

src/pdf/SkPDFShader.cpp

diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp
index 1f58f1f..92b0429 100644
--- a/src/pdf/SkPDFShader.cpp
+++ b/src/pdf/SkPDFShader.cpp
@@ -287,11 +287,13 @@
 }
 
 SkPDFShader::~SkPDFShader() {
-    SkAutoMutexAcquire lock(canonicalShadersMutex());
+    SkAutoMutexAcquire lock(CanonicalShadersMutex());
     ShaderCanonicalEntry entry(this, fState.get());
-    int index = canonicalShaders().find(entry);
-    SkASSERT(index >= 0);
-    canonicalShaders().removeShuffle(index);
+    int index = CanonicalShaders().find(entry);
+    if (fContent.get()) {
+        SkASSERT(index >= 0);
+        CanonicalShaders().removeShuffle(index);
+    }
     fResources.unrefAll();
 }
 
@@ -323,13 +325,13 @@
                                        const SkMatrix& matrix,
                                        const SkIRect& surfaceBBox) {
     SkRefPtr<SkPDFShader> pdfShader;
-    SkAutoMutexAcquire lock(canonicalShadersMutex());
+    SkAutoMutexAcquire lock(CanonicalShadersMutex());
     SkAutoTDelete<State> shaderState(new State(shader, matrix, surfaceBBox));
 
     ShaderCanonicalEntry entry(NULL, shaderState.get());
-    int index = canonicalShaders().find(entry);
+    int index = CanonicalShaders().find(entry);
     if (index >= 0) {
-        SkPDFShader* result = canonicalShaders()[index].fPDFShader;
+        SkPDFShader* result = CanonicalShaders()[index].fPDFShader;
         result->ref();
         return result;
     }
@@ -341,19 +343,19 @@
         return NULL;
     }
     entry.fPDFShader = pdfShader.get();
-    canonicalShaders().push(entry);
+    CanonicalShaders().push(entry);
     return pdfShader.get();  // return the reference that came from new.
 }
 
 // static
-SkTDArray<SkPDFShader::ShaderCanonicalEntry>& SkPDFShader::canonicalShaders() {
+SkTDArray<SkPDFShader::ShaderCanonicalEntry>& SkPDFShader::CanonicalShaders() {
     // This initialization is only thread safe with gcc.
     static SkTDArray<ShaderCanonicalEntry> gCanonicalShaders;
     return gCanonicalShaders;
 }
 
 // static
-SkMutex& SkPDFShader::canonicalShadersMutex() {
+SkMutex& SkPDFShader::CanonicalShadersMutex() {
     // This initialization is only thread safe with gcc.
     static SkMutex gCanonicalShadersMutex;
     return gCanonicalShadersMutex;
@@ -363,7 +365,7 @@
 SkPDFObject* SkPDFShader::rangeObject() {
     // This initialization is only thread safe with gcc.
     static SkPDFArray* range = NULL;
-    // This method is only used with canonicalShadersMutex, so it's safe to
+    // This method is only used with CanonicalShadersMutex, so it's safe to
     // populate domain.
     if (range == NULL) {
         range = new SkPDFArray;
@@ -424,7 +426,6 @@
             break;
         case SkShader::kColor_GradientType:
         case SkShader::kNone_GradientType:
-            SkASSERT(false);
             return;
     }