Cherry-pick fix for GrAADistanceFieldPathRenderer bounds to M53.
authorbsalomon <bsalomon@google.com>
Mon, 18 Jul 2016 21:28:53 +0000 (14:28 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 18 Jul 2016 21:28:54 +0000 (14:28 -0700)
-----------

Initialize fGammaCorrect in DF Path Renderer unit test

TBR=jvanverth@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2157933003

Review-Url: https://codereview.chromium.org/2157933003

Fix leak when DFPR fails to draw path

TBR=jvanverth@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2144283002

Review-Url: https://codereview.chromium.org/2144283002

Make GrBatchAtlas robust against attempts to add large rects.

Make GrAADistanceFieldPathRenderer robust against paths that in src space wind up being too large for the atlas.

BUG=chromium:627443
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2144663004

Review-Url: https://codereview.chromium.org/2144663004
NOTREECHECKS=true
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2160563004

src/gpu/GrBatchAtlas.cpp
src/gpu/GrBatchAtlas.h
src/gpu/batches/GrAADistanceFieldPathRenderer.cpp
tests/DFPathRendererTest.cpp [new file with mode: 0644]

index 40ab0e6..8cc6f61 100644 (file)
@@ -116,11 +116,11 @@ GrBatchAtlas::GrBatchAtlas(GrTexture* texture, int numPlotsX, int numPlotsY)
     : fTexture(texture)
     , fAtlasGeneration(kInvalidAtlasGeneration + 1) {
 
-    int plotWidth = texture->width() / numPlotsX;
-    int plotHeight = texture->height() / numPlotsY;
+    fPlotWidth = texture->width() / numPlotsX;
+    fPlotHeight = texture->height() / numPlotsY;
     SkASSERT(numPlotsX * numPlotsY <= BulkUseTokenUpdater::kMaxPlots);
-    SkASSERT(plotWidth * numPlotsX == texture->width());
-    SkASSERT(plotHeight * numPlotsY == texture->height());
+    SkASSERT(fPlotWidth * numPlotsX == texture->width());
+    SkASSERT(fPlotHeight * numPlotsY == texture->height());
 
     SkDEBUGCODE(fNumPlots = numPlotsX * numPlotsY;)
 
@@ -134,7 +134,7 @@ GrBatchAtlas::GrBatchAtlas(GrTexture* texture, int numPlotsX, int numPlotsY)
     for (int y = numPlotsY - 1, r = 0; y >= 0; --y, ++r) {
         for (int x = numPlotsX - 1, c = 0; x >= 0; --x, ++c) {
             uint32_t index = r * numPlotsX + c;
-            currPlot->reset(new BatchPlot(index, 1, x, y, plotWidth, plotHeight,
+            currPlot->reset(new BatchPlot(index, 1, x, y, fPlotWidth, fPlotHeight,
                                           texture->desc().fConfig));
 
             // build LRU list
@@ -179,6 +179,9 @@ bool GrBatchAtlas::addToAtlas(AtlasID* id, GrDrawBatch::Target* target,
                               int width, int height, const void* image, SkIPoint16* loc) {
     // We should already have a texture, TODO clean this up
     SkASSERT(fTexture);
+    if (width > fPlotWidth || height > fPlotHeight) {
+        return false;
+    }
 
     // now look through all allocated plots for one we can share, in Most Recently Refed order
     GrBatchPlotList::Iter plotIter;
index 707f463..827106f 100644 (file)
@@ -241,6 +241,8 @@ private:
     inline void processEviction(AtlasID);
 
     GrTexture* fTexture;
+    int        fPlotWidth;
+    int        fPlotHeight;
     SkDEBUGCODE(uint32_t fNumPlots;)
 
     uint64_t fAtlasGeneration;
index 725358e..11a6c03 100644 (file)
@@ -226,6 +226,8 @@ private:
         }
 
         flushInfo.fInstancesToFlush = 0;
+        // Pointer to the next set of vertices to write.
+        intptr_t offset = reinterpret_cast<intptr_t>(vertices);
         for (int i = 0; i < instanceCount; i++) {
             const Geometry& args = fGeoData[i];
 
@@ -263,24 +265,22 @@ private:
                                           args.fAntiAlias,
                                           desiredDimension,
                                           scale)) {
+                    delete shapeData;
                     SkDebugf("Can't rasterize path\n");
-                    return;
+                    continue;
                 }
             }
 
             atlas->setLastUseToken(shapeData->fID, target->nextDrawToken());
 
-            // Now set vertices
-            intptr_t offset = reinterpret_cast<intptr_t>(vertices);
-            offset += i * kVerticesPerQuad * vertexStride;
             this->writePathVertices(target,
                                     atlas,
                                     offset,
                                     args.fColor,
                                     vertexStride,
                                     this->viewMatrix(),
-                                    args.fShape,
                                     shapeData);
+            offset += kVerticesPerQuad * vertexStride;
             flushInfo.fInstancesToFlush++;
         }
 
@@ -371,15 +371,11 @@ private:
         // add to atlas
         SkIPoint16 atlasLocation;
         GrBatchAtlas::AtlasID id;
-        bool success = atlas->addToAtlas(&id, target, width, height, dfStorage.get(),
-                                         &atlasLocation);
-        if (!success) {
+       if (!atlas->addToAtlas(&id, target, width, height, dfStorage.get(), &atlasLocation)) {
             this->flush(target, flushInfo);
-
-            SkDEBUGCODE(success =) atlas->addToAtlas(&id, target, width, height,
-                                                     dfStorage.get(), &atlasLocation);
-            SkASSERT(success);
-
+            if (!atlas->addToAtlas(&id, target, width, height, dfStorage.get(), &atlasLocation)) {
+                return false;
+            }
         }
 
         // add to cache
@@ -415,7 +411,6 @@ private:
                            GrColor color,
                            size_t vertexStride,
                            const SkMatrix& viewMatrix,
-                           const GrShape& shape,
                            const ShapeData* shapeData) const {
         GrTexture* texture = atlas->getTexture();
 
@@ -456,15 +451,17 @@ private:
     }
 
     void flush(GrVertexBatch::Target* target, FlushInfo* flushInfo) const {
-        GrMesh mesh;
-        int maxInstancesPerDraw =
-            static_cast<int>(flushInfo->fIndexBuffer->gpuMemorySize() / sizeof(uint16_t) / 6);
-        mesh.initInstanced(kTriangles_GrPrimitiveType, flushInfo->fVertexBuffer,
-            flushInfo->fIndexBuffer, flushInfo->fVertexOffset, kVerticesPerQuad,
-            kIndicesPerQuad, flushInfo->fInstancesToFlush, maxInstancesPerDraw);
-        target->draw(flushInfo->fGeometryProcessor.get(), mesh);
-        flushInfo->fVertexOffset += kVerticesPerQuad * flushInfo->fInstancesToFlush;
-        flushInfo->fInstancesToFlush = 0;
+        if (flushInfo->fInstancesToFlush) {
+            GrMesh mesh;
+            int maxInstancesPerDraw =
+                static_cast<int>(flushInfo->fIndexBuffer->gpuMemorySize() / sizeof(uint16_t) / 6);
+            mesh.initInstanced(kTriangles_GrPrimitiveType, flushInfo->fVertexBuffer,
+                flushInfo->fIndexBuffer, flushInfo->fVertexOffset, kVerticesPerQuad,
+                kIndicesPerQuad, flushInfo->fInstancesToFlush, maxInstancesPerDraw);
+            target->draw(flushInfo->fGeometryProcessor.get(), mesh);
+            flushInfo->fVertexOffset += kVerticesPerQuad * flushInfo->fInstancesToFlush;
+            flushInfo->fInstancesToFlush = 0;
+        }
     }
 
     GrColor color() const { return fGeoData[0].fColor; }
diff --git a/tests/DFPathRendererTest.cpp b/tests/DFPathRendererTest.cpp
new file mode 100644 (file)
index 0000000..9613a8c
--- /dev/null
@@ -0,0 +1,80 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Test.h"
+
+#if SK_SUPPORT_GPU
+#include "GrContext.h"
+#include "GrTest.h"
+#include "batches/GrAADistanceFieldPathRenderer.h"
+#include "SkPath.h"
+
+// This test case including path coords and matrix taken from crbug.com/627443.
+// Because of inaccuracies in large floating point values this causes the
+// the path renderer to attempt to add a path DF to its atlas that is larger
+// than the plot size which used to crash rather than fail gracefully.
+static void test_far_from_origin(GrDrawContext* drawContext, GrPathRenderer* pr,
+                                 GrResourceProvider* rp) {
+    SkPath path;
+    path.lineTo(49.0255089839f, 0.473541f);
+    static constexpr SkScalar mvals[] = {14.0348252854f, 2.13026182736f,
+                                         13.6122547187f, 118.309922702f,
+                                         1912337682.09f, 2105391889.87f};
+    SkMatrix matrix;
+    matrix.setAffine(mvals);
+    SkMatrix inverse;
+    SkAssertResult(matrix.invert(&inverse));
+    path.transform(inverse);
+
+    SkStrokeRec rec(SkStrokeRec::kFill_InitStyle);
+    rec.setStrokeStyle(1.f);
+    rec.setStrokeParams(SkPaint::kRound_Cap, SkPaint::kRound_Join, 1.f);
+    GrStyle style(rec, nullptr);
+
+    GrShape shape(path, style);
+    shape = shape.applyStyle(GrStyle::Apply::kPathEffectAndStrokeRec, 1.f);
+
+    GrPaint paint;
+    paint.setXPFactory(GrPorterDuffXPFactory::Make(SkXfermode::kSrc_Mode));
+
+    GrNoClip noClip;
+    GrPathRenderer::DrawPathArgs args;
+    args.fPaint = &paint;
+    args.fUserStencilSettings = &GrUserStencilSettings::kUnused;
+    args.fDrawContext = drawContext;
+    args.fClip = &noClip;
+    args.fResourceProvider = rp;
+    args.fViewMatrix = &matrix;
+    args.fShape = &shape;
+    args.fAntiAlias = true;
+    args.fGammaCorrect = false;
+    pr->drawPath(args);
+}
+
+DEF_GPUTEST_FOR_ALL_GL_CONTEXTS(AADistanceFieldPathRenderer, reporter, ctxInfo) {
+    // The DF PR only works with contexts that support derivatives
+    if (!ctxInfo.grContext()->caps()->shaderCaps()->shaderDerivativeSupport()) {
+        return;
+    }
+    sk_sp<GrDrawContext> drawContext(ctxInfo.grContext()->newDrawContext(SkBackingFit::kApprox,
+                                                                         800, 800,
+                                                                         kSkia8888_GrPixelConfig,
+                                                                         0,
+                                                                         kTopLeft_GrSurfaceOrigin));
+    if (!drawContext) {
+        return;
+    }
+
+    GrAADistanceFieldPathRenderer dfpr;
+    GrTestTarget tt;
+    ctxInfo.grContext()->getTestTarget(&tt, drawContext);
+    GrResourceProvider* rp = tt.resourceProvider();
+
+    test_far_from_origin(drawContext.get(), &dfpr, rp);
+    ctxInfo.grContext()->flush();
+}
+#endif