SkPDF: with opaque draws, treat SRC mode as SRC_OVER
authorhalcanary <halcanary@google.com>
Wed, 27 May 2015 15:53:36 +0000 (08:53 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 27 May 2015 15:53:36 +0000 (08:53 -0700)
BUG=473572

Review URL: https://codereview.chromium.org/1159763004

gyp/core.gypi
src/core/SkBlitter.cpp
src/core/SkXfermodeInterpretation.cpp [new file with mode: 0644]
src/core/SkXfermodeInterpretation.h [new file with mode: 0644]
src/pdf/SkPDFDevice.cpp
tests/skpdf_opaquesrcmodetosrcover.cpp [new file with mode: 0644]

index 82c0c43..48e691e 100644 (file)
         '<(skia_src_path)/core/SkWriteBuffer.cpp',
         '<(skia_src_path)/core/SkWriter32.cpp',
         '<(skia_src_path)/core/SkXfermode.cpp',
+        '<(skia_src_path)/core/SkXfermodeInterpretation.cpp',
+        '<(skia_src_path)/core/SkXfermodeInterpretation.h',
         '<(skia_src_path)/core/SkYUVPlanesCache.cpp',
         '<(skia_src_path)/core/SkYUVPlanesCache.h',
 
index d8553f1..5276356 100644 (file)
@@ -19,6 +19,7 @@
 #include "SkTLazy.h"
 #include "SkUtils.h"
 #include "SkXfermode.h"
+#include "SkXfermodeInterpretation.h"
 
 SkBlitter::~SkBlitter() {}
 
@@ -777,63 +778,6 @@ private:
 
 #include "SkCoreBlitters.h"
 
-static bool just_solid_color(const SkPaint& paint) {
-    if (paint.getAlpha() == 0xFF && paint.getColorFilter() == NULL) {
-        SkShader* shader = paint.getShader();
-        if (NULL == shader) {
-            return true;
-        }
-    }
-    return false;
-}
-
-/** By analyzing the paint (with an xfermode), we may decide we can take
-    special action. This enum lists our possible actions
- */
-enum XferInterp {
-    kNormal_XferInterp,         // no special interpretation, draw normally
-    kSrcOver_XferInterp,        // draw as if in srcover mode
-    kSkipDrawing_XferInterp     // draw nothing
-};
-
-static XferInterp interpret_xfermode(const SkPaint& paint, SkXfermode* xfer,
-                                     SkColorType deviceCT) {
-    SkXfermode::Mode  mode;
-
-    if (SkXfermode::AsMode(xfer, &mode)) {
-        switch (mode) {
-            case SkXfermode::kSrc_Mode:
-                if (just_solid_color(paint)) {
-                    return kSrcOver_XferInterp;
-                }
-                break;
-            case SkXfermode::kDst_Mode:
-                return kSkipDrawing_XferInterp;
-            case SkXfermode::kSrcOver_Mode:
-                return kSrcOver_XferInterp;
-            case SkXfermode::kDstOver_Mode:
-                if (kRGB_565_SkColorType == deviceCT) {
-                    return kSkipDrawing_XferInterp;
-                }
-                break;
-            case SkXfermode::kSrcIn_Mode:
-                if (kRGB_565_SkColorType == deviceCT &&
-                    just_solid_color(paint)) {
-                    return kSrcOver_XferInterp;
-                }
-                break;
-            case SkXfermode::kDstIn_Mode:
-                if (just_solid_color(paint)) {
-                    return kSkipDrawing_XferInterp;
-                }
-                break;
-            default:
-                break;
-        }
-    }
-    return kNormal_XferInterp;
-}
-
 SkBlitter* SkBlitter::Choose(const SkBitmap& device,
                              const SkMatrix& matrix,
                              const SkPaint& origPaint,
@@ -864,12 +808,13 @@ SkBlitter* SkBlitter::Choose(const SkBitmap& device,
     }
 
     if (mode) {
-        switch (interpret_xfermode(*paint, mode, device.colorType())) {
-            case kSrcOver_XferInterp:
+        bool deviceIsOpaque = kRGB_565_SkColorType == device.colorType();
+        switch (SkInterpretXfermode(*paint, deviceIsOpaque)) {
+            case kSrcOver_SkXfermodeInterpretation:
                 mode = NULL;
                 paint.writable()->setXfermode(NULL);
                 break;
-            case kSkipDrawing_XferInterp:{
+            case kSkipDrawing_SkXfermodeInterpretation:{
                 return allocator->createT<SkNullBlitter>();
             }
             default:
diff --git a/src/core/SkXfermodeInterpretation.cpp b/src/core/SkXfermodeInterpretation.cpp
new file mode 100644 (file)
index 0000000..1b2c8e3
--- /dev/null
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkXfermodeInterpretation.h"
+#include "SkPaint.h"
+
+static bool just_solid_color(const SkPaint& p) {
+    return SK_AlphaOPAQUE == p.getAlpha()
+        && !p.getColorFilter() && !p.getShader();
+}
+
+SkXfermodeInterpretation SkInterpretXfermode(const SkPaint& paint,
+                                             bool dstIsOpaque) {
+    const SkXfermode* xfer = paint.getXfermode();
+    SkXfermode::Mode mode;
+    if (!SkXfermode::AsMode(xfer, &mode)) {
+        return kNormal_SkXfermodeInterpretation;
+    }
+    switch (mode) {
+        case SkXfermode::kSrcOver_Mode:
+            return kSrcOver_SkXfermodeInterpretation;
+        case SkXfermode::kSrc_Mode:
+            if (just_solid_color(paint)) {
+                return kSrcOver_SkXfermodeInterpretation;
+            }
+            return kNormal_SkXfermodeInterpretation;
+        case SkXfermode::kDst_Mode:
+            return kSkipDrawing_SkXfermodeInterpretation;
+        case SkXfermode::kDstOver_Mode:
+            if (dstIsOpaque) {
+                return kSkipDrawing_SkXfermodeInterpretation;
+            }
+            return kNormal_SkXfermodeInterpretation;
+        case SkXfermode::kSrcIn_Mode:
+            if (dstIsOpaque && just_solid_color(paint)) {
+                return kSrcOver_SkXfermodeInterpretation;
+            }
+            return kNormal_SkXfermodeInterpretation;
+        case SkXfermode::kDstIn_Mode:
+            if (just_solid_color(paint)) {
+                return kSkipDrawing_SkXfermodeInterpretation;
+            }
+            return kNormal_SkXfermodeInterpretation;
+        default:
+            return kNormal_SkXfermodeInterpretation;
+    }
+}
diff --git a/src/core/SkXfermodeInterpretation.h b/src/core/SkXfermodeInterpretation.h
new file mode 100644 (file)
index 0000000..f559b33
--- /dev/null
@@ -0,0 +1,22 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkXfermodeInterpretation_DEFINED
+#define SkXfermodeInterpretation_DEFINED
+
+class SkPaint;
+
+/** By analyzing the paint, we may decide we can take special
+    action. This enum lists our possible actions. */
+enum SkXfermodeInterpretation {
+    kNormal_SkXfermodeInterpretation,      // draw normally
+    kSrcOver_SkXfermodeInterpretation,     // draw as if in srcover mode
+    kSkipDrawing_SkXfermodeInterpretation  // draw nothing
+};
+SkXfermodeInterpretation SkInterpretXfermode(const SkPaint&, bool dstIsOpaque);
+
+#endif  // SkXfermodeInterpretation_DEFINED
index 3b0d6f7..b6ad6fe 100644 (file)
 #include "SkTextFormatParams.h"
 #include "SkTemplates.h"
 #include "SkTypefacePriv.h"
+#include "SkXfermodeInterpretation.h"
 
 #define DPI_FOR_RASTER_SCALE_ONE 72
 
 // Utility functions
 
+// If the paint will definitely draw opaquely, replace kSrc_Mode with
+// kSrcOver_Mode.  http://crbug.com/473572
+static void replace_srcmode_on_opaque_paint(SkPaint* paint) {
+    if (kSrcOver_SkXfermodeInterpretation
+        == SkInterpretXfermode(*paint, false)) {
+        paint->setXfermode(NULL);
+    }
+}
+
 static void emit_pdf_color(SkColor color, SkWStream* result) {
     SkASSERT(SkColorGetA(color) == 0xFF);  // We handle alpha elsewhere.
     SkScalar colorScale = SkScalarInvert(0xFF);
@@ -756,6 +766,8 @@ void SkPDFDevice::cleanUp(bool clearFontUsage) {
 
 void SkPDFDevice::drawPaint(const SkDraw& d, const SkPaint& paint) {
     SkPaint newPaint = paint;
+    replace_srcmode_on_opaque_paint(&newPaint);
+
     newPaint.setStyle(SkPaint::kFill_Style);
     ScopedContentEntry content(this, d, newPaint);
     internalDrawPaint(newPaint, content.entry());
@@ -779,9 +791,14 @@ void SkPDFDevice::internalDrawPaint(const SkPaint& paint,
                           &contentEntry->fContent);
 }
 
-void SkPDFDevice::drawPoints(const SkDraw& d, SkCanvas::PointMode mode,
-                             size_t count, const SkPoint* points,
-                             const SkPaint& passedPaint) {
+void SkPDFDevice::drawPoints(const SkDraw& d,
+                             SkCanvas::PointMode mode,
+                             size_t count,
+                             const SkPoint* points,
+                             const SkPaint& srcPaint) {
+    SkPaint passedPaint = srcPaint;
+    replace_srcmode_on_opaque_paint(&passedPaint);
+
     if (count == 0) {
         return;
     }
@@ -866,8 +883,11 @@ void SkPDFDevice::drawPoints(const SkDraw& d, SkCanvas::PointMode mode,
     }
 }
 
-void SkPDFDevice::drawRect(const SkDraw& d, const SkRect& rect,
-                           const SkPaint& paint) {
+void SkPDFDevice::drawRect(const SkDraw& d,
+                           const SkRect& rect,
+                           const SkPaint& srcPaint) {
+    SkPaint paint = srcPaint;
+    replace_srcmode_on_opaque_paint(&paint);
     SkRect r = rect;
     r.sort();
 
@@ -894,21 +914,33 @@ void SkPDFDevice::drawRect(const SkDraw& d, const SkRect& rect,
                           &content.entry()->fContent);
 }
 
-void SkPDFDevice::drawRRect(const SkDraw& draw, const SkRRect& rrect, const SkPaint& paint) {
+void SkPDFDevice::drawRRect(const SkDraw& draw,
+                            const SkRRect& rrect,
+                            const SkPaint& srcPaint) {
+    SkPaint paint = srcPaint;
+    replace_srcmode_on_opaque_paint(&paint);
     SkPath  path;
     path.addRRect(rrect);
     this->drawPath(draw, path, paint, NULL, true);
 }
 
-void SkPDFDevice::drawOval(const SkDraw& draw, const SkRect& oval, const SkPaint& paint) {
+void SkPDFDevice::drawOval(const SkDraw& draw,
+                           const SkRect& oval,
+                           const SkPaint& srcPaint) {
+    SkPaint paint = srcPaint;
+    replace_srcmode_on_opaque_paint(&paint);
     SkPath  path;
     path.addOval(oval);
     this->drawPath(draw, path, paint, NULL, true);
 }
 
-void SkPDFDevice::drawPath(const SkDraw& d, const SkPath& origPath,
-                           const SkPaint& paint, const SkMatrix* prePathMatrix,
+void SkPDFDevice::drawPath(const SkDraw& d,
+                           const SkPath& origPath,
+                           const SkPaint& srcPaint,
+                           const SkMatrix* prePathMatrix,
                            bool pathIsMutable) {
+    SkPaint paint = srcPaint;
+    replace_srcmode_on_opaque_paint(&paint);
     SkPath modifiedPath;
     SkPath* pathPtr = const_cast<SkPath*>(&origPath);
 
@@ -967,8 +999,13 @@ void SkPDFDevice::drawPath(const SkDraw& d, const SkPath& origPath,
 
 void SkPDFDevice::drawBitmapRect(const SkDraw& draw, const SkBitmap& bitmap,
                                  const SkRect* src, const SkRect& dst,
-                                 const SkPaint& paint,
+                                 const SkPaint& srcPaint,
                                  SkCanvas::DrawBitmapRectFlags flags) {
+    SkPaint paint = srcPaint;
+    if (bitmap.isOpaque()) {
+        replace_srcmode_on_opaque_paint(&paint);
+    }
+
     // TODO: this code path must be updated to respect the flags parameter
     SkMatrix    matrix;
     SkRect      bitmapBounds, tmpSrc, tmpDst;
@@ -1023,7 +1060,12 @@ void SkPDFDevice::drawBitmapRect(const SkDraw& draw, const SkBitmap& bitmap,
 }
 
 void SkPDFDevice::drawBitmap(const SkDraw& d, const SkBitmap& bitmap,
-                             const SkMatrix& matrix, const SkPaint& paint) {
+                             const SkMatrix& matrix, const SkPaint& srcPaint) {
+    SkPaint paint = srcPaint;
+    if (bitmap.isOpaque()) {
+        replace_srcmode_on_opaque_paint(&paint);
+    }
+
     if (d.fClip->isEmpty()) {
         return;
     }
@@ -1035,7 +1077,12 @@ void SkPDFDevice::drawBitmap(const SkDraw& d, const SkBitmap& bitmap,
 }
 
 void SkPDFDevice::drawSprite(const SkDraw& d, const SkBitmap& bitmap,
-                             int x, int y, const SkPaint& paint) {
+                             int x, int y, const SkPaint& srcPaint) {
+    SkPaint paint = srcPaint;
+    if (bitmap.isOpaque()) {
+        replace_srcmode_on_opaque_paint(&paint);
+    }
+
     if (d.fClip->isEmpty()) {
         return;
     }
@@ -1082,7 +1129,10 @@ static SkString format_wide_string(const uint16_t* input,
 }
 
 void SkPDFDevice::drawText(const SkDraw& d, const void* text, size_t len,
-                           SkScalar x, SkScalar y, const SkPaint& paint) {
+                           SkScalar x, SkScalar y, const SkPaint& srcPaint) {
+    SkPaint paint = srcPaint;
+    replace_srcmode_on_opaque_paint(&paint);
+
     NOT_IMPLEMENTED(paint.getMaskFilter() != NULL, false);
     if (paint.getMaskFilter() != NULL) {
         // Don't pretend we support drawing MaskFilters, it makes for artifacts
@@ -1131,7 +1181,10 @@ void SkPDFDevice::drawText(const SkDraw& d, const void* text, size_t len,
 
 void SkPDFDevice::drawPosText(const SkDraw& d, const void* text, size_t len,
                               const SkScalar pos[], int scalarsPerPos,
-                              const SkPoint& offset, const SkPaint& paint) {
+                              const SkPoint& offset, const SkPaint& srcPaint) {
+    SkPaint paint = srcPaint;
+    replace_srcmode_on_opaque_paint(&paint);
+
     NOT_IMPLEMENTED(paint.getMaskFilter() != NULL, false);
     if (paint.getMaskFilter() != NULL) {
         // Don't pretend we support drawing MaskFilters, it makes for artifacts
diff --git a/tests/skpdf_opaquesrcmodetosrcover.cpp b/tests/skpdf_opaquesrcmodetosrcover.cpp
new file mode 100644 (file)
index 0000000..f742f3d
--- /dev/null
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#include "SkCanvas.h"
+#include "SkDocument.h"
+#include "SkStream.h"
+#include "Test.h"
+
+static void run_test(SkWStream* out, SkXfermode::Mode mode, U8CPU alpha) {
+    SkAutoTUnref<SkDocument> pdfDoc(SkDocument::CreatePDF(out));
+    SkCanvas* c = pdfDoc->beginPage(612.0f, 792.0f);
+    SkPaint black;
+    SkPaint background;
+    background.setColor(SK_ColorWHITE);
+    background.setAlpha(alpha);
+    background.setXfermodeMode(mode);
+    c->drawRect(SkRect::MakeWH(612.0f, 792.0f), background);
+    c->drawRect(SkRect::MakeXYWH(36.0f, 36.0f, 9.0f, 9.0f), black);
+    c->drawRect(SkRect::MakeXYWH(72.0f, 72.0f, 468.0f, 648.0f), background);
+    c->drawRect(SkRect::MakeXYWH(108.0f, 108.0f, 9.0f, 9.0f), black);
+    pdfDoc->close();
+}
+
+// http://crbug.com/473572
+DEF_TEST(SkPDF_OpaqueSrcModeToSrcOver, r) {
+    SkDynamicMemoryWStream srcMode;
+    SkDynamicMemoryWStream srcOverMode;
+
+    U8CPU alpha = SK_AlphaOPAQUE;
+    run_test(&srcMode, SkXfermode::kSrc_Mode, alpha);
+    run_test(&srcOverMode, SkXfermode::kSrcOver_Mode, alpha);
+    REPORTER_ASSERT(r, srcMode.getOffset() == srcOverMode.getOffset());
+    // The two PDFs should be equal because they have an opaque alpha.
+
+    srcMode.reset();
+    srcOverMode.reset();
+
+    alpha = 0x80;
+    run_test(&srcMode, SkXfermode::kSrc_Mode, alpha);
+    run_test(&srcOverMode, SkXfermode::kSrcOver_Mode, alpha);
+    REPORTER_ASSERT(r, srcMode.getOffset() > srcOverMode.getOffset());
+    // The two PDFs should not be equal because they have a non-opaque alpha.
+}