Implemented transparent gradients

src/pdf/SkPDFShader.cpp

Index: src/pdf/SkPDFShader.cpp
diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp
index a0dffb7791fc4724ecc5a16403fc4b4b2efa6b58..e41d07650aeaf24e4d05facf4231f73f876cdc00 100644
--- a/src/pdf/SkPDFShader.cpp
+++ b/src/pdf/SkPDFShader.cpp
@@ -13,12 +13,15 @@
 #include "SkData.h"
 #include "SkPDFCatalog.h"
 #include "SkPDFDevice.h"
+#include "SkPDFFormXObject.h"
+#include "SkPDFGraphicState.h"
 #include "SkPDFTypes.h"
 #include "SkPDFUtils.h"
 #include "SkScalar.h"
 #include "SkStream.h"
 #include "SkTemplates.h"
 #include "SkThread.h"
+#include "SkTSet.h"
 #include "SkTypes.h"
 
 static bool transformBBox(const SkMatrix& matrix, SkRect* bbox) {
@@ -398,7 +401,7 @@ class SkPDFShader::State {
 public:
     SkShader::GradientType fType;
     SkShader::GradientInfo fInfo;
-    SkAutoFree fColorData;
+    SkAutoFree fColorData;    // This provides storage for arrays in fInfo.
     SkMatrix fCanvasTransform;
     SkMatrix fShaderTransform;
     SkIRect fBBox;
@@ -409,7 +412,17 @@ public:
 
     explicit State(const SkShader& shader, const SkMatrix& canvasTransform,

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

nit: remove explicit

[ducky, 2013/07/09 02:56:23]

Done.

                    const SkIRect& bbox);
+
     bool operator==(const State& b) const;
+
+    SkPDFShader::State* CreateAlphaToLuminosityState() const;
+    SkPDFShader::State* CreateOpaqueState() const;
+
+    bool GradientHasAlpha() const;
+
+private:
+    State(const State& other);

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

The Rule of three says you should also declare an assignment operator method. 
It can be private and not defined.

[ducky, 2013/07/09 02:56:23]

Done.

+    void AllocateGradientInfoStorage();
 };
 
 class SkPDFFunctionShader : public SkPDFDict, public SkPDFShader {
@@ -431,6 +444,8 @@ public:
                            newResourceObjects);
     }
 
+    SkPDFFunctionShader* CreateAlphaToLuminosityShader();

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

unused

[ducky, 2013/07/09 02:56:23]

Done.

+
 private:
     static SkPDFObject* RangeObject();
 
@@ -440,6 +455,39 @@ private:
     SkPDFStream* makePSFunction(const SkString& psCode, SkPDFArray* domain);
 };
 
+/**
+ * A shader for PDF gradients. This encapsulates the function shader
+ * inside a tiling pattern while providing a common pattern interface.
+ * The encapsulation allows the use of a SMask for transparency gradients.
+ */
+class SkPDFGradientShader : public SkPDFStream, public SkPDFShader {

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

nit: There's probably a better name for this... FunctionShader is also a
gradient shader. Not sure what the better name is, but it probably includes
alpha...?

[ducky, 2013/07/09 02:56:23]

There probably is a better name. SkPDFAlphaGradientShader? That completely
describes it, though it's a bit wordy...

[vandebo (ex-Chrome), 2013/07/09 17:47:44]

SkPDFFunctionShaderWithAlpha ?

[ducky, 2013/07/10 21:42:26]

Also pretty long... how about SkPDFAlphaFunctionShader?

[vandebo (ex-Chrome), 2013/07/11 22:22:23]

SGTM

+public:
+    explicit SkPDFGradientShader(SkPDFShader::State* state);
+    virtual ~SkPDFGradientShader() {
+        if (isValid()) {
+            RemoveShader(this);
+        }
+    }
+
+    virtual bool isValid() {
+        return fColorPattern.get() != NULL;
+    }
+
+private:
+    SkAutoTDelete<const SkPDFShader::State> fState;
+
+    void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                      SkTSet<SkPDFObject*>* newResourceObjects) {
+        fResourceDict->getResources(knownResourceObjects,
+                                    newResourceObjects);
+    }
+
+    SkAutoTUnref<SkPDFObject> fColorPattern;
+    SkAutoTUnref<SkPDFGraphicState> fAlphaGs;
+
+    SkAutoTUnref<SkPDFResourceDict> fResourceDict;
+};
+
 class SkPDFImageShader : public SkPDFStream, public SkPDFShader {
 public:
     explicit SkPDFImageShader(SkPDFShader::State* state);
@@ -477,9 +525,16 @@ void SkPDFShader::RemoveShader(SkPDFObject* shader) {
 SkPDFObject* SkPDFShader::GetPDFShader(const SkShader& shader,
                                        const SkMatrix& matrix,
                                        const SkIRect& surfaceBBox) {
-    SkPDFObject* result;
     SkAutoMutexAcquire lock(CanonicalShadersMutex());
-    SkAutoTDelete<State> shaderState(new State(shader, matrix, surfaceBBox));
+    return GetPDFShaderByState(new State(shader, matrix, surfaceBBox));
+}
+
+// static
+// This is an internal method. CanonicalShadersMutex() should already be acquired.

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

Also comment that this method takes ownership of |inState|.

[ducky, 2013/07/09 02:56:23]

Done.

+SkPDFObject* SkPDFShader::GetPDFShaderByState(State* inState) {
+    SkPDFObject* result;
+
+    SkAutoTDelete<State> shaderState(inState);
     if (shaderState.get()->fType == SkShader::kNone_GradientType &&
             shaderState.get()->fImage.isNull()) {
         // TODO(vandebo) This drops SKComposeShader on the floor.  We could
@@ -505,10 +560,17 @@ SkPDFObject* SkPDFShader::GetPDFShader(const SkShader& shader,
         valid = imageShader->isValid();
         result = imageShader;
     } else {
-        SkPDFFunctionShader* functionShader =
-            new SkPDFFunctionShader(shaderState.detach());
-        valid = functionShader->isValid();
-        result = functionShader;
+        if (shaderState.get()->GradientHasAlpha()) {
+            SkPDFGradientShader* gradientShader =
+                new SkPDFGradientShader(shaderState.detach());
+            valid = gradientShader->isValid();
+            result = gradientShader;
+        } else {
+            SkPDFFunctionShader* functionShader =
+                new SkPDFFunctionShader(shaderState.detach());
+            valid = functionShader->isValid();
+            result = functionShader;
+        }
     }
     if (!valid) {
         delete result;
@@ -553,6 +615,94 @@ SkPDFObject* SkPDFFunctionShader::RangeObject() {
     return range;
 }
 
+static SkPDFResourceDict* getGradientResourceDict(

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

static helper functions are named with underscores and no capitals.
nit: s/get/new/

[ducky, 2013/07/09 02:56:23]

Done. Though the functions generating postscript code on top don't conform to
this...

[vandebo (ex-Chrome), 2013/07/09 17:47:44]

That was before the style guide was codified.  You may fix those functions if
you like; though it would be best to do it in a different CL.

+        SkPDFObject* functionShader,
+        SkPDFObject* gState) {
+    SkPDFResourceDict* dict = new SkPDFResourceDict();
+
+    if (functionShader != NULL) {
+        dict->insertResourceAsRef("Pattern", "P0", functionShader);
+    }
+    if (gState != NULL) {
+        dict->insertResourceAsRef("ExtGState", "G0", gState);
+    }
+
+    dict->insert("ProcSet", SkPDFUtils::CreateFullProcSetsArray())->unref();

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

Is this needed, I wasn't sure if it was.  If it is, the full proc set certainly
isn't needed; no text or images used here.

[ducky, 2013/07/09 02:56:23]

ProcSet is no longer used in current versions of the PDF spec. However, the spec
still recommends adding it only for backwards compatibility. I added the full
set since it was how it was done in the only other instance ProcSet was defined.
It's definitely overkill, but is it worth optimizing for something that
shouldn't actually be used?

[vandebo (ex-Chrome), 2013/07/09 17:47:44]

Well, it's optimizing for size.  Though I guess the number of copies of this is
small enough that it doesn't matter.

+
+    return dict;
+}
+
+static void populateTilingPatternDict(SkPDFDict* pattern,

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

Here too and below...

[ducky, 2013/07/09 02:56:23]

Done.

+                                      SkRect& bbox, SkPDFDict* resources,
+                                      const SkMatrix& matrix) {
+    pattern->insertName("Type", "Pattern");
+    pattern->insertInt("PatternType", 1);
+    pattern->insertInt("PaintType", 1);
+    pattern->insertInt("TilingType", 1);
+    pattern->insert("BBox", SkPDFUtils::RectToArray(bbox))->unref();
+    pattern->insertScalar("XStep", bbox.width());
+    pattern->insertScalar("YStep", bbox.height());
+    pattern->insert("Resources", resources);
+    if (!matrix.isIdentity()) {
+        pattern->insert("Matrix", SkPDFUtils::MatrixToArray(matrix))->unref();
+    }
+}
+
+/**
+ * Creates a content stream which fills the pattern P0 across bounds.
+ * If setGs0 is true, this additionally sets the graphics state to G0.
+ */
+static SkData* createPatternFillContent(bool setGs0, SkRect& bounds) {

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

setGs0 feels odd.  Maybe pass a SkString stream_setup ?

[ducky, 2013/07/09 02:56:23]

Done. Yeah, that was ugly.

+    SkDynamicMemoryWStream content;
+    if (setGs0) {
+        content.writeText("/G0 gs");
+    }
+    content.writeText("/Pattern CS/Pattern cs/P0 SCN/P0 scn\n");
+
+    SkPDFUtils::AppendRectangle(bounds, &content);
+    SkPDFUtils::PaintPath(SkPaint::kFill_Style, SkPath::kEvenOdd_FillType, &content);
+
+    return content.copyToData();
+}
+
+/**
+ * Creates a ExtGState with the SMask set to the luminosityShader in
+ * luminosity mode. The shader pattern extends to the bbox.
+ */
+static SkPDFGraphicState* createSMaskGraphicState(SkPDFObject* luminosityShader, SkRect& bbox) {

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

I think if this function takes ownership of |luminosityShader| the caller will
be a bit cleaner.

[ducky, 2013/07/09 02:56:23]

But doesn't that just shift the unref code from the caller to inside this
function? Could it also be more confusing - in that unlike most other functions,
this one will unref one of its arguments? I also kind of like having the
creation and destruction in the same scope (where applicable).

[vandebo (ex-Chrome), 2013/07/09 17:47:44]

Fair. What about making it a member of the class and passing in state instead?

[ducky, 2013/07/10 21:42:26]

That also seems a bit inelegant, where you have one helper function pulled into
the class, while the rest are static (since there's little reason to pull those
in). Additionally, it just means moving that one line of SkAutoTUnref code into
the function, so I don't see what we get? Furthermore, we have both shaders
being constructed in the SkPDFGradientShader constructor, so it's kind of nice
from a consistency standpoint.

[vandebo (ex-Chrome), 2013/07/11 22:22:23]

You're only thinking of it as a helper function because that's where it is now. 
By doing it, you get better encapsulation of the functionality of this method.

[ducky, 2013/07/12 03:39:59]

Ah, I see. Done.

On 2013/07/11 22:22:23, vandebo wrote:

+    SkAutoTUnref<SkPDFResourceDict>

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

Move this to line 679. - Generally, define variables just before they are used.

[ducky, 2013/07/09 02:56:23]

Done.

+        resources(getGradientResourceDict(luminosityShader, NULL));
+    SkTSet<SkPDFObject*> resourcesArray;

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

This isn't used...

[ducky, 2013/07/09 02:56:23]

Done.

+    resourcesArray.add(luminosityShader);
+
+    SkAutoTUnref<SkData> alphaData(createPatternFillContent(false, bbox));

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

Event though this is of type SkData, I would call it alphaStream because it's
the alpha content stream.

[ducky, 2013/07/09 02:56:23]

Done, and similarly for colorStream below.

+
+    SkAutoTUnref<SkPDFFormXObject> alphaMask(
+            new SkPDFFormXObject(alphaData.get(), bbox, resources.get()));
+
+    return SkPDFGraphicState::GetSMaskGraphicState(
+            alphaMask.get(), false,
+            SkPDFGraphicState::kLuminosity_SMaskMode);
+}
+
+SkPDFGradientShader::SkPDFGradientShader(SkPDFShader::State* state) : fState(state) {
+    SkRect bbox;
+    bbox.set(fState.get()->fBBox);
+
+    fColorPattern.reset(SkPDFShader::GetPDFShaderByState(state->CreateOpaqueState()));
+
+    SkAutoTUnref<SkPDFObject> luminosityShader(SkPDFShader::GetPDFShaderByState(
+                 state->CreateAlphaToLuminosityState()));
+    fAlphaGs.reset(createSMaskGraphicState(luminosityShader.get(), bbox));
+
+    fResourceDict.reset(getGradientResourceDict(fColorPattern.get(), fAlphaGs.get()));
+
+    SkAutoTUnref<SkData> colorData(createPatternFillContent(true, bbox));
+    setData(colorData.get());

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

Can you combine lines 700 and 701?

[ducky, 2013/07/09 02:56:23]

Not unless the definition for setData is changed to return the SkData* passed
in, in which case I could chain it with a unref.
Incidentally, changing setData like that will also clean up some of the code for
the SkPDFStream change CL.
Otherwise, I can't think of a way to simultaneously set the data and unref the
object after the data is set.

[vandebo (ex-Chrome), 2013/07/09 17:47:44]

Fair.
Having setData return the new data is a bit strange (if it returned the old
data, that might make some sense, but isn't useful). Lets leave it as it is.

[ducky, 2013/07/10 21:42:26]

setData could return its input parameter (which incidentally is the new data, at
least after the refactor CL). This seems to be the convention with
SkPDFDict::insert, which returns the value being inserted, and is sometimes
chained with unref()

[vandebo (ex-Chrome), 2013/07/11 22:22:23]

If you like.

[ducky, 2013/07/12 03:39:59]

Actually, I think I'm going to leave this as-is.
If it becomes worthwhile later, it probably should get its own refactor CL,
where things become consistent across the board.

+
+    populateTilingPatternDict(this, bbox, fResourceDict.get(), SkMatrix::I());
+}
+
 SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
         : SkPDFDict("Pattern"),
           fState(state) {
@@ -828,27 +978,13 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
         }
     }
 
-    SkAutoTUnref<SkPDFArray> patternBBoxArray(new SkPDFArray);
-    patternBBoxArray->reserve(4);
-    patternBBoxArray->appendScalar(patternBBox.fLeft);
-    patternBBoxArray->appendScalar(patternBBox.fTop);
-    patternBBoxArray->appendScalar(patternBBox.fRight);
-    patternBBoxArray->appendScalar(patternBBox.fBottom);
-
     // Put the canvas into the pattern stream (fContent).
     SkAutoTUnref<SkStream> content(pattern.content());
     setData(content.get());
     pattern.getResources(fResources, &fResources, false);
 
-    insertName("Type", "Pattern");
-    insertInt("PatternType", 1);
-    insertInt("PaintType", 1);
-    insertInt("TilingType", 1);
-    insert("BBox", patternBBoxArray.get());
-    insertScalar("XStep", patternBBox.width());
-    insertScalar("YStep", patternBBox.height());
-    insert("Resources", pattern.getResourceDict());
-    insert("Matrix", SkPDFUtils::MatrixToArray(finalMatrix))->unref();
+    populateTilingPatternDict(this, patternBBox,
+                              pattern.getResourceDict(), finalMatrix);
 
     fState.get()->fImage.unlockPixels();
 }
@@ -954,11 +1090,87 @@ SkPDFShader::State::State(const SkShader& shader,
         SkASSERT(matrix.isIdentity());
         fPixelGeneration = fImage.getGenerationID();
     } else {
-        fColorData.set(sk_malloc_throw(
-                    fInfo.fColorCount * (sizeof(SkColor) + sizeof(SkScalar))));
-        fInfo.fColors = reinterpret_cast<SkColor*>(fColorData.get());
-        fInfo.fColorOffsets =
-            reinterpret_cast<SkScalar*>(fInfo.fColors + fInfo.fColorCount);
+        AllocateGradientInfoStorage();
         shader.asAGradient(&fInfo);
     }
 }
+
+SkPDFShader::State::State(const SkPDFShader::State& other)
+  : fType(other.fType),
+    fCanvasTransform(other.fCanvasTransform),
+    fShaderTransform(other.fShaderTransform),
+    fBBox(other.fBBox)
+{
+    // Only gradients supported for now, since that is all that is used.
+    // If needed, image state copy constructor can be added here later.
+    SkASSERT(fType != SkShader::kNone_GradientType);
+
+    if (fType != SkShader::kNone_GradientType) {
+        fInfo = other.fInfo;
+
+        AllocateGradientInfoStorage();
+        for (int i = 0; i < fInfo.fColorCount; i++) {
+            fInfo.fColors[i] = other.fInfo.fColors[i];
+            fInfo.fColorOffsets[i] = other.fInfo.fColorOffsets[i];
+        }
+    }
+}
+
+/**
+ * Create a copy of this gradient state with alpha assigned to RGB luminousity.
+ * Only valid for gradient states!

[vandebo (ex-Chrome), 2013/07/08 19:02:25]

nit: ! not needed, a . will suffice.

[ducky, 2013/07/09 02:56:23]

Done.

[vandebo (ex-Chrome), 2013/07/09 17:47:44]

Not done.

[ducky, 2013/07/10 21:42:26]

No, it is done. I think you're looking at an outdated version. Should be fixed
as of Patch Set 5.

[vandebo (ex-Chrome), 2013/07/11 22:22:23]

In patch set 5 (and 7) line 1120 (1123) has a !

[ducky, 2013/07/12 03:39:59]

I see. There were two. D'oh.

+ */
+SkPDFShader::State* SkPDFShader::State::CreateAlphaToLuminosityState() const {
+    SkASSERT(fType != SkShader::kNone_GradientType);
+
+    SkPDFShader::State* newState = new SkPDFShader::State(*this);
+
+    for (int i = 0; i < fInfo.fColorCount; i++) {
+        SkAlpha alpha = SkColorGetA(fInfo.fColors[i]);
+        newState->fInfo.fColors[i] = SkColorSetARGB(255, alpha, alpha, alpha);
+    }
+
+    return newState;
+}
+
+/**
+ * Create a copy of this gradient state with alpha set to fully opaque
+ * Only valid for gradient states!
+ */
+SkPDFShader::State* SkPDFShader::State::CreateOpaqueState() const {
+    SkASSERT(fType != SkShader::kNone_GradientType);
+
+    SkPDFShader::State* newState = new SkPDFShader::State(*this);
+    SkAlpha opaque = SkColorGetA(SK_ColorBLACK);
+    for (int i = 0; i < fInfo.fColorCount; i++) {
+        newState->fInfo.fColors[i] = SkColorSetA(fInfo.fColors[i], opaque);
+    }
+
+    return newState;
+}
+
+/**
+ * Returns true if state is a gradient and the gradient has alpha.
+ */
+bool SkPDFShader::State::GradientHasAlpha() const {
+    if (fType == SkShader::kNone_GradientType) {
+        return false;
+    }
+
+    SkAlpha opaque = SkColorGetA(SK_ColorBLACK);
+    for (int i = 0; i < fInfo.fColorCount; i++) {
+        SkAlpha alpha = SkColorGetA(fInfo.fColors[i]);
+        if (alpha != opaque) {
+            return true;
+        }
+    }
+    return false;
+}
+
+void SkPDFShader::State::AllocateGradientInfoStorage() {
+    fColorData.set(sk_malloc_throw(
+               fInfo.fColorCount * (sizeof(SkColor) + sizeof(SkScalar))));
+    fInfo.fColors = reinterpret_cast<SkColor*>(fColorData.get());
+    fInfo.fColorOffsets =
+                    reinterpret_cast<SkScalar*>(fInfo.fColors + fInfo.fColorCount);
+}