diff options
author | msarett <msarett@google.com> | 2016-08-18 14:29:22 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-08-18 14:29:22 -0700 |
commit | 7fc08585d0d4daada1c8600b6cdef970ee6c2369 (patch) | |
tree | 0b39ac90f0f21f4496efef50c5fff1aed1f3302c | |
parent | 530032a18e373ee673ae96fdbfa1fae6292f8f08 (diff) |
Revert of Batched implementation of drawLattice() for GPU (patchset #7 id:180001 of https://codereview.chromium.org/2255963002/ )
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
-rw-r--r-- | bench/DrawLatticeBench.cpp | 21 | ||||
-rw-r--r-- | include/gpu/GrDrawContext.h | 28 | ||||
-rw-r--r-- | include/private/SkTArray.h | 13 | ||||
-rw-r--r-- | src/core/SkLatticeIter.cpp | 17 | ||||
-rw-r--r-- | src/core/SkLatticeIter.h | 17 | ||||
-rw-r--r-- | src/gpu/GrDrawContext.cpp | 20 | ||||
-rw-r--r-- | src/gpu/SkGpuDevice.cpp | 61 | ||||
-rw-r--r-- | src/gpu/SkGpuDevice.h | 8 | ||||
-rw-r--r-- | src/gpu/batches/GrNinePatch.cpp | 51 | ||||
-rw-r--r-- | src/gpu/batches/GrNinePatch.h | 4 |
10 files changed, 57 insertions, 183 deletions
diff --git a/bench/DrawLatticeBench.cpp b/bench/DrawLatticeBench.cpp index 9c4d31a099..4c03ca1dfa 100644 --- a/bench/DrawLatticeBench.cpp +++ b/bench/DrawLatticeBench.cpp @@ -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");) diff --git a/include/gpu/GrDrawContext.h b/include/gpu/GrDrawContext.h index 77a3443f9e..6e98404a44 100644 --- a/include/gpu/GrDrawContext.h +++ b/include/gpu/GrDrawContext.h @@ -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 diff --git a/include/private/SkTArray.h b/include/private/SkTArray.h index 1c06bf331a..1fe2c3857c 100644 --- a/include/private/SkTArray.h +++ b/include/private/SkTArray.h @@ -238,19 +238,6 @@ public: } /** - * 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. */ void pop_back() { diff --git a/src/core/SkLatticeIter.cpp b/src/core/SkLatticeIter.cpp index ba6ac97a5b..24ab3f1cbc 100644 --- a/src/core/SkLatticeIter.cpp +++ b/src/core/SkLatticeIter.cpp @@ -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; - } -} diff --git a/src/core/SkLatticeIter.h b/src/core/SkLatticeIter.h index 192b6c5b54..e2d767f1c8 100644 --- a/src/core/SkLatticeIter.h +++ b/src/core/SkLatticeIter.h @@ -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 diff --git a/src/gpu/GrDrawContext.cpp b/src/gpu/GrDrawContext.cpp index db14498962..202be23590 100644 --- a/src/gpu/GrDrawContext.cpp +++ b/src/gpu/GrDrawContext.cpp @@ -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); diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 040833b731..376d5e1b57 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -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 diff --git a/src/gpu/SkGpuDevice.h b/src/gpu/SkGpuDevice.h index d5fe04febf..2185f162d7 100644 --- a/src/gpu/SkGpuDevice.h +++ b/src/gpu/SkGpuDevice.h @@ -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&); diff --git a/src/gpu/batches/GrNinePatch.cpp b/src/gpu/batches/GrNinePatch.cpp index 729d27ecac..cde3d266ca 100644 --- a/src/gpu/batches/GrNinePatch.cpp +++ b/src/gpu/batches/GrNinePatch.cpp @@ -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); } }; diff --git a/src/gpu/batches/GrNinePatch.h b/src/gpu/batches/GrNinePatch.h index 02664c6449..0a4ffd6e03 100644 --- a/src/gpu/batches/GrNinePatch.h +++ b/src/gpu/batches/GrNinePatch.h @@ -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 |