diff options
author | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2013-08-05 13:28:55 +0000 |
---|---|---|
committer | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2013-08-05 13:28:55 +0000 |
commit | 19dd017a6256be636ccb550752bb563c4e7caeb5 (patch) | |
tree | 8a87fee0a9ef8956c71998edb95db2205b1c2719 | |
parent | a62efcc1e000e4b0ee4fdb3390cadd56452ce3c1 (diff) |
Fix a crash on stroking empty paths with nv_path_rendering enabled
Fix the crash by defining that GrPathRenderer::drawPath and
GrPathRenderer::stencilPath are called only with non-empty paths.
Adds a new test "GpuDrawPath" and tests the condition.
BUG=1477
R=bsalomon@google.com
Author: kkinnunen@nvidia.com
Review URL: https://chromiumcodereview.appspot.com/22173002
git-svn-id: http://skia.googlecode.com/svn/trunk@10528 2bbb7eff-a529-9590-31e7-b0007b416f81
-rw-r--r-- | gyp/tests.gyp | 1 | ||||
-rw-r--r-- | src/gpu/GrClipMaskManager.cpp | 12 | ||||
-rw-r--r-- | src/gpu/GrContext.cpp | 12 | ||||
-rw-r--r-- | src/gpu/GrPathRenderer.h | 2 | ||||
-rw-r--r-- | src/gpu/gl/GrGLPath.cpp | 9 | ||||
-rw-r--r-- | tests/GpuDrawPathTest.cpp | 78 |
6 files changed, 105 insertions, 9 deletions
diff --git a/gyp/tests.gyp b/gyp/tests.gyp index a4ea32d948..b151031f53 100644 --- a/gyp/tests.gyp +++ b/gyp/tests.gyp @@ -62,6 +62,7 @@ '../tests/GLInterfaceValidation.cpp', '../tests/GLProgramsTest.cpp', '../tests/GpuBitmapCopyTest.cpp', + '../tests/GpuDrawPathTest.cpp', '../tests/GrContextFactoryTest.cpp', '../tests/GradientTest.cpp', '../tests/GrMemoryPoolTest.cpp', diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index 93c45641ee..806928ca14 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -691,11 +691,13 @@ bool GrClipMaskManager::createStencilClipMask(InitialState initialState, fGpu->drawSimpleRect(element->getRect(), NULL); } else { GrAssert(Element::kPath_Type == element->getType()); - if (canRenderDirectToStencil) { - *drawState->stencil() = gDrawToStencil; - pr->drawPath(*clipPath, stroke, fGpu, false); - } else { - pr->stencilPath(*clipPath, stroke, fGpu); + if (!clipPath->isEmpty()) { + if (canRenderDirectToStencil) { + *drawState->stencil() = gDrawToStencil; + pr->drawPath(*clipPath, stroke, fGpu, false); + } else { + pr->stencilPath(*clipPath, stroke, fGpu); + } } } } diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 0d98ecd320..20c633dc67 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -25,6 +25,7 @@ #include "GrStencilBuffer.h" #include "GrTextStrike.h" #include "SkRTConf.h" +#include "SkRRect.h" #include "SkStrokeRec.h" #include "SkTLazy.h" #include "SkTLS.h" @@ -952,6 +953,9 @@ void GrContext::drawVertices(const GrPaint& paint, void GrContext::drawRRect(const GrPaint& paint, const SkRRect& rect, const SkStrokeRec& stroke) { + if (rect.isEmpty()) { + return; + } AutoRestoreEffects are; GrDrawTarget* target = this->prepareToDraw(&paint, BUFFERED_DRAW, &are); @@ -972,6 +976,9 @@ void GrContext::drawRRect(const GrPaint& paint, void GrContext::drawOval(const GrPaint& paint, const SkRect& oval, const SkStrokeRec& stroke) { + if (oval.isEmpty()) { + return; + } AutoRestoreEffects are; GrDrawTarget* target = this->prepareToDraw(&paint, BUFFERED_DRAW, &are); @@ -1082,6 +1089,7 @@ void GrContext::drawPath(const GrPaint& paint, const SkPath& path, const SkStrok void GrContext::internalDrawPath(GrDrawTarget* target, bool useAA, const SkPath& path, const SkStrokeRec& stroke) { + SkASSERT(!path.isEmpty()); // An Assumption here is that path renderer would use some form of tweaking // the src color (either the input alpha or in the frag shader) to implement @@ -1112,6 +1120,10 @@ void GrContext::internalDrawPath(GrDrawTarget* target, bool useAA, const SkPath& strokeRec.setFillStyle(); } } + if (pathPtr->isEmpty()) { + return; + } + // This time, allow SW renderer pr = this->getPathRenderer(*pathPtr, strokeRec, target, true, type); } diff --git a/src/gpu/GrPathRenderer.h b/src/gpu/GrPathRenderer.h index f1402d6b23..a64b9e3444 100644 --- a/src/gpu/GrPathRenderer.h +++ b/src/gpu/GrPathRenderer.h @@ -117,6 +117,7 @@ public: const SkStrokeRec& stroke, GrDrawTarget* target, bool antiAlias) { + GrAssert(!path.isEmpty()); GrAssert(this->canDrawPath(path, stroke, target, antiAlias)); GrAssert(target->drawState()->getStencil().isDisabled() || kNoRestriction_StencilSupport == this->getStencilSupport(path, stroke, target)); @@ -132,6 +133,7 @@ public: * @param target target that the path will be rendered to */ void stencilPath(const SkPath& path, const SkStrokeRec& stroke, GrDrawTarget* target) { + GrAssert(!path.isEmpty()); GrAssert(kNoSupport_StencilSupport != this->getStencilSupport(path, stroke, target)); this->onStencilPath(path, stroke, target); } diff --git a/src/gpu/gl/GrGLPath.cpp b/src/gpu/gl/GrGLPath.cpp index 63c2fa107c..d46fa03b5e 100644 --- a/src/gpu/gl/GrGLPath.cpp +++ b/src/gpu/gl/GrGLPath.cpp @@ -59,13 +59,14 @@ inline int num_pts(const SkPath::Verb verb) { static const bool kIsWrapped = false; // The constructor creates the GL path object. GrGLPath::GrGLPath(GrGpuGL* gpu, const SkPath& path) : INHERITED(gpu, kIsWrapped) { - GL_CALL_RET(fPathID, GenPaths(1)); - SkPath::Iter iter(path, true); - - SkSTArray<16, GrGLubyte, true> pathCommands; #ifndef SK_SCALAR_IS_FLOAT GrCrash("Assumes scalar is float."); #endif + SkASSERT(!path.isEmpty()); + + GL_CALL_RET(fPathID, GenPaths(1)); + + SkSTArray<16, GrGLubyte, true> pathCommands; SkSTArray<16, SkPoint, true> pathPoints; int verbCnt = path.countVerbs(); diff --git a/tests/GpuDrawPathTest.cpp b/tests/GpuDrawPathTest.cpp new file mode 100644 index 0000000000..3ec87d4777 --- /dev/null +++ b/tests/GpuDrawPathTest.cpp @@ -0,0 +1,78 @@ + +/* + * Copyright 2013 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#if SK_SUPPORT_GPU + +#include "GrContext.h" +#include "GrContextFactory.h" +#include "SkBitmap.h" +#include "SkCanvas.h" +#include "SkColor.h" +#include "SkGpuDevice.h" +#include "SkPaint.h" +#include "SkRect.h" +#include "SkRRect.h" +#include "Test.h" + +static void test_drawPathEmpty(skiatest::Reporter*, SkCanvas* canvas) +{ + // Filling an empty path should not crash. + SkPaint paint; + canvas->drawRect(SkRect(), paint); + canvas->drawPath(SkPath(), paint); + canvas->drawOval(SkRect(), paint); + canvas->drawRect(SkRect(), paint); + canvas->drawRRect(SkRRect(), paint); + + // Stroking an empty path should not crash. + paint.setAntiAlias(true); + paint.setStyle(SkPaint::kStroke_Style); + paint.setColor(SK_ColorGRAY); + paint.setStrokeWidth(SkIntToScalar(20)); + paint.setStrokeJoin(SkPaint::kRound_Join); + canvas->drawRect(SkRect(), paint); + canvas->drawPath(SkPath(), paint); + canvas->drawOval(SkRect(), paint); + canvas->drawRect(SkRect(), paint); + canvas->drawRRect(SkRRect(), paint); +} + + +static void TestGpuDrawPath(skiatest::Reporter* reporter, GrContextFactory* factory) { + for (int type = 0; type < GrContextFactory::kLastGLContextType; ++type) { + GrContextFactory::GLContextType glType = static_cast<GrContextFactory::GLContextType>(type); + + GrContext* grContext = factory->get(glType); + if (NULL == grContext) { + continue; + } + static const int sampleCounts[] = { 0, 4, 16 }; + + for (size_t i = 0; i < SK_ARRAY_COUNT(sampleCounts); ++i) { + const int W = 255; + const int H = 255; + + GrTextureDesc desc; + desc.fConfig = kSkia8888_GrPixelConfig; + desc.fFlags = kRenderTarget_GrTextureFlagBit; + desc.fWidth = W; + desc.fHeight = H; + desc.fSampleCnt = sampleCounts[i]; + SkAutoTUnref<GrTexture> texture(grContext->createUncachedTexture(desc, NULL, 0)); + SkAutoTUnref<SkGpuDevice> device(SkNEW_ARGS(SkGpuDevice, (grContext, texture.get()))); + SkCanvas drawingCanvas(device.get()); + + test_drawPathEmpty(reporter, &drawingCanvas); + } + } +} + +#include "TestClassDef.h" +DEFINE_GPUTESTCLASS("GpuDrawPath", TestGpuDrawPathClass, TestGpuDrawPath) + +#endif |