Revert of Batched implementation of drawLattice() for GPU (patchset #7 id:180001...
authormsarett <msarett@google.com>
Thu, 18 Aug 2016 21:29:22 +0000 (14:29 -0700)
committerCommit bot <commit-bot@chromium.org>
Thu, 18 Aug 2016 21:29:22 +0000 (14:29 -0700)
Reason for revert:
Things drawing weird.

Original issue's description:
> Batched implementation of drawLattice() for GPU
>
> Bechmarks (Nexus 6P):
>
> Src=100x100, Dst=250x250, NumRects=9
> Android              77.7us
> Skia (without patch) 57.2us
> Skia (with patch)    34.7us
>
> Src=100x100, Dst=500x500, NumRects=9
> Android              77.0us
> Skia (without patch) 56.9us
> Skia (with patch)    44.5us
>
> Src=100x100, Dst=1000x1000, NumRects=9
> Android              180us
> Skia (without patch) 96.8us
> Skia (with patch)    70.5us
>
> Src=100x100, Dst=250x250, NumRects=15
> Android              208us
> Skia (without patch) 155us
> Skia (with patch)    55.9us
>
> Src=100x100, Dst=500x500, NumRects=15
> Android              207us
> Skia (without patch) 152us
> Skia (with patch)    63.0us
>
> Src=100x100, Dst=1000x1000, NumRects=15
> Android              233us
> Skia (without patch) 156us
> Skia (with patch)    99.9us
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255963002
>
> Committed: https://skia.googlesource.com/skia/+/93242c4ae50dfcc0d922cdb3ba80bbc7b4bbe93d

TBR=bsalomon@google.com,reed@google.com,djsollen@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

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

bench/DrawLatticeBench.cpp
include/gpu/GrDrawContext.h
include/private/SkTArray.h
src/core/SkLatticeIter.cpp
src/core/SkLatticeIter.h
src/gpu/GrDrawContext.cpp
src/gpu/SkGpuDevice.cpp
src/gpu/SkGpuDevice.h
src/gpu/batches/GrNinePatch.cpp
src/gpu/batches/GrNinePatch.h

index 9c4d31a099d1fffe759da7450fac8f89b118c028..4c03ca1dfa020fdf0038800b30c964222b3bf025 100644 (file)
@@ -29,10 +29,6 @@ public:
         return fName.c_str();
     }
 
-    SkIPoint onGetSize() override {
-        return SkIPoint::Make(1000, 1000);
-    }
-
     bool isSuitableFor(Backend backend) override {
         return kRaster_Backend == backend || kGPU_Backend == backend;
     }
@@ -58,17 +54,6 @@ private:
     typedef Benchmark INHERITED;
 };
 
-static int gDivs9[2] = { 25, 75, };
-DEF_BENCH(return new DrawLatticeBench(gDivs9, 2, gDivs9, 2, SkISize::Make(100, 100),
-                                      SkRect::MakeWH(250.0f, 250.0f), "Src100_Dst250_Rects9");)
-DEF_BENCH(return new DrawLatticeBench(gDivs9, 2, gDivs9, 2, SkISize::Make(100, 100),
-                                      SkRect::MakeWH(500.0f, 500.0f), "Src100_Dst500_Rects9");)
-DEF_BENCH(return new DrawLatticeBench(gDivs9, 2, gDivs9, 2, SkISize::Make(100, 100),
-                                      SkRect::MakeWH(1000.0f, 1000.0f), "Src100_Dst1000_Rects9");)
-static int gDivs15[4] = { 15, 45, 55, 85, };
-DEF_BENCH(return new DrawLatticeBench(gDivs15, 4, gDivs15, 4, SkISize::Make(100, 100),
-                                      SkRect::MakeWH(250.0f, 250.0f), "Src100_Dst250_Rects15");)
-DEF_BENCH(return new DrawLatticeBench(gDivs15, 4, gDivs15, 4, SkISize::Make(100, 100),
-                                      SkRect::MakeWH(500.0f, 500.0f), "Src100_Dst500_Rects15");)
-DEF_BENCH(return new DrawLatticeBench(gDivs15, 4, gDivs15, 4, SkISize::Make(100, 100),
-                                      SkRect::MakeWH(1000.0f, 1000.0f), "Src100_Dst1000_Rects15");)
+static int gDivs[2] = { 250, 750, };
+DEF_BENCH(return new DrawLatticeBench(gDivs, 2, gDivs, 2, SkISize::Make(1000, 1000),
+                                      SkRect::MakeWH(4000.0f, 4000.0f), "StandardNine");)
index 77a3443f9e514dc8abaf8955a9cae8d36b23cb5d..6e98404a44bd6751c2ef03281e96ccbf1ee0e498 100644 (file)
@@ -35,7 +35,6 @@ struct GrUserStencilSettings;
 class SkDrawFilter;
 struct SkIPoint;
 struct SkIRect;
-class SkLatticeIter;
 class SkMatrix;
 class SkPaint;
 class SkPath;
@@ -232,15 +231,26 @@ public:
                   const GrStyle& style);
 
     /**
-     *  Draw the image as a set of rects, specified by |iter|.
+     *  Draw the image stretched differentially to fit into dst.
+     *  center is a rect within the image, and logically divides the image
+     *  into 9 sections (3x3). For example, if the middle pixel of a [5x5]
+     *  image is the "center", then the center-rect should be [2, 2, 3, 3].
+     *
+     *  If the dst is >= the image size, then...
+     *  - The 4 corners are not stretched at all.
+     *  - The sides are stretched in only one axis.
+     *  - The center is stretched in both axes.
+     * Else, for each axis where dst < image,
+     *  - The corners shrink proportionally
+     *  - The sides (along the shrink axis) and center are not drawn
      */
-    void drawImageLattice(const GrClip&,
-                          const GrPaint& paint,
-                          const SkMatrix& viewMatrix,
-                          int imageWidth,
-                          int imageHeight,
-                          std::unique_ptr<SkLatticeIter> iter,
-                          const SkRect& dst);
+    void drawImageNine(const GrClip&,
+                       const GrPaint& paint,
+                       const SkMatrix& viewMatrix,
+                       int imageWidth,
+                       int imageHeight,
+                       const SkIRect& center,
+                       const SkRect& dst);
 
     /**
      * After this returns any pending surface IO will be issued to the backend 3D API and
index 1c06bf331ae8e92f3efcdb54b60fd3af3572443e..1fe2c3857c1555c58efe20303709dbde649ee868 100644 (file)
@@ -237,19 +237,6 @@ public:
         return fItemArray + fCount - n;
     }
 
-    /**
-     * Version of above that uses the move constructor to set n items.
-     */
-    T* move_back_n(int n, T* t) {
-        SkASSERT(n >= 0);
-        this->checkRealloc(n);
-        for (int i = 0; i < n; ++i) {
-            new (fItemArray + fCount + i) T(std::move(t[i]));
-        }
-        fCount += n;
-        return fItemArray + fCount - n;
-    }
-
     /**
      * Removes the last element. Not safe to call when count() == 0.
      */
index ba6ac97a5b4395807a22c482b05e95ccc985eb0c..24ab3f1cbc29a11af6ded16de3965c93ac76b292 100644 (file)
@@ -159,7 +159,6 @@ SkLatticeIter::SkLatticeIter(int srcWidth, int srcHeight, const SkCanvas::Lattic
 
     fCurrX = fCurrY = 0;
     fDone = false;
-    fNumRects = (xCount + 1) * (yCount + 1);
 }
 
 bool SkLatticeIter::Valid(int width, int height, const SkIRect& center) {
@@ -206,7 +205,6 @@ SkLatticeIter::SkLatticeIter(int w, int h, const SkIRect& c, const SkRect& dst)
 
     fCurrX = fCurrY = 0;
     fDone = false;
-    fNumRects = 9;
 }
 
 bool SkLatticeIter::next(SkRect* src, SkRect* dst) {
@@ -230,18 +228,3 @@ bool SkLatticeIter::next(SkRect* src, SkRect* dst) {
     }
     return true;
 }
-
-void SkLatticeIter::mapDstScaleTranslate(const SkMatrix& matrix) {
-    SkASSERT(matrix.isScaleTranslate());
-    SkScalar tx = matrix.getTranslateX();
-    SkScalar sx = matrix.getScaleX();
-    for (int i = 0; i < fDstX.count(); i++) {
-        fDstX[i] = fDstX[i] * sx + tx;
-    }
-
-    SkScalar ty = matrix.getTranslateY();
-    SkScalar sy = matrix.getScaleY();
-    for (int i = 0; i < fDstY.count(); i++) {
-        fDstY[i] = fDstY[i] * sy + ty;
-    }
-}
index 192b6c5b54e49f1d4d96cefd18821e7ed250c278..e2d767f1c8a5383d2f6d2e1b0bb115129a35156c 100644 (file)
@@ -35,28 +35,15 @@ public:
      */
     bool next(SkRect* src, SkRect* dst);
 
-    /**
-     *  Apply a matrix to the dst points.
-     */
-    void mapDstScaleTranslate(const SkMatrix& matrix);
-
-    /**
-     *  Returns the total number of rects that will be drawn.
-     */
-    int numRects() const {
-        return fNumRects;
-    }
-
 private:
     SkTArray<SkScalar> fSrcX;
     SkTArray<SkScalar> fSrcY;
     SkTArray<SkScalar> fDstX;
     SkTArray<SkScalar> fDstY;
 
-    int  fCurrX;
-    int  fCurrY;
+    int fCurrX;
+    int fCurrY;
     bool fDone;
-    int  fNumRects;
 };
 
 #endif
index db144989621e7290c24ca00c07f3d482dae31074..202be23590eb2a15540469e31a2fbd2a43e9efbc 100644 (file)
@@ -35,8 +35,6 @@
 
 #include "../private/GrAuditTrail.h"
 
-#include "SkLatticeIter.h"
-
 #define ASSERT_OWNED_RESOURCE(R) SkASSERT(!(R) || (R)->getContext() == fDrawingManager->getContext())
 #define ASSERT_SINGLE_OWNER \
     SkDEBUGCODE(GrSingleOwner::AutoEnforce debug_SingleOwner(fSingleOwner);)
@@ -1001,23 +999,23 @@ void GrDrawContext::drawOval(const GrClip& clip,
     this->internalDrawPath(clip, paint, viewMatrix, path, style);
 }
 
-void GrDrawContext::drawImageLattice(const GrClip& clip,
-                                     const GrPaint& paint,
-                                     const SkMatrix& viewMatrix,
-                                     int imageWidth,
-                                     int imageHeight,
-                                     std::unique_ptr<SkLatticeIter> iter,
-                                     const SkRect& dst) {
+void GrDrawContext::drawImageNine(const GrClip& clip,
+                                  const GrPaint& paint,
+                                  const SkMatrix& viewMatrix,
+                                  int imageWidth,
+                                  int imageHeight,
+                                  const SkIRect& center,
+                                  const SkRect& dst) {
     ASSERT_SINGLE_OWNER
     RETURN_IF_ABANDONED
     SkDEBUGCODE(this->validate();)
-    GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::drawImageLattice");
+    GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::drawImageNine");
 
     AutoCheckFlush acf(fDrawingManager);
 
     SkAutoTUnref<GrDrawBatch> batch(GrNinePatch::CreateNonAA(paint.getColor(), viewMatrix,
                                                              imageWidth, imageHeight,
-                                                             std::move(iter), dst));
+                                                             center, dst));
 
     GrPipelineBuilder pipelineBuilder(paint, this->mustUseHWAA(paint));
     this->getDrawTarget()->drawBatch(pipelineBuilder, this, clip, batch);
index 040833b731d22c359289807e4f9374971425430c..376d5e1b57669789c47d504126db628f06313761 100644 (file)
@@ -1409,10 +1409,8 @@ void SkGpuDevice::drawProducerNine(const SkDraw& draw, GrTextureProducer* produc
         return;
     }
 
-    std::unique_ptr<SkLatticeIter> iter(
-            new SkLatticeIter(producer->width(), producer->height(), center, dst));
-    fDrawContext->drawImageLattice(fClip, grPaint, *draw.fMatrix, producer->width(),
-                                   producer->height(), std::move(iter), dst);
+    fDrawContext->drawImageNine(fClip, grPaint, *draw.fMatrix, producer->width(),
+                                producer->height(), center, dst);
 }
 
 void SkGpuDevice::drawImageNine(const SkDraw& draw, const SkImage* image,
@@ -1442,61 +1440,6 @@ void SkGpuDevice::drawBitmapNine(const SkDraw& draw, const SkBitmap& bitmap, con
     this->drawProducerNine(draw, &maker, center, dst, paint);
 }
 
-void SkGpuDevice::drawProducerLattice(const SkDraw& draw, GrTextureProducer* producer,
-                                      const SkCanvas::Lattice& lattice, const SkRect& dst,
-                                      const SkPaint& paint) {
-    GR_CREATE_TRACE_MARKER_CONTEXT("SkGpuDevice", "drawProducerLattice", fContext);
-
-    CHECK_SHOULD_DRAW(draw);
-
-    static const GrTextureParams::FilterMode kMode = GrTextureParams::kNone_FilterMode;
-    sk_sp<GrFragmentProcessor> fp(
-        producer->createFragmentProcessor(SkMatrix::I(),
-                                          SkRect::MakeIWH(producer->width(), producer->height()),
-                                          GrTextureProducer::kNo_FilterConstraint, true,
-                                          &kMode, fDrawContext->getColorSpace(),
-                                          fDrawContext->sourceGammaTreatment()));
-    GrPaint grPaint;
-    if (!SkPaintToGrPaintWithTexture(this->context(), fDrawContext.get(), paint, *draw.fMatrix,
-                                     std::move(fp), producer->isAlphaOnly(), &grPaint)) {
-        return;
-    }
-
-    std::unique_ptr<SkLatticeIter> iter(
-            new SkLatticeIter(producer->width(), producer->height(), lattice, dst));
-    fDrawContext->drawImageLattice(fClip, grPaint, *draw.fMatrix, producer->width(),
-                                   producer->height(), std::move(iter), dst);
-}
-
-void SkGpuDevice::drawImageLattice(const SkDraw& draw, const SkImage* image,
-                                   const SkCanvas::Lattice& lattice, const SkRect& dst,
-                                   const SkPaint& paint) {
-    ASSERT_SINGLE_OWNER
-    uint32_t pinnedUniqueID;
-    if (sk_sp<GrTexture> tex = as_IB(image)->refPinnedTexture(&pinnedUniqueID)) {
-        CHECK_SHOULD_DRAW(draw);
-        GrTextureAdjuster adjuster(tex.get(), image->alphaType(), image->bounds(), pinnedUniqueID,
-                                   as_IB(image)->onImageInfo().colorSpace());
-        this->drawProducerLattice(draw, &adjuster, lattice, dst, paint);
-    } else {
-        SkBitmap bm;
-        if (SkImageCacherator* cacher = as_IB(image)->peekCacherator()) {
-            GrImageTextureMaker maker(fContext, cacher, image, SkImage::kAllow_CachingHint);
-            this->drawProducerLattice(draw, &maker, lattice, dst, paint);
-        } else if (as_IB(image)->getROPixels(&bm)) {
-            this->drawBitmapLattice(draw, bm, lattice, dst, paint);
-        }
-    }
-}
-
-void SkGpuDevice::drawBitmapLattice(const SkDraw& draw, const SkBitmap& bitmap,
-                                    const SkCanvas::Lattice& lattice, const SkRect& dst,
-                                    const SkPaint& paint) {
-    ASSERT_SINGLE_OWNER
-    GrBitmapTextureMaker maker(fContext, bitmap);
-    this->drawProducerLattice(draw, &maker, lattice, dst, paint);
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 // must be in SkCanvas::VertexMode order
index d5fe04febf7d61b58b068bce36cd5f73f93de2e3..2185f162d789b19bd28fce0fd3dae6c1edbe5b6a 100644 (file)
@@ -122,11 +122,6 @@ public:
     void drawBitmapNine(const SkDraw& draw, const SkBitmap& bitmap, const SkIRect& center,
                         const SkRect& dst, const SkPaint& paint) override;
 
-    void drawImageLattice(const SkDraw&, const SkImage*, const SkCanvas::Lattice&,
-                          const SkRect& dst, const SkPaint&) override;
-    void drawBitmapLattice(const SkDraw&, const SkBitmap&, const SkCanvas::Lattice&,
-                           const SkRect& dst, const SkPaint&) override;
-
     void drawSpecial(const SkDraw&, SkSpecialImage*,
                      int left, int top, const SkPaint& paint) override;
     sk_sp<SkSpecialImage> makeSpecial(const SkBitmap&) override;
@@ -248,9 +243,6 @@ private:
     void drawProducerNine(const SkDraw&, GrTextureProducer*, const SkIRect& center,
                           const SkRect& dst, const SkPaint&);
 
-    void drawProducerLattice(const SkDraw&, GrTextureProducer*, const SkCanvas::Lattice& lattice,
-                             const SkRect& dst, const SkPaint&);
-
     bool drawDashLine(const SkPoint pts[2], const SkPaint& paint);
     void drawStrokedLine(const SkPoint pts[2], const SkDraw&, const SkPaint&);
 
index 729d27ecac268b7d7cf4bec723d2545bc9d29715..cde3d266ca6f26b18ba0e797dbb489c79c09a01f 100644 (file)
@@ -29,14 +29,15 @@ public:
 
     static const int kVertsPerRect = 4;
     static const int kIndicesPerRect = 6;
+    static const int kRectsPerInstance = 9; // We could skip empty rects
 
     GrNonAANinePatchBatch(GrColor color, const SkMatrix& viewMatrix, int imageWidth,
-                          int imageHeight, std::unique_ptr<SkLatticeIter> iter, const SkRect &dst)
+                          int imageHeight, const SkIRect& center, const SkRect &dst)
         : INHERITED(ClassID()) {
         Patch& patch = fPatches.push_back();
         patch.fViewMatrix = viewMatrix;
         patch.fColor = color;
-        patch.fIter = std::move(iter);
+        patch.fCenter = center;
         patch.fDst = dst;
 
         fImageWidth = imageWidth;
@@ -52,9 +53,12 @@ public:
         SkString str;
 
         for (int i = 0; i < fPatches.count(); ++i) {
-            str.appendf("%d: Color: 0x%08x Dst [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
+            str.appendf("%d: Color: 0x%08x Center [L: %d, T: %d, R: %d, B: %d], "
+                        "Dst [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
                         i,
                         fPatches[i].fColor,
+                        fPatches[i].fCenter.fLeft, fPatches[i].fCenter.fTop,
+                        fPatches[i].fCenter.fRight, fPatches[i].fCenter.fBottom,
                         fPatches[i].fDst.fLeft, fPatches[i].fDst.fTop,
                         fPatches[i].fDst.fRight, fPatches[i].fDst.fBottom);
         }
@@ -80,40 +84,35 @@ private:
 
         size_t vertexStride = gp->getVertexStride();
         int patchCnt = fPatches.count();
-        int numRects = 0;
-        for (int i = 0; i < patchCnt; i++) {
-            numRects += fPatches[i].fIter->numRects();
-        }
 
         SkAutoTUnref<const GrBuffer> indexBuffer(
                 target->resourceProvider()->refQuadIndexBuffer());
         InstancedHelper helper;
         void* vertices = helper.init(target, kTriangles_GrPrimitiveType, vertexStride,
                                      indexBuffer, kVertsPerRect,
-                                     kIndicesPerRect, patchCnt * numRects);
+                                     kIndicesPerRect, patchCnt * kRectsPerInstance);
         if (!vertices || !indexBuffer) {
             SkDebugf("Could not allocate vertices\n");
             return;
         }
 
-        intptr_t verts = reinterpret_cast<intptr_t>(vertices);
         for (int i = 0; i < patchCnt; i++) {
-            const Patch& patch = fPatches[i];
+            intptr_t verts = reinterpret_cast<intptr_t>(vertices) +
+                             i * kRectsPerInstance * kVertsPerRect * vertexStride;
 
-            // Apply the view matrix here if it is scale-translate.  Otherwise, we need to
-            // wait until we've created the dst rects.
-            bool isScaleTranslate = patch.fViewMatrix.isScaleTranslate();
-            if (isScaleTranslate) {
-                patch.fIter->mapDstScaleTranslate(patch.fViewMatrix);
-            }
+            const Patch& patch = fPatches[i];
+            SkLatticeIter iter(fImageWidth, fImageHeight, patch.fCenter, patch.fDst);
 
             SkRect srcR, dstR;
-            intptr_t patchVerts = verts;
-            while (patch.fIter->next(&srcR, &dstR)) {
+            while (iter.next(&srcR, &dstR)) {
                 SkPoint* positions = reinterpret_cast<SkPoint*>(verts);
+
                 positions->setRectFan(dstR.fLeft, dstR.fTop,
                                       dstR.fRight, dstR.fBottom, vertexStride);
 
+                SkASSERT(!patch.fViewMatrix.hasPerspective());
+                patch.fViewMatrix.mapPointsWithStride(positions, vertexStride, kVertsPerRect);
+
                 // Setup local coords
                 static const int kLocalOffset = sizeof(SkPoint) + sizeof(GrColor);
                 SkPoint* coords = reinterpret_cast<SkPoint*>(verts + kLocalOffset);
@@ -127,13 +126,6 @@ private:
                 }
                 verts += kVertsPerRect * vertexStride;
             }
-
-            // If we didn't handle it above, apply the matrix here.
-            if (!isScaleTranslate) {
-                SkPoint* positions = reinterpret_cast<SkPoint*>(patchVerts);
-                patch.fViewMatrix.mapPointsWithStride(positions, vertexStride,
-                                                      kVertsPerRect * patch.fIter->numRects());
-            }
         }
         helper.recordDraw(target, gp.get());
     }
@@ -159,14 +151,14 @@ private:
             fOverrides = that->fOverrides;
         }
 
-        fPatches.move_back_n(that->fPatches.count(), that->fPatches.begin());
+        fPatches.push_back_n(that->fPatches.count(), that->fPatches.begin());
         this->joinBounds(*that);
         return true;
     }
 
     struct Patch {
         SkMatrix fViewMatrix;
-        std::unique_ptr<SkLatticeIter> fIter;
+        SkIRect fCenter;
         SkRect fDst;
         GrColor fColor;
     };
@@ -181,8 +173,7 @@ private:
 
 namespace GrNinePatch {
 GrDrawBatch* CreateNonAA(GrColor color, const SkMatrix& viewMatrix, int imageWidth, int imageHeight,
-                         std::unique_ptr<SkLatticeIter> iter, const SkRect& dst) {
-    return new GrNonAANinePatchBatch(color, viewMatrix, imageWidth, imageHeight, std::move(iter),
-                                     dst);
+                         const SkIRect& center, const SkRect& dst) {
+    return new GrNonAANinePatchBatch(color, viewMatrix, imageWidth, imageHeight, center, dst);
 }
 };
index 02664c6449e8e38fff7ff3a55f8b5eccc9bc4100..0a4ffd6e03aaae6b3e40335152b0ba4c9579e86b 100644 (file)
@@ -9,18 +9,16 @@
 #define GrNinePatch_DEFINED
 
 #include "GrColor.h"
-#include "SkCanvas.h"
 
 class GrDrawBatch;
 class SkBitmap;
-class SkLatticeIter;
 class SkMatrix;
 struct SkIRect;
 struct SkRect;
 
 namespace GrNinePatch {
 GrDrawBatch* CreateNonAA(GrColor color, const SkMatrix& viewMatrix, int imageWidth, int imageHeight,
-                         std::unique_ptr<SkLatticeIter> iter, const SkRect& dst);
+                         const SkIRect& center, const SkRect& dst);
 };
 
 #endif