[PDF] Fixes the cases where SkPDFDevice::finishContentEntry is called with empty...

There are some cases (like drawing a completely transparent image) where the code tries to finish a content entry with no content and an xfermode other than clear.  This makes those cases work correctly.

This is likely what was happening in chromium:316546, but it wasn't clear what the core problem was. desk_techcrunch.skp tripped a debug only assert that exposed the core issue.

BUG=skia:1868,chromium:316546
R=reed@google.com, bungeman@google.com

Author: vandebo@chromium.org

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

git-svn-id: http://skia.googlecode.com/svn/trunk@12473 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/expectations/gm/ignored-tests.txt b/expectations/gm/ignored-tests.txt
index 8d89cbc..bf0bf74 100644 (file)
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -45,6 +45,7 @@ perlinnoise
 
 # Added by bsalomon in support of adding HQ filter to GPU device drawBitmap
 bleed
+
 downsamplebitmap_image_high_mandrill_512.png
 filterbitmap_image_mandrill_512.png
 filterbitmap_image_mandrill_256.png
@@ -65,3 +66,6 @@ filterbitmap_text_10.00pt
 
 # Added by jvanverth in https://codereview.chromium.org/99993002/
 colortype_gpu
+
+# Added by vandebo in https://codereview.chromium.org/92453002/
+xfermodes

diff --git a/gm/xfermodes.cpp b/gm/xfermodes.cpp
index 02cdd61..6e1de3e 100644 (file)
--- a/gm/xfermodes.cpp
+++ b/gm/xfermodes.cpp
@@ -12,7 +12,8 @@
 
 namespace skiagm {
 
-static void make_bitmaps(int w, int h, SkBitmap* src, SkBitmap* dst) {
+static void make_bitmaps(int w, int h, SkBitmap* src, SkBitmap* dst,
+                         SkBitmap* transparent) {
     src->setConfig(SkBitmap::kARGB_8888_Config, w, h);
     src->allocPixels();
     src->eraseColor(SK_ColorTRANSPARENT);
@@ -41,6 +42,10 @@ static void make_bitmaps(int w, int h, SkBitmap* src, SkBitmap* dst) {
         r.set(ww/3, hh/3, ww*19/20, hh*19/20);
         c.drawRect(r, p);
     }
+
+    transparent->setConfig(SkBitmap::kARGB_8888_Config, w, h);
+    transparent->allocPixels();
+    transparent->eraseColor(SK_ColorTRANSPARENT);
 }
 
 static uint16_t gData[] = { 0xFFFF, 0xCCCF, 0xCCCF, 0xFFFF };
@@ -59,13 +64,15 @@ class XfermodesGM : public GM {
      kQuarterClear_SrcType                 = 0x10,
      //! kQuarterClear_SrcType in a layer.
      kQuarterClearInLayer_SrcType          = 0x20,
+     //! A W/2xH/2 transparent image.
+     kSmallTransparentImage_SrcType        = 0x40,
 
-     kAll_SrcType                          = 0x3F, //!< All the source types.
+     kAll_SrcType                          = 0x7F, //!< All the source types.
      kBasic_SrcType                        = 0x03, //!< Just basic source types.
     };
 
     SkBitmap    fBG;
-    SkBitmap    fSrcB, fDstB;
+    SkBitmap    fSrcB, fDstB, fTransparent;
 
     /* The srcType argument indicates what to draw for the source part. Skia
      * uses the implied shape of the drawing command and these modes
@@ -81,6 +88,10 @@ class XfermodesGM : public GM {
         canvas->drawBitmapMatrix(fSrcB, m, &p);
         p.setXfermode(mode);
         switch (srcType) {
+            case kSmallTransparentImage_SrcType:
+                m.postScale(SK_ScalarHalf, SK_ScalarHalf, x, y);
+                canvas->drawBitmapMatrix(fTransparent, m, &p);
+                break;
             case kQuarterClearInLayer_SrcType: {
                 SkRect bounds = SkRect::MakeXYWH(x, y, SkIntToScalar(W),
                                                  SkIntToScalar(H));
@@ -132,7 +143,7 @@ class XfermodesGM : public GM {
         fBG.setConfig(SkBitmap::kARGB_4444_Config, 2, 2, 4, kOpaque_SkAlphaType);
         fBG.setPixels(gData);
 
-        make_bitmaps(W, H, &fSrcB, &fDstB);
+        make_bitmaps(W, H, &fSrcB, &fDstB, &fTransparent);
     }
 
 public:
@@ -146,7 +157,7 @@ protected:
     }
 
     virtual SkISize onISize() {
-        return make_isize(1590, 640);
+        return make_isize(1990, 640);
     }
 
     virtual void onDraw(SkCanvas* canvas) {

diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index bdabc53..cdfec33 100644 (file)
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -1875,11 +1875,12 @@ ContentEntry* SkPDFDevice::setUpContentEntry(const SkClipStack* clipStack,
     return entry;
 }
 
-void SkPDFDevice::finishContentEntry(const SkXfermode::Mode xfermode,
+void SkPDFDevice::finishContentEntry(SkXfermode::Mode xfermode,
                                      SkPDFFormXObject* dst,
                                      SkPath* shape) {
     if (xfermode != SkXfermode::kClear_Mode       &&
             xfermode != SkXfermode::kSrc_Mode     &&
+            xfermode != SkXfermode::kDstOver_Mode &&
             xfermode != SkXfermode::kSrcIn_Mode   &&
             xfermode != SkXfermode::kDstIn_Mode   &&
             xfermode != SkXfermode::kSrcOut_Mode  &&
@@ -1890,6 +1891,18 @@ void SkPDFDevice::finishContentEntry(const SkXfermode::Mode xfermode,
         SkASSERT(!dst);
         return;
     }
+    if (xfermode == SkXfermode::kDstOver_Mode) {
+        SkASSERT(!dst);
+        ContentEntry* firstContentEntry = getContentEntries()->get();
+        if (firstContentEntry->fContent.getOffset() == 0) {
+            // For DstOver, an empty content entry was inserted before the rest
+            // of the content entries. If nothing was drawn, it needs to be
+            // removed.
+            SkAutoTDelete<ContentEntry>* contentEntries = getContentEntries();
+            contentEntries->reset(firstContentEntry->fNext.detach());
+        }
+        return;
+    }
     if (!dst) {
         SkASSERT(xfermode == SkXfermode::kSrc_Mode ||
                  xfermode == SkXfermode::kSrcOut_Mode);
@@ -1904,17 +1917,32 @@ void SkPDFDevice::finishContentEntry(const SkXfermode::Mode xfermode,
     SkClipStack clipStack = contentEntries->fState.fClipStack;
     SkRegion clipRegion = contentEntries->fState.fClipRegion;
 
+    SkMatrix identity;
+    identity.reset();
+    SkPaint stockPaint;
+
     SkAutoTUnref<SkPDFFormXObject> srcFormXObject;
     if (isContentEmpty()) {
-        SkASSERT(xfermode == SkXfermode::kClear_Mode);
+        // If nothing was drawn and there's no shape, then the draw was a
+        // no-op, but dst needs to be restored for that to be true.
+        // If there is shape, then an empty source with Src, SrcIn, SrcOut,
+        // DstIn, DstAtop or Modulate reduces to Clear and DstOut or SrcAtop
+        // reduces to Dst.
+        if (shape == NULL || xfermode == SkXfermode::kDstOut_Mode ||
+                xfermode == SkXfermode::kSrcATop_Mode) {
+            ScopedContentEntry content(this, &clipStack, clipRegion, identity,
+                                       stockPaint);
+            SkPDFUtils::DrawFormXObject(this->addXObjectResource(dst),
+                                        &content.entry()->fContent);
+            return;
+        } else {
+            xfermode = SkXfermode::kClear_Mode;
+        }
     } else {
         SkASSERT(!fContentEntries->fNext.get());
         srcFormXObject.reset(createFormXObjectFromDevice());
     }
 
-    SkMatrix identity;
-    identity.reset();
-
     // TODO(vandebo) srcFormXObject may contain alpha, but here we want it
     // without alpha.
     if (xfermode == SkXfermode::kSrcATop_Mode) {
@@ -1946,16 +1974,7 @@ void SkPDFDevice::finishContentEntry(const SkXfermode::Mode xfermode,
                                 clipRegion, SkXfermode::kSrcOver_Mode, true);
     }
 
-    SkPaint stockPaint;
-
-    if (xfermode == SkXfermode::kSrcATop_Mode) {
-        ScopedContentEntry content(this, &clipStack, clipRegion, identity,
-                                   stockPaint);
-        if (content.entry()) {
-            SkPDFUtils::DrawFormXObject(this->addXObjectResource(dst),
-                                        &content.entry()->fContent);
-        }
-    } else if (xfermode == SkXfermode::kClear_Mode || !srcFormXObject.get()) {
+    if (xfermode == SkXfermode::kClear_Mode) {
         return;
     } else if (xfermode == SkXfermode::kSrc_Mode ||
             xfermode == SkXfermode::kDstATop_Mode) {
@@ -1969,6 +1988,13 @@ void SkPDFDevice::finishContentEntry(const SkXfermode::Mode xfermode,
         if (xfermode == SkXfermode::kSrc_Mode) {
             return;
         }
+    } else if (xfermode == SkXfermode::kSrcATop_Mode) {
+        ScopedContentEntry content(this, &clipStack, clipRegion, identity,
+                                   stockPaint);
+        if (content.entry()) {
+            SkPDFUtils::DrawFormXObject(this->addXObjectResource(dst),
+                                        &content.entry()->fContent);
+        }
     }
 
     SkASSERT(xfermode == SkXfermode::kSrcIn_Mode   ||