Bug 1316654 - Fix the DrawTargetSkia::CreateSimilarDrawTarget check for non-raster backed SkCanvas to avoid false positives. r=lsalzman
authorJonathan Watt <jwatt@jwatt.org>
Thu, 22 Dec 2016 01:35:22 +0000
changeset 327967 358b2e64b5c1cf592607159188843c7fbbe9367c
parent 327966 56f12a46fbca0a38d6ce4ef8a33ff8f98286de06
child 327968 910efd159ce835e3fd7540d77e87921826297406
push id31160
push userphilringnalda@gmail.com
push dateThu, 05 Jan 2017 02:33:44 +0000
treeherdermozilla-central@f13abb8ba9f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1316654
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1316654 - Fix the DrawTargetSkia::CreateSimilarDrawTarget check for non-raster backed SkCanvas to avoid false positives. r=lsalzman
gfx/2d/DrawTargetSkia.cpp
gfx/2d/HelpersSkia.h
--- a/gfx/2d/DrawTargetSkia.cpp
+++ b/gfx/2d/DrawTargetSkia.cpp
@@ -1582,21 +1582,21 @@ DrawTargetSkia::CreateSimilarDrawTarget(
     if (target->InitWithGrContext(mGrContext.get(), aSize, aFormat, true)) {
       return target.forget();
     }
     // Otherwise, just fall back to a software draw target.
   }
 #endif
 
 #ifdef DEBUG
-  // Check that our SkCanvas isn't backed by vector storage such as PDF.  If it
-  // is then we want similar storage to avoid losing fidelity (if and when this
-  // DrawTarget is Snapshot()'ed, drawning a raster back into this DrawTarget
-  // will lose fidelity).
-  if (mCanvas->imageInfo().colorType() == kUnknown_SkColorType) {
+  if (!IsBackedByPixels(mCanvas.get())) {
+    // If our canvas is backed by vector storage such as PDF then we want to
+    // create a new DrawTarget with similar storage to avoid losing fidelity
+    // (fidelity will be lost if the returned DT is Snapshot()'ed and drawn
+    // back onto us since a raster will be drawn instead of vector commands).
     NS_WARNING("Not backed by pixels - we need to handle PDF backed SkCanvas");
   }
 #endif
 
   if (!target->Init(aSize, aFormat)) {
     return nullptr;
   }
   return target.forget();
@@ -1788,19 +1788,17 @@ bool
 DrawTargetSkia::Init(SkCanvas* aCanvas)
 {
   mCanvas = sk_ref_sp(aCanvas);
 
   SkImageInfo imageInfo = mCanvas->imageInfo();
 
   // If the canvas is backed by pixels we clear it to be on the safe side.  If
   // it's not (for example, for PDF output) we don't.
-  bool isBackedByPixels = imageInfo.colorType() != kUnknown_SkColorType;
-  if (isBackedByPixels) {
-    // Note for PDF backed SkCanvas |alphaType == kUnknown_SkAlphaType|.
+  if (IsBackedByPixels(mCanvas.get())) {
     SkColor clearColor = imageInfo.isOpaque() ? SK_ColorBLACK : SK_ColorTRANSPARENT;
     mCanvas->clear(clearColor);
   }
 
   SkISize size = mCanvas->getBaseLayerSize();
   mSize.width = size.width();
   mSize.height = size.height();
   mFormat = SkiaColorTypeToGfxFormat(imageInfo.colorType(),
--- a/gfx/2d/HelpersSkia.h
+++ b/gfx/2d/HelpersSkia.h
@@ -366,12 +366,31 @@ static inline FillRule GetFillRule(SkPat
   default:
     NS_WARNING("Unsupported fill type\n");
     break;
   }
 
   return FillRule::FILL_EVEN_ODD;
 }
 
+/**
+ * Returns true if the canvas is backed by pixels.  Returns false if the canvas
+ * wraps an SkPDFDocument, for example.
+ *
+ * Note: It is not clear whether the test used to implement this function may
+ * result in it returning false in some circumstances even when the canvas
+ * _is_ pixel backed.  In other words maybe it is possible for such a canvas to
+ * have kUnknown_SkPixelGeometry?
+ */
+static inline bool IsBackedByPixels(const SkCanvas* aCanvas)
+{
+  SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
+  if (!aCanvas->getProps(&props) ||
+      props.pixelGeometry() == kUnknown_SkPixelGeometry) {
+    return false;
+  }
+  return true;
+}
+
 } // namespace gfx
 } // namespace mozilla
 
 #endif /* MOZILLA_GFX_HELPERSSKIA_H_ */