SkPDF: with opaque draws, treat SRC mode as SRC_OVER
BUG=473572
Review URL: https://codereview.chromium.org/1159763004
diff --git a/gyp/core.gypi b/gyp/core.gypi
index 82c0c43..48e691e 100644
--- a/gyp/core.gypi
+++ b/gyp/core.gypi
@@ -228,6 +228,8 @@
'<(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',
diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index d8553f1..5276356 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -19,6 +19,7 @@
#include "SkTLazy.h"
#include "SkUtils.h"
#include "SkXfermode.h"
+#include "SkXfermodeInterpretation.h"
SkBlitter::~SkBlitter() {}
@@ -777,63 +778,6 @@
#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 @@
}
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
index 0000000..1b2c8e3
--- /dev/null
+++ b/src/core/SkXfermodeInterpretation.cpp
@@ -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
index 0000000..f559b33
--- /dev/null
+++ b/src/core/SkXfermodeInterpretation.h
@@ -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
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 3b0d6f7..b6ad6fe 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -32,11 +32,21 @@
#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::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 @@
&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::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 @@
&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::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::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::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 @@
}
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::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
index 0000000..f742f3d
--- /dev/null
+++ b/tests/skpdf_opaquesrcmodetosrcover.cpp
@@ -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.
+}