From b88cfe58e117ffe781e4ce2cba73cc4f7a795de7 Mon Sep 17 00:00:00 2001 From: "vandebo@chromium.org" Date: Mon, 18 Jul 2011 18:40:32 +0000 Subject: [PATCH] [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 --- include/pdf/SkPDFShader.h | 4 ++-- src/pdf/SkPDFDevice.cpp | 29 ++++++++++++++--------------- src/pdf/SkPDFShader.cpp | 25 +++++++++++++------------ 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/include/pdf/SkPDFShader.h b/include/pdf/SkPDFShader.h index 17f7f03316..0d7f685a63 100644 --- a/include/pdf/SkPDFShader.h +++ b/include/pdf/SkPDFShader.h @@ -95,8 +95,8 @@ private: } }; // This should be made a hash table if performance is a problem. - static SkTDArray& canonicalShaders(); - static SkMutex& canonicalShadersMutex(); + static SkTDArray& CanonicalShaders(); + static SkMutex& CanonicalShadersMutex(); static SkPDFObject* rangeObject(); diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp index 75782d2bfd..81cfdaa094 100644 --- a/src/pdf/SkPDFDevice.cpp +++ b/src/pdf/SkPDFDevice.cpp @@ -1409,9 +1409,20 @@ void SkPDFDevice::populateGraphicStateEntryFromPaint( 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 @@ void SkPDFDevice::populateGraphicStateEntryFromPaint( 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); diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp index 1f58f1fae3..92b0429f90 100644 --- a/src/pdf/SkPDFShader.cpp +++ b/src/pdf/SkPDFShader.cpp @@ -287,11 +287,13 @@ static SkString sweepCode(const SkShader::GradientInfo& info) { } 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 @@ SkPDFShader* SkPDFShader::getPDFShader(const SkShader& shader, const SkMatrix& matrix, const SkIRect& surfaceBBox) { SkRefPtr pdfShader; - SkAutoMutexAcquire lock(canonicalShadersMutex()); + SkAutoMutexAcquire lock(CanonicalShadersMutex()); SkAutoTDelete 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 @@ SkPDFShader* SkPDFShader::getPDFShader(const SkShader& shader, 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::canonicalShaders() { +SkTDArray& SkPDFShader::CanonicalShaders() { // This initialization is only thread safe with gcc. static SkTDArray 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 @@ SkMutex& SkPDFShader::canonicalShadersMutex() { 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 @@ void SkPDFShader::doFunctionShader() { break; case SkShader::kColor_GradientType: case SkShader::kNone_GradientType: - SkASSERT(false); return; }