Reland "Refactor trimming logic for read/writePixels()"
Original CL: https://skia-review.googlesource.com/c/7326/
(1) Move trimming logic into Bitmap/Pixmap level for
raster. Everything goes through here, so we'll
only do the work once.
(2) This means it also goes to GPU level.
(3) Always use SkReadPixelsRec rather than inlining
the logic.
(4) Create an SkWritePixelsRec to encapsulate write
trimming.
(5) Disabled kIndex8 as a dst - always.
CQ_INCLUDE_TRYBOTS=skia.primary:Perf-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Debug
BUG=skia:6021
Change-Id: I25a964e3c610c4e36d195a255e2150657baec649
Reviewed-on: https://skia-review.googlesource.com/7404
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index d57b62f..7596c3a 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -30,7 +30,6 @@
#include "SkRadialShadowMapShader.h"
#include "SkRasterClip.h"
#include "SkRasterHandleAllocator.h"
-#include "SkReadPixelsRec.h"
#include "SkRRect.h"
#include "SkShadowPaintFilterCanvas.h"
#include "SkShadowShader.h"
@@ -858,10 +857,6 @@
}
bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) {
- if (kUnknown_SkColorType == bitmap->colorType()) {
- return false;
- }
-
bool weAllocated = false;
if (nullptr == bitmap->pixelRef()) {
if (!bitmap->tryAllocPixels()) {
@@ -908,15 +903,8 @@
if (!device) {
return false;
}
- const SkISize size = this->getBaseLayerSize();
- SkReadPixelsRec rec(dstInfo, dstP, rowBytes, x, y);
- if (!rec.trim(size.width(), size.height())) {
- return false;
- }
-
- // The device can assert that the requested area is always contained in its bounds
- return device->readPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
+ return device->readPixels(dstInfo, dstP, rowBytes, x, y);
}
bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
@@ -928,49 +916,30 @@
return false;
}
-bool SkCanvas::writePixels(const SkImageInfo& origInfo, const void* pixels, size_t rowBytes,
+bool SkCanvas::writePixels(const SkImageInfo& srcInfo, const void* pixels, size_t rowBytes,
int x, int y) {
- switch (origInfo.colorType()) {
- case kUnknown_SkColorType:
- case kIndex_8_SkColorType:
- return false;
- default:
- break;
- }
- if (nullptr == pixels || rowBytes < origInfo.minRowBytes()) {
- return false;
- }
-
- const SkISize size = this->getBaseLayerSize();
- SkIRect target = SkIRect::MakeXYWH(x, y, origInfo.width(), origInfo.height());
- if (!target.intersect(0, 0, size.width(), size.height())) {
- return false;
- }
-
SkBaseDevice* device = this->getDevice();
if (!device) {
return false;
}
- // the intersect may have shrunk info's logical size
- const SkImageInfo info = origInfo.makeWH(target.width(), target.height());
-
- // if x or y are negative, then we have to adjust pixels
- if (x > 0) {
- x = 0;
+ // This check gives us an early out and prevents generation ID churn on the surface.
+ // This is purely optional: it is a subset of the checks performed by SkWritePixelsRec.
+ SkIRect srcRect = SkIRect::MakeXYWH(x, y, srcInfo.width(), srcInfo.height());
+ if (!srcRect.intersect(0, 0, device->width(), device->height())) {
+ return false;
}
- if (y > 0) {
- y = 0;
- }
- // here x,y are either 0 or negative
- pixels = ((const char*)pixels - y * rowBytes - x * info.bytesPerPixel());
- // Tell our owning surface to bump its generation ID
- const bool completeOverwrite = info.dimensions() == size;
+ // Tell our owning surface to bump its generation ID.
+ const bool completeOverwrite =
+ srcRect.size() == SkISize::Make(device->width(), device->height());
this->predrawNotify(completeOverwrite);
- // The device can assert that the requested area is always contained in its bounds
- return device->writePixels(info, pixels, rowBytes, target.x(), target.y());
+ // This can still fail, most notably in the case of a invalid color type or alpha type
+ // conversion. We could pull those checks into this function and avoid the unnecessary
+ // generation ID bump. But then we would be performing those checks twice, since they
+ // are also necessary at the bitmap/pixmap entry points.
+ return device->writePixels(srcInfo, pixels, rowBytes, x, y);
}
//////////////////////////////////////////////////////////////////////////////