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
diff --git a/bench/DrawLatticeBench.cpp b/bench/DrawLatticeBench.cpp
index 9c4d31a..4c03ca1 100644
--- a/bench/DrawLatticeBench.cpp
+++ b/bench/DrawLatticeBench.cpp
@@ -29,10 +29,6 @@
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 @@
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 77a3443..6e98404 100644
--- a/include/gpu/GrDrawContext.h
+++ b/include/gpu/GrDrawContext.h
@@ -35,7 +35,6 @@
class SkDrawFilter;
struct SkIPoint;
struct SkIRect;
-class SkLatticeIter;
class SkMatrix;
class SkPaint;
class SkPath;
@@ -232,15 +231,26 @@
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 1c06bf3..1fe2c38 100644
--- a/include/private/SkTArray.h
+++ b/include/private/SkTArray.h
@@ -238,19 +238,6 @@
}
/**
- * 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 ba6ac97..24ab3f1 100644
--- a/src/core/SkLatticeIter.cpp
+++ b/src/core/SkLatticeIter.cpp
@@ -159,7 +159,6 @@
fCurrX = fCurrY = 0;
fDone = false;
- fNumRects = (xCount + 1) * (yCount + 1);
}
bool SkLatticeIter::Valid(int width, int height, const SkIRect& center) {
@@ -206,7 +205,6 @@
fCurrX = fCurrY = 0;
fDone = false;
- fNumRects = 9;
}
bool SkLatticeIter::next(SkRect* src, SkRect* dst) {
@@ -230,18 +228,3 @@
}
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 192b6c5..e2d767f 100644
--- a/src/core/SkLatticeIter.h
+++ b/src/core/SkLatticeIter.h
@@ -35,28 +35,15 @@
*/
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 db14498..202be23 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 @@
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 040833b..376d5e1 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1409,10 +1409,8 @@
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 @@
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 d5fe04f..2185f16 100644
--- a/src/gpu/SkGpuDevice.h
+++ b/src/gpu/SkGpuDevice.h
@@ -122,11 +122,6 @@
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 @@
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 729d27e..cde3d26 100644
--- a/src/gpu/batches/GrNinePatch.cpp
+++ b/src/gpu/batches/GrNinePatch.cpp
@@ -29,14 +29,15 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 02664c6..0a4ffd6 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