Don't draw if SkShader::asNewFragmentProcessor fails.
BUG=chromium:473156
Review URL: https://codereview.chromium.org/1089063002
diff --git a/gm/badpaint.cpp b/gm/badpaint.cpp
new file mode 100644
index 0000000..45b17f8
--- /dev/null
+++ b/gm/badpaint.cpp
@@ -0,0 +1,62 @@
+/*
+ * 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 "gm.h"
+#include "SkCanvas.h"
+#include "SkShader.h"
+
+
+/** This GM draws with invalid paints. It should draw nothing other than the background. */
+class BadPaintGM : public skiagm::GM {
+ public:
+ BadPaintGM() {}
+
+protected:
+ SkString onShortName() override { return SkString("badpaint"); }
+
+ SkISize onISize() override { return SkISize::Make(100, 100); }
+
+ void onOnceBeforeDraw() override {
+ SkBitmap emptyBmp;
+
+ SkBitmap blueBmp;
+ blueBmp.allocN32Pixels(10, 10);
+ blueBmp.eraseColor(SK_ColorBLUE);
+
+ SkMatrix badMatrix;
+ badMatrix.setAll(0, 0, 0, 0, 0, 0, 0, 0, 0);
+
+#if 0 // This crashes pipe!
+ // Empty bitmap.
+ fPaints.push_back().setColor(SK_ColorGREEN);
+ fPaints.back().setShader(SkShader::CreateBitmapShader(emptyBmp, SkShader::kClamp_TileMode,
+ SkShader::kClamp_TileMode))->unref();
+#endif
+
+ // Non-invertible local matrix.
+ fPaints.push_back().setColor(SK_ColorGREEN);
+ fPaints.back().setShader(SkShader::CreateBitmapShader(blueBmp, SkShader::kClamp_TileMode,
+ SkShader::kClamp_TileMode,
+ &badMatrix))->unref();
+ }
+
+ void onDraw(SkCanvas* canvas) override {
+ SkRect rect = SkRect::MakeXYWH(10, 10, 80, 80);
+ for (int i = 0; i < fPaints.count(); ++i) {
+ canvas->drawRect(rect, fPaints[i]);
+ }
+ }
+
+private:
+ SkTArray<SkPaint> fPaints;
+
+ typedef skiagm::GM INHERITED;
+};
+
+/////////////////////////////////////////////////////////////////////////////////////
+
+DEF_GM( return SkNEW(BadPaintGM); )
diff --git a/gm/constcolorprocessor.cpp b/gm/constcolorprocessor.cpp
index 672e07e..159bf06 100644
--- a/gm/constcolorprocessor.cpp
+++ b/gm/constcolorprocessor.cpp
@@ -104,7 +104,8 @@
} else {
skPaint.setColor(kPaintColors[paintType]);
}
- SkPaint2GrPaintShader(context, rt, skPaint, viewMatrix, false, &grPaint);
+ SkAssertResult(SkPaint2GrPaint(context, rt, skPaint, viewMatrix, false,
+ &grPaint));
GrConstColorProcessor::InputMode mode = (GrConstColorProcessor::InputMode) m;
GrColor color = kColors[procColor];
diff --git a/gyp/gmslides.gypi b/gyp/gmslides.gypi
index 3f02937..d7cc51a 100644
--- a/gyp/gmslides.gypi
+++ b/gyp/gmslides.gypi
@@ -26,6 +26,7 @@
'../gm/arcofzorro.cpp',
'../gm/arithmode.cpp',
'../gm/astcbitmap.cpp',
+ '../gm/badpaint.cpp',
'../gm/beziereffects.cpp',
'../gm/beziers.cpp',
'../gm/bigblurs.cpp',
diff --git a/include/gpu/SkGr.h b/include/gpu/SkGr.h
index 026b256..c73701e 100644
--- a/include/gpu/SkGr.h
+++ b/include/gpu/SkGr.h
@@ -81,14 +81,14 @@
// an optimization where the color filter can be applied to the SkPaint's
// color once while converting to GrPaint and then ignored. TODO: Remove this
// bool and use the invariant info to automatically apply the color filter.
-void SkPaint2GrPaintNoShader(GrContext* context, GrRenderTarget*, const SkPaint& skPaint,
+bool SkPaint2GrPaintNoShader(GrContext* context, GrRenderTarget*, const SkPaint& skPaint,
GrColor paintColor, bool constantColor, GrPaint* grPaint);
// This function is similar to skPaint2GrPaintNoShader but also converts
// skPaint's shader to a GrFragmentProcessor if possible.
// constantColor has the same meaning as in skPaint2GrPaintNoShader.
-void SkPaint2GrPaintShader(GrContext* context, GrRenderTarget*, const SkPaint& skPaint,
- const SkMatrix& viewM, bool constantColor, GrPaint* grPaint);
+bool SkPaint2GrPaint(GrContext* context, GrRenderTarget*, const SkPaint& skPaint,
+ const SkMatrix& viewM, bool constantColor, GrPaint* grPaint);
////////////////////////////////////////////////////////////////////////////////
// Classes
diff --git a/src/gpu/GrAtlasTextContext.cpp b/src/gpu/GrAtlasTextContext.cpp
index 29dd072..f2e993a 100644
--- a/src/gpu/GrAtlasTextContext.cpp
+++ b/src/gpu/GrAtlasTextContext.cpp
@@ -247,7 +247,9 @@
// Though for the time being runs in the textblob can override the paint, they only touch font
// info.
GrPaint grPaint;
- SkPaint2GrPaintShader(fContext, rt, skPaint, viewMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(fContext, rt, skPaint, viewMatrix, true, &grPaint)) {
+ return;
+ }
if (cacheBlob) {
if (MustRegenerateBlob(&transX, &transY, *cacheBlob, skPaint, blurRec, viewMatrix, x, y)) {
diff --git a/src/gpu/GrTextContext.cpp b/src/gpu/GrTextContext.cpp
index 3218710..6153f3b 100644
--- a/src/gpu/GrTextContext.cpp
+++ b/src/gpu/GrTextContext.cpp
@@ -116,7 +116,9 @@
runPaint.setFlags(fGpuDevice->filterTextFlags(runPaint));
GrPaint grPaint;
- SkPaint2GrPaintShader(fContext, fRenderTarget, runPaint, viewMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(fContext, fRenderTarget, runPaint, viewMatrix, true, &grPaint)) {
+ return;
+ }
switch (it.positioning()) {
case SkTextBlob::kDefault_Positioning:
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index a3f2614..337f400 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -365,7 +365,9 @@
GR_CREATE_TRACE_MARKER_CONTEXT("SkGpuDevice::drawPaint", fContext);
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint)) {
+ return;
+ }
fContext->drawPaint(fRenderTarget, fClip, grPaint, *draw.fMatrix);
}
@@ -390,7 +392,10 @@
if (paint.getPathEffect() && 2 == count && SkCanvas::kLines_PointMode == mode) {
GrStrokeInfo strokeInfo(paint, SkPaint::kStroke_Style);
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true,
+ &grPaint)) {
+ return;
+ }
SkPath path;
path.setIsVolatile(true);
path.moveTo(pts[0]);
@@ -407,7 +412,9 @@
}
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint)) {
+ return;
+ }
fContext->drawVertices(fRenderTarget,
fClip,
@@ -479,7 +486,9 @@
}
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint)) {
+ return;
+ }
fContext->drawRect(fRenderTarget, fClip, grPaint, *draw.fMatrix, rect, &strokeInfo);
}
@@ -493,7 +502,9 @@
CHECK_SHOULD_DRAW(draw);
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint)) {
+ return;
+ }
GrStrokeInfo strokeInfo(paint);
if (paint.getMaskFilter()) {
@@ -561,7 +572,10 @@
CHECK_SHOULD_DRAW(draw);
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true,
+ &grPaint)) {
+ return;
+ }
if (NULL == paint.getMaskFilter() && NULL == paint.getPathEffect()) {
fContext->drawDRRect(fRenderTarget, fClip, grPaint, *draw.fMatrix, outer, inner);
@@ -609,7 +623,9 @@
}
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint)) {
+ return;
+ }
fContext->drawOval(fRenderTarget, fClip, grPaint, *draw.fMatrix, oval, strokeInfo);
}
@@ -804,7 +820,9 @@
SkDEBUGCODE(prePathMatrix = (const SkMatrix*)0x50FF8001;)
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, viewMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, viewMatrix, true, &grPaint)) {
+ return;
+ }
const SkRect* cullRect = NULL; // TODO: what is our bounds?
SkStrokeRec* strokePtr = strokeInfo.getStrokeRecPtr();
@@ -1478,7 +1496,10 @@
bool alphaOnly = !(kAlpha_8_SkColorType == bitmap.colorType());
GrColor paintColor = (alphaOnly) ? SkColor2GrColorJustAlpha(paint.getColor()) :
SkColor2GrColor(paint.getColor());
- SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint, paintColor, false, &grPaint);
+ if (!SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint, paintColor, false,
+ &grPaint)) {
+ return;
+ }
fContext->drawNonAARectToRect(fRenderTarget, fClip, grPaint, viewMatrix, dstRect,
paintRect);
@@ -1551,8 +1572,10 @@
GrPaint grPaint;
grPaint.addColorTextureProcessor(texture, SkMatrix::I());
- SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint,
- SkColor2GrColorJustAlpha(paint.getColor()), false, &grPaint);
+ if (!SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint,
+ SkColor2GrColorJustAlpha(paint.getColor()), false, &grPaint)) {
+ return;
+ }
fContext->drawNonAARectToRect(fRenderTarget,
fClip,
@@ -1665,8 +1688,10 @@
GrPaint grPaint;
grPaint.addColorTextureProcessor(devTex, SkMatrix::I());
- SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint,
- SkColor2GrColorJustAlpha(paint.getColor()), false, &grPaint);
+ if (!SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint,
+ SkColor2GrColorJustAlpha(paint.getColor()), false, &grPaint)) {
+ return;
+ }
SkRect dstRect = SkRect::MakeXYWH(SkIntToScalar(x),
SkIntToScalar(y),
@@ -1741,8 +1766,10 @@
copy.setStrokeWidth(0);
// we ignore the shader if texs is null.
- SkPaint2GrPaintNoShader(this->context(), fRenderTarget, copy,
- SkColor2GrColor(copy.getColor()), NULL == colors, &grPaint);
+ if (!SkPaint2GrPaintNoShader(this->context(), fRenderTarget, copy,
+ SkColor2GrColor(copy.getColor()), NULL == colors, &grPaint)) {
+ return;
+ }
primType = kLines_GrPrimitiveType;
int triangleCount = 0;
@@ -1781,12 +1808,16 @@
primType = gVertexMode2PrimitiveType[vmode];
if (NULL == texs || NULL == paint.getShader()) {
- SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint,
- SkColor2GrColor(paint.getColor()),
- NULL == colors, &grPaint);
+ if (!SkPaint2GrPaintNoShader(this->context(), fRenderTarget, paint,
+ SkColor2GrColor(paint.getColor()),
+ NULL == colors, &grPaint)) {
+ return;
+ }
} else {
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix,
- NULL == colors, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix,
+ NULL == colors, &grPaint)) {
+ return;
+ }
}
}
@@ -1835,7 +1866,9 @@
GR_CREATE_TRACE_MARKER_CONTEXT("SkGpuDevice::drawText", fContext);
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint)) {
+ return;
+ }
SkDEBUGCODE(this->validate();)
@@ -1850,7 +1883,9 @@
CHECK_SHOULD_DRAW(draw);
GrPaint grPaint;
- SkPaint2GrPaintShader(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint);
+ if (!SkPaint2GrPaint(this->context(), fRenderTarget, paint, *draw.fMatrix, true, &grPaint)) {
+ return;
+ }
SkDEBUGCODE(this->validate();)
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index b44276c..3161c81 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -642,7 +642,7 @@
///////////////////////////////////////////////////////////////////////////////
-void SkPaint2GrPaintNoShader(GrContext* context, GrRenderTarget* rt, const SkPaint& skPaint,
+bool SkPaint2GrPaintNoShader(GrContext* context, GrRenderTarget* rt, const SkPaint& skPaint,
GrColor paintColor, bool constantColor, GrPaint* grPaint) {
grPaint->setDither(skPaint.isDither());
@@ -652,6 +652,7 @@
GrXPFactory* xpFactory = NULL;
if (!SkXfermode::AsXPFactory(mode, &xpFactory)) {
// Fall back to src-over
+ // return false here?
xpFactory = GrPorterDuffXPFactory::Create(SkXfermode::kSrcOver_Mode);
}
SkASSERT(xpFactory);
@@ -669,6 +670,7 @@
grPaint->setColor(SkColor2GrColor(filtered));
} else {
SkTDArray<GrFragmentProcessor*> array;
+ // return false if failed?
if (colorFilter->asFragmentProcessors(context, &array)) {
for (int i = 0; i < array.count(); ++i) {
grPaint->addColorProcessor(array[i]);
@@ -699,15 +701,15 @@
}
}
#endif
+ return true;
}
-void SkPaint2GrPaintShader(GrContext* context, GrRenderTarget* rt, const SkPaint& skPaint,
- const SkMatrix& viewM, bool constantColor, GrPaint* grPaint) {
+bool SkPaint2GrPaint(GrContext* context, GrRenderTarget* rt, const SkPaint& skPaint,
+ const SkMatrix& viewM, bool constantColor, GrPaint* grPaint) {
SkShader* shader = skPaint.getShader();
if (NULL == shader) {
- SkPaint2GrPaintNoShader(context, rt, skPaint, SkColor2GrColor(skPaint.getColor()),
- constantColor, grPaint);
- return;
+ return SkPaint2GrPaintNoShader(context, rt, skPaint, SkColor2GrColor(skPaint.getColor()),
+ constantColor, grPaint);
}
GrColor paintColor = SkColor2GrColor(skPaint.getColor());
@@ -719,7 +721,10 @@
// Allow the shader to modify paintColor and also create an effect to be installed as
// the first color effect on the GrPaint.
GrFragmentProcessor* fp = NULL;
- if (shader->asFragmentProcessor(context, skPaint, viewM, NULL, &paintColor, &fp) && fp) {
+ if (!shader->asFragmentProcessor(context, skPaint, viewM, NULL, &paintColor, &fp)) {
+ return false;
+ }
+ if (fp) {
grPaint->addColorProcessor(fp)->unref();
constantColor = false;
}
@@ -727,5 +732,5 @@
// The grcolor is automatically set when calling asFragmentProcessor.
// If the shader can be seen as an effect it returns true and adds its effect to the grpaint.
- SkPaint2GrPaintNoShader(context, rt, skPaint, paintColor, constantColor, grPaint);
+ return SkPaint2GrPaintNoShader(context, rt, skPaint, paintColor, constantColor, grPaint);
}