From 461ef7af88cc966007c464130a971ec86c803f1d Mon Sep 17 00:00:00 2001 From: Florin Malita Date: Wed, 7 Mar 2018 14:22:55 +0000 Subject: Revert "add tiler for SkDraw" This reverts commit be1b3971806e3d80aa9673a36e2b35d0145198ac. Reason for revert: Unexpected layout test diffs: https://test-results.appspot.com/data/layout_results/linux_trusty_blink_rel/24989/layout-test-results/results.html Original change's description: > add tiler for SkDraw > > Bug: skia:2122 > Change-Id: I276de2064939151eef5fa14c53188e8b5728b7c9 > Reviewed-on: https://skia-review.googlesource.com/110840 > Commit-Queue: Mike Reed > Reviewed-by: Yuqian Li TBR=liyuqian@google.com,reed@google.com Change-Id: Ia598c0d7c4ac6cfcdb905b847040c250fa366402 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:2122 Reviewed-on: https://skia-review.googlesource.com/112740 Reviewed-by: Florin Malita Commit-Queue: Florin Malita --- src/core/SkBitmapDevice.cpp | 160 ++++++++----------------------------------- src/core/SkBitmapDevice.h | 1 - src/core/SkDeviceLooper.cpp | 126 ++++++++++++++++++++++++++++++++++ src/core/SkDeviceLooper.h | 95 +++++++++++++++++++++++++ src/core/SkDraw.cpp | 74 +++++++++++--------- src/core/SkScan_Hairline.cpp | 10 +-- 6 files changed, 293 insertions(+), 173 deletions(-) create mode 100644 src/core/SkDeviceLooper.cpp create mode 100644 src/core/SkDeviceLooper.h (limited to 'src') diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index 219724451a..3d66ba6dfc 100644 --- a/src/core/SkBitmapDevice.cpp +++ b/src/core/SkBitmapDevice.cpp @@ -23,116 +23,6 @@ #include "SkTLazy.h" #include "SkVertices.h" -class SkDrawTiler { - enum { - // 8K is 1 too big, since 8K << supersample == 32768 which is too big for SkFixed - kMaxDim = 8192 - 1 - }; - - SkBitmapDevice* fDevice; - SkPixmap fRootPixmap; - - // Used for tiling and non-tiling - SkDraw fDraw; - - // fCurr... are only used if fNeedTiling - SkMatrix fTileMatrix; - SkRasterClip fTileRC; - SkIPoint fCurrOrigin, fOrigin; - - bool fDone, fNeedsTiling; - -public: - SkDrawTiler(SkBitmapDevice* dev) : fDevice(dev) { - // we need fDst to be set, and if we're actually drawing, to dirty the genID - if (!dev->accessPixels(&fRootPixmap)) { - // NoDrawDevice uses us (why?) so we have to catch this case w/ no pixels - fRootPixmap.reset(dev->imageInfo(), nullptr, 0); - } - - fDone = false; - fNeedsTiling = fRootPixmap.width() > kMaxDim || fRootPixmap.height() > kMaxDim; - fOrigin.set(0, 0); - fCurrOrigin = fOrigin; - - if (fNeedsTiling) { - // fDraw.fDst is reset each time in setupTileDraw() - fDraw.fMatrix = &fTileMatrix; - fDraw.fRC = &fTileRC; - } else { - fDraw.fDst = fRootPixmap; - fDraw.fMatrix = &dev->ctm(); - fDraw.fRC = &dev->fRCStack.rc(); - } - } - - bool needsTiling() const { return fNeedsTiling; } - - const SkDraw* next() { - if (fDone) { - return nullptr; - } - if (fNeedsTiling) { - do { - this->setupTileDraw(); // might set the clip to empty - this->stepOrigin(); // might set fDone to true - } while (!fDone && fTileRC.isEmpty()); - // if we exit the loop and we're still empty, we're (past) done - if (fTileRC.isEmpty()) { - SkASSERT(fDone); - return nullptr; - } - SkASSERT(!fTileRC.isEmpty()); - } else { - fDone = true; // only draw untiled once - } - return &fDraw; - } - - int curr_x() const { return fCurrOrigin.x(); } - int curr_y() const { return fCurrOrigin.y(); } - -private: - void setupTileDraw() { - SkASSERT(!fDone); - SkIRect bounds = SkIRect::MakeXYWH(fOrigin.x(), fOrigin.y(), kMaxDim, kMaxDim); - SkASSERT(!bounds.isEmpty()); - bool success = fRootPixmap.extractSubset(&fDraw.fDst, bounds); - SkASSERT_RELEASE(success); - // now don't use bounds, since fDst has the clipped dimensions. - - fTileMatrix = fDevice->ctm(); - fTileMatrix.postTranslate(SkIntToScalar(-fOrigin.x()), SkIntToScalar(-fOrigin.y())); - fDevice->fRCStack.rc().translate(-fOrigin.x(), -fOrigin.y(), &fTileRC); - fTileRC.op(SkIRect::MakeWH(fDraw.fDst.width(), fDraw.fDst.height()), - SkRegion::kIntersect_Op); - - fCurrOrigin = fOrigin; - } - - void stepOrigin() { - SkASSERT(!fDone); - SkASSERT(fNeedsTiling); - fOrigin.fX += kMaxDim; - if (fOrigin.fX >= fRootPixmap.width()) { // too far - fOrigin.fX = 0; - fOrigin.fY += kMaxDim; - if (fOrigin.fY >= fRootPixmap.height()) { - fDone = true; // way too far - } - } - } -}; - -#define LOOP_TILER(code) \ - SkDrawTiler priv_tiler(this); \ - while (const SkDraw* priv_draw = priv_tiler.next()) { \ - priv_draw->code; \ - } -#define TILER_X(x) (x) - priv_tiler.curr_x() -#define TILER_Y(y) (y) - priv_tiler.curr_y() - - class SkColorTable; static bool valid_for_bitmap_device(const SkImageInfo& info, @@ -283,17 +173,30 @@ bool SkBitmapDevice::onReadPixels(const SkPixmap& pm, int x, int y) { /////////////////////////////////////////////////////////////////////////////// +class SkBitmapDevice::BDDraw : public SkDraw { +public: + BDDraw(SkBitmapDevice* dev) { + // we need fDst to be set, and if we're actually drawing, to dirty the genID + if (!dev->accessPixels(&fDst)) { + // NoDrawDevice uses us (why?) so we have to catch this case w/ no pixels + fDst.reset(dev->imageInfo(), nullptr, 0); + } + fMatrix = &dev->ctm(); + fRC = &dev->fRCStack.rc(); + } +}; + void SkBitmapDevice::drawPaint(const SkPaint& paint) { - LOOP_TILER( drawPaint(paint)) + BDDraw(this).drawPaint(paint); } void SkBitmapDevice::drawPoints(SkCanvas::PointMode mode, size_t count, const SkPoint pts[], const SkPaint& paint) { - LOOP_TILER( drawPoints(mode, count, pts, paint, nullptr)) + BDDraw(this).drawPoints(mode, count, pts, paint, nullptr); } void SkBitmapDevice::drawRect(const SkRect& r, const SkPaint& paint) { - LOOP_TILER( drawRect(r, paint)) + BDDraw(this).drawRect(r, paint); } void SkBitmapDevice::drawOval(const SkRect& oval, const SkPaint& paint) { @@ -313,27 +216,21 @@ void SkBitmapDevice::drawRRect(const SkRRect& rrect, const SkPaint& paint) { // required to override drawRRect. this->drawPath(path, paint, nullptr, true); #else - LOOP_TILER( drawRRect(rrect, paint)) + BDDraw(this).drawRRect(rrect, paint); #endif } void SkBitmapDevice::drawPath(const SkPath& path, const SkPaint& paint, const SkMatrix* prePathMatrix, bool pathIsMutable) { - SkDrawTiler tiler(this); - if (tiler.needsTiling()) { - pathIsMutable = false; - } - while (const SkDraw* draw = tiler.next()) { - draw->drawPath(path, paint, prePathMatrix, pathIsMutable); - } + BDDraw(this).drawPath(path, paint, prePathMatrix, pathIsMutable); } void SkBitmapDevice::drawBitmap(const SkBitmap& bitmap, SkScalar x, SkScalar y, const SkPaint& paint) { SkMatrix matrix = SkMatrix::MakeTrans(x, y); LogDrawScaleFactor(SkMatrix::Concat(this->ctm(), matrix), paint.getFilterQuality()); - LOOP_TILER( drawBitmap(bitmap, matrix, nullptr, paint)) + BDDraw(this).drawBitmap(bitmap, matrix, nullptr, paint); } static inline bool CanApplyDstMatrixAsCTM(const SkMatrix& m, const SkPaint& paint) { @@ -428,7 +325,7 @@ void SkBitmapDevice::drawBitmapRect(const SkBitmap& bitmap, // matrix with the CTM, and try to call drawSprite if it can. If not, // it will make a shader and call drawRect, as we do below. if (CanApplyDstMatrixAsCTM(matrix, paint)) { - LOOP_TILER( drawBitmap(*bitmapPtr, matrix, dstPtr, paint)) + BDDraw(this).drawBitmap(*bitmapPtr, matrix, dstPtr, paint); return; } } @@ -457,25 +354,25 @@ void SkBitmapDevice::drawBitmapRect(const SkBitmap& bitmap, } void SkBitmapDevice::drawSprite(const SkBitmap& bitmap, int x, int y, const SkPaint& paint) { - LOOP_TILER( drawSprite(bitmap, TILER_X(x), TILER_Y(y), paint)) + BDDraw(this).drawSprite(bitmap, x, y, paint); } void SkBitmapDevice::drawText(const void* text, size_t len, SkScalar x, SkScalar y, const SkPaint& paint) { - LOOP_TILER( drawText((const char*)text, len, x, y, paint, &fSurfaceProps)) + BDDraw(this).drawText((const char*)text, len, x, y, paint, &fSurfaceProps); } void SkBitmapDevice::drawPosText(const void* text, size_t len, const SkScalar xpos[], int scalarsPerPos, const SkPoint& offset, const SkPaint& paint) { - LOOP_TILER( drawPosText((const char*)text, len, xpos, scalarsPerPos, offset, paint, - &fSurfaceProps)) + BDDraw(this).drawPosText((const char*)text, len, xpos, scalarsPerPos, offset, paint, + &fSurfaceProps); } void SkBitmapDevice::drawVertices(const SkVertices* vertices, SkBlendMode bmode, const SkPaint& paint) { - LOOP_TILER( drawVertices(vertices->mode(), vertices->vertexCount(), vertices->positions(), - vertices->texCoords(), vertices->colors(), bmode, - vertices->indices(), vertices->indexCount(), paint)) + BDDraw(this).drawVertices(vertices->mode(), vertices->vertexCount(), vertices->positions(), + vertices->texCoords(), vertices->colors(), bmode, + vertices->indices(), vertices->indexCount(), paint); } void SkBitmapDevice::drawDevice(SkBaseDevice* device, int x, int y, const SkPaint& origPaint) { @@ -487,8 +384,7 @@ void SkBitmapDevice::drawDevice(SkBaseDevice* device, int x, int y, const SkPain paint.writable()->setMaskFilter(paint->getMaskFilter()->makeWithLocalMatrix(this->ctm())); } - LOOP_TILER( drawSprite(static_cast(device)->fBitmap, - TILER_X(x), TILER_Y(y), *paint)) + BDDraw(this).drawSprite(static_cast(device)->fBitmap, x, y, *paint); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkBitmapDevice.h b/src/core/SkBitmapDevice.h index dbe04fa1cc..b890066da6 100644 --- a/src/core/SkBitmapDevice.h +++ b/src/core/SkBitmapDevice.h @@ -142,7 +142,6 @@ private: friend struct DeviceCM; //for setMatrixClip friend class SkDraw; friend class SkDrawIter; - friend class SkDrawTiler; friend class SkDeviceFilteredPaint; friend class SkSurface_Raster; friend class SkThreadedBMPDevice; // to copy fRCStack diff --git a/src/core/SkDeviceLooper.cpp b/src/core/SkDeviceLooper.cpp new file mode 100644 index 0000000000..cea92704ef --- /dev/null +++ b/src/core/SkDeviceLooper.cpp @@ -0,0 +1,126 @@ +/* + * Copyright 2013 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkDeviceLooper.h" + +SkDeviceLooper::SkDeviceLooper(const SkPixmap& base, const SkRasterClip& rc, const SkIRect& bounds, + bool aa) + : fBaseDst(base) + , fBaseRC(rc) + , fDelta(aa ? kAA_Delta : kBW_Delta) +{ + // sentinels that next() has not yet been called, and so our mapper functions + // should not be called either. + fCurrDst = nullptr; + fCurrRC = nullptr; + + if (!rc.isEmpty()) { + // clip must be contained by the bitmap + SkASSERT(SkIRect::MakeWH(base.width(), base.height()).contains(rc.getBounds())); + } + + if (rc.isEmpty() || !fClippedBounds.intersect(bounds, rc.getBounds())) { + fState = kDone_State; + } else if (this->fitsInDelta(fClippedBounds)) { + fState = kSimple_State; + } else { + // back up by 1 DX, so that next() will put us in a correct starting + // position. + fCurrOffset.set(fClippedBounds.left() - fDelta, + fClippedBounds.top()); + fState = kComplex_State; + } +} + +SkDeviceLooper::~SkDeviceLooper() {} + +void SkDeviceLooper::mapRect(SkRect* dst, const SkRect& src) const { + SkASSERT(kDone_State != fState); + SkASSERT(fCurrDst); + SkASSERT(fCurrRC); + + *dst = src; + dst->offset(SkIntToScalar(-fCurrOffset.fX), + SkIntToScalar(-fCurrOffset.fY)); +} + +void SkDeviceLooper::mapMatrix(SkMatrix* dst, const SkMatrix& src) const { + SkASSERT(kDone_State != fState); + SkASSERT(fCurrDst); + SkASSERT(fCurrRC); + + *dst = src; + dst->postTranslate(SkIntToScalar(-fCurrOffset.fX), SkIntToScalar(-fCurrOffset.fY)); +} + +bool SkDeviceLooper::computeCurrBitmapAndClip() { + SkASSERT(kComplex_State == fState); + + SkIRect r = SkIRect::MakeXYWH(fCurrOffset.x(), fCurrOffset.y(), + fDelta, fDelta); + if (!fBaseDst.extractSubset(&fSubsetDst, r)) { + fSubsetRC.setEmpty(); + } else { + fBaseRC.translate(-r.left(), -r.top(), &fSubsetRC); + (void)fSubsetRC.op(SkIRect::MakeWH(fDelta, fDelta), SkRegion::kIntersect_Op); + } + + fCurrDst = &fSubsetDst; + fCurrRC = &fSubsetRC; + return !fCurrRC->isEmpty(); +} + +static bool next_tile(const SkIRect& boundary, int delta, SkIPoint* offset) { + // can we move to the right? + if (offset->x() + delta < boundary.right()) { + offset->fX += delta; + return true; + } + + // reset to the left, but move down a row + offset->fX = boundary.left(); + if (offset->y() + delta < boundary.bottom()) { + offset->fY += delta; + return true; + } + + // offset is now outside of boundary, so we're done + return false; +} + +bool SkDeviceLooper::next() { + switch (fState) { + case kDone_State: + // in theory, we should not get called here, since we must have + // previously returned false, but we check anyway. + break; + + case kSimple_State: + // first time for simple + if (nullptr == fCurrDst) { + fCurrDst = &fBaseDst; + fCurrRC = &fBaseRC; + fCurrOffset.set(0, 0); + return true; + } + // 2nd time for simple, we are done + break; + + case kComplex_State: + // need to propogate fCurrOffset through clippedbounds + // left to right, until we wrap around and move down + + while (next_tile(fClippedBounds, fDelta, &fCurrOffset)) { + if (this->computeCurrBitmapAndClip()) { + return true; + } + } + break; + } + fState = kDone_State; + return false; +} diff --git a/src/core/SkDeviceLooper.h b/src/core/SkDeviceLooper.h new file mode 100644 index 0000000000..dd346d7445 --- /dev/null +++ b/src/core/SkDeviceLooper.h @@ -0,0 +1,95 @@ +/* + * Copyright 2013 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkDeviceLooper_DEFINED +#define SkDeviceLooper_DEFINED + +#include "SkBitmap.h" +#include "SkMatrix.h" +#include "SkRasterClip.h" + +/** + * Helper class to manage "tiling" a large coordinate space into managable + * chunks, where managable means areas that are <= some max critical coordinate + * size. + * + * The constructor takes an antialiasing bool, which affects what this maximum + * allowable size is: If we're drawing BW, then we need coordinates to stay + * safely within fixed-point range (we use +- 16K, to give ourselves room to + * add/subtract two fixed values and still be in range. If we're drawing AA, + * then we reduce that size by the amount that the supersampler scan converter + * needs (at the moment, that is 4X, so the "safe" range is +- 4K). + * + * For performance reasons, the class first checks to see if any help is needed + * at all, and if not (i.e. the specified bounds and base bitmap area already + * in the safe-zone, then the class does nothing (effectively). + */ +class SkDeviceLooper { +public: + SkDeviceLooper(const SkPixmap& base, const SkRasterClip&, const SkIRect& bounds, bool aa); + ~SkDeviceLooper(); + + const SkPixmap& getPixmap() const { + SkASSERT(kDone_State != fState); + SkASSERT(fCurrDst); + return *fCurrDst; + } + + const SkRasterClip& getRC() const { + SkASSERT(kDone_State != fState); + SkASSERT(fCurrRC); + return *fCurrRC; + } + + void mapRect(SkRect* dst, const SkRect& src) const; + void mapMatrix(SkMatrix* dst, const SkMatrix& src) const; + + /** + * Call next to setup the looper to return a valid coordinate chunk. + * Each time this returns true, it is safe to call mapRect() and + * mapMatrix(), to convert from "global" coordinate values to ones that + * are local to this chunk. + * + * When next() returns false, the list of chunks is done, and mapRect() + * and mapMatrix() should no longer be called. + */ + bool next(); + +private: + const SkPixmap& fBaseDst; + const SkRasterClip& fBaseRC; + + enum State { + kDone_State, // iteration is complete, getters will assert + kSimple_State, // no translate/clip mods needed + kComplex_State + }; + + // storage for our tiled versions. Perhaps could use SkTLazy + SkPixmap fSubsetDst; + SkRasterClip fSubsetRC; + + const SkPixmap* fCurrDst; + const SkRasterClip* fCurrRC; + SkIRect fClippedBounds; + SkIPoint fCurrOffset; + int fDelta; + State fState; + + enum Delta { + kBW_Delta = 1 << 14, // 16K, gives room to spare for fixedpoint + kAA_Delta = kBW_Delta >> 2 // supersample 4x + }; + + bool fitsInDelta(const SkIRect& r) const { + return r.right() < fDelta && r.bottom() < fDelta; + } + + bool computeCurrBitmapAndClip(); +}; + +#endif diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp index cf06b7b3b5..21a89ae5a9 100644 --- a/src/core/SkDraw.cpp +++ b/src/core/SkDraw.cpp @@ -14,6 +14,7 @@ #include "SkCanvas.h" #include "SkColorData.h" #include "SkDevice.h" +#include "SkDeviceLooper.h" #include "SkDraw.h" #include "SkDrawProcs.h" #include "SkFindAndPlaceGlyph.h" @@ -784,7 +785,8 @@ void SkDraw::drawRect(const SkRect& prePaintRect, const SkPaint& paint, } } - if (!SkRectPriv::FitsInFixed(bbox) && rtype != kHair_RectType) { + if (!SkRectPriv::MakeLargeS32().contains(bbox)) { + // bbox.roundOut() is undefined; use slow path. draw_rect_as_path(*this, prePaintRect, paint, matrix); return; } @@ -794,37 +796,45 @@ void SkDraw::drawRect(const SkRect& prePaintRect, const SkPaint& paint, return; } - SkAutoBlitterChoose blitterStorage(fDst, *matrix, paint); - const SkRasterClip& clip = *fRC; - SkBlitter* blitter = blitterStorage.get(); - - // we want to "fill" if we are kFill or kStrokeAndFill, since in the latter - // case we are also hairline (if we've gotten to here), which devolves to - // effectively just kFill - switch (rtype) { - case kFill_RectType: - if (paint.isAntiAlias()) { - SkScan::AntiFillRect(devRect, clip, blitter); - } else { - SkScan::FillRect(devRect, clip, blitter); - } - break; - case kStroke_RectType: - if (paint.isAntiAlias()) { - SkScan::AntiFrameRect(devRect, strokeSize, clip, blitter); - } else { - SkScan::FrameRect(devRect, strokeSize, clip, blitter); - } - break; - case kHair_RectType: - if (paint.isAntiAlias()) { - SkScan::AntiHairRect(devRect, clip, blitter); - } else { - SkScan::HairRect(devRect, clip, blitter); - } - break; - default: - SkDEBUGFAIL("bad rtype"); + SkDeviceLooper looper(fDst, *fRC, ir, paint.isAntiAlias()); + while (looper.next()) { + SkRect localDevRect; + looper.mapRect(&localDevRect, devRect); + SkMatrix localMatrix; + looper.mapMatrix(&localMatrix, *matrix); + + SkAutoBlitterChoose blitterStorage(looper.getPixmap(), localMatrix, paint); + const SkRasterClip& clip = looper.getRC(); + SkBlitter* blitter = blitterStorage.get(); + + // we want to "fill" if we are kFill or kStrokeAndFill, since in the latter + // case we are also hairline (if we've gotten to here), which devolves to + // effectively just kFill + switch (rtype) { + case kFill_RectType: + if (paint.isAntiAlias()) { + SkScan::AntiFillRect(localDevRect, clip, blitter); + } else { + SkScan::FillRect(localDevRect, clip, blitter); + } + break; + case kStroke_RectType: + if (paint.isAntiAlias()) { + SkScan::AntiFrameRect(localDevRect, strokeSize, clip, blitter); + } else { + SkScan::FrameRect(localDevRect, strokeSize, clip, blitter); + } + break; + case kHair_RectType: + if (paint.isAntiAlias()) { + SkScan::AntiHairRect(localDevRect, clip, blitter); + } else { + SkScan::HairRect(localDevRect, clip, blitter); + } + break; + default: + SkDEBUGFAIL("bad rtype"); + } } } diff --git a/src/core/SkScan_Hairline.cpp b/src/core/SkScan_Hairline.cpp index 65dd397d01..1f12efefc5 100644 --- a/src/core/SkScan_Hairline.cpp +++ b/src/core/SkScan_Hairline.cpp @@ -144,14 +144,8 @@ void SkScan::HairLineRgn(const SkPoint array[], int arrayCount, const SkRegion* // we don't just draw 4 lines, 'cause that can leave a gap in the bottom-right // and double-hit the top-left. -void SkScan::HairRect(const SkRect& origRect, const SkRasterClip& clip, SkBlitter* blitter) { - SkRect rect = origRect; - // clip against a slightly larger clip, to get us into "int" range. Need it slightly - // larger since we're a hairline, and may draw outside of our bounds. - if (!rect.intersect(SkRect::Make(clip.getBounds()).makeOutset(1, 1))) { - return; - } - +void SkScan::HairRect(const SkRect& rect, const SkRasterClip& clip, + SkBlitter* blitter) { SkAAClipBlitterWrapper wrapper; SkBlitterClipper clipper; const SkIRect r = SkIRect::MakeLTRB(SkScalarFloorToInt(rect.fLeft), -- cgit v1.2.3