Inverse fill support in PDF

src/pdf/SkPDFDevice.cpp

Index: src/pdf/SkPDFDevice.cpp
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index f7077a949b9921e7a8ac9835f53710bb69b9c0c4..dfbddec1b2f02c69839ba25223d0a6f233703207 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -16,6 +16,7 @@
 #include "SkGlyphCache.h"
 #include "SkPaint.h"
 #include "SkPath.h"
+#include "SkPathOps.h"
 #include "SkPDFFont.h"
 #include "SkPDFFormXObject.h"
 #include "SkPDFGraphicState.h"
@@ -826,7 +827,6 @@ void SkPDFDevice::drawPath(const SkDraw& d, const SkPath& origPath,
         }
         if (!pathIsMutable) {
             pathPtr = &modifiedPath;
-            pathIsMutable = true;

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

Even though this isn't (currently) used after this point, please leave it in so
that if it does get used after this point, it will be accurate.

[ducky, 2013/07/29 19:07:51]

Done.

         }
         bool fill = paint.getFillPath(origPath, pathPtr);
 
@@ -842,6 +842,58 @@ void SkPDFDevice::drawPath(const SkDraw& d, const SkPath& origPath,
         return;
     }
 
+#ifdef SK_PDF_USE_PATHOPS
+    if (origPath.isInverseFillType()) {

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

Lets put this all into a method: handleInverseFill()

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

If paint style is just stroke do we need to do anything special?  Or does
inverse fill with stroke mean to stroke everywhere but the path?

[ducky, 2013/07/29 19:07:51]

Inverse stroke is kind of funny:
For a hairline (or width 1) stroke, it means do the positive stroke (non
inverted).
For thicker strokes, it means inverse stroke (draw everywhere but the path).

[ducky, 2013/07/29 19:07:51]

Done.

+        if (d.fClip->isEmpty()) {
+            return;
+        }
+        if (!pathIsMutable) {

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

Pull this into the if on line 857 so that you can get rid of the else clause.

[ducky, 2013/07/29 19:07:51]

Done - though this whole PathIsMutable thing is a fair bit of complexity just to
save one extra copy operation in some cases. Is it worth it?

[vandebo (ex-Chrome), 2013/07/30 16:50:42]

It depends on how big the thing you are copying is.  A path can be very complex
though.

+            pathPtr = &modifiedPath;

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

pathIsMutable = true;

[ducky, 2013/07/29 19:07:51]

Done.

+        }
+
+        SkPaint finalPaint(paint);

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

nit: finalPaint -> noInversePaint

[ducky, 2013/07/29 19:07:51]

Done.

+
+        // Merge stroking operations into final path.
+        if (SkPaint::kStroke_Style == paint.getStyle()
+            || SkPaint::kStrokeAndFill_Style == paint.getStyle()) {

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

predominate style in this file (and Chrome style) is to put || at the end of the
line.

[ducky, 2013/07/29 19:07:51]

Done.

+            if (paint.getFillPath(origPath, pathPtr)) {
+                finalPaint.setStyle(SkPaint::kFill_Style);
+                finalPaint.setStrokeWidth(0);
+            } else {
+                // Hairline strokes rendered as non-inverted.

[vandebo (ex-Chrome), 2013/07/26 17:48:44]

What does this comment mean?

[ducky, 2013/07/29 19:07:51]

See above explanation of inverse hairline strokes. I'll expand this comment to
be more clear, though.

+                pathPtr->toggleInverseFillType();
+                drawPath(d, *pathPtr, paint, NULL, true);
+                return;
+            }
+        } else {
+            *pathPtr = origPath;
+        }
+
+        // Get bounds of clip in current transform space
+        // (clip bounds are given in device space).
+        SkRect bounds;
+        SkMatrix transformInverse;
+        if (!d.fMatrix->invert(&transformInverse)) {
+            return;
+        }
+        bounds.set(d.fClip->getBounds());
+        transformInverse.mapRect(&bounds);
+
+        // Extend the bounds by the line width (and then some, for hairline
+        // strokes) so the edge doesn't cause a visible stroke.
+        bounds.outset(paint.getStrokeWidth() + SK_Scalar1,
+                      paint.getStrokeWidth() + SK_Scalar1);
+
+        SkPath clipPath;
+        clipPath.addRect(bounds);
+
+        if (Op(clipPath, *pathPtr, kIntersect_PathOp, pathPtr)) {
+            drawPath(d, *pathPtr, finalPaint, NULL, true);
+        }
+        return;
+    }
+#endif
+
     if (handleRectAnnotation(pathPtr->getBounds(), *d.fMatrix, paint)) {
         return;
     }