From 1e28e5d79e895e502afddecda2eb66a73a453d38 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Wed, 21 Nov 2018 13:13:29 +0000 Subject: [PATCH] Revert "add rect-parameter to makeImageSnapshot" This reverts commit e195d1c22e4f40dd3c2fa06303291aff5158c30c. Reason for revert: broke android subclass (illegal) Original change's description: > add rect-parameter to makeImageSnapshot > > Bug: skia: > Change-Id: I56044fb1f21b959993d69fc587f95e3dbf30c371 > Reviewed-on: https://skia-review.googlesource.com/c/171905 > Commit-Queue: Mike Reed > Auto-Submit: Mike Reed > Reviewed-by: Brian Salomon TBR=djsollen@google.com,egdaniel@google.com,jvanverth@google.com,bsalomon@google.com,reed@google.com Change-Id: Ied294732b332192e251a845a5cb6349a670c25b0 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia: Reviewed-on: https://skia-review.googlesource.com/c/172301 Reviewed-by: Mike Reed Commit-Queue: Mike Reed --- gm/surface.cpp | 59 ---------------------------------- include/core/SkSurface.h | 11 ------- src/image/SkSurface.cpp | 16 +-------- src/image/SkSurface_Base.h | 5 +-- src/image/SkSurface_Gpu.cpp | 14 +++----- src/image/SkSurface_Gpu.h | 2 +- src/image/SkSurface_Raster.cpp | 13 ++------ 7 files changed, 10 insertions(+), 110 deletions(-) diff --git a/gm/surface.cpp b/gm/surface.cpp index 4d302ac36f..80c23c89a6 100644 --- a/gm/surface.cpp +++ b/gm/surface.cpp @@ -185,62 +185,3 @@ DEF_SIMPLE_GM(copy_on_write_savelayer, canvas, 256, 256) { // expect to see two rects: blue blended on red canvas->drawImage(surf->makeImageSnapshot(), 0, 0, nullptr); } - -DEF_SIMPLE_GM(surface_underdraw, canvas, 256, 256) { - SkImageInfo info = SkImageInfo::MakeN32Premul(256, 256, nullptr); - auto surf = sk_tool_utils::makeSurface(canvas, info); - - const SkIRect subset = SkIRect::MakeLTRB(180, 0, 256, 256); - - // noisy background - { - SkPoint pts[] = {{0, 0}, {40, 50}}; - SkColor colors[] = {SK_ColorRED, SK_ColorBLUE}; - auto sh = SkGradientShader::MakeLinear(pts, colors, nullptr, 2, SkShader::kRepeat_TileMode); - SkPaint paint; - paint.setShader(sh); - surf->getCanvas()->drawPaint(paint); - } - - // save away the right-hand strip, then clear it - sk_sp saveImg = surf->makeImageSnapshot(subset); - { - SkPaint paint; - paint.setBlendMode(SkBlendMode::kClear); - surf->getCanvas()->drawRect(SkRect::Make(subset), paint); - } - - // draw the "foreground" - { - SkPaint paint; - paint.setColor(SK_ColorGREEN); - SkRect r = { 0, 10, 256, 35 }; - while (r.fBottom < 256) { - surf->getCanvas()->drawRect(r, paint); - r.offset(0, r.height() * 2); - } - } - - // apply the "fade" - { - SkPoint pts[] = {{SkIntToScalar(subset.left()), 0}, {SkIntToScalar(subset.right()), 0}}; - SkColor colors[] = {0xFF000000, 0}; - auto sh = SkGradientShader::MakeLinear(pts, colors, nullptr, 2, SkShader::kClamp_TileMode); - SkPaint paint; - paint.setShader(sh); - paint.setBlendMode(SkBlendMode::kDstIn); - surf->getCanvas()->drawRect(SkRect::Make(subset), paint); - } - - // restore the original strip, drawing it "under" the current foreground - { - SkPaint paint; - paint.setBlendMode(SkBlendMode::kDstOver); - surf->getCanvas()->drawImage(saveImg, - SkIntToScalar(subset.left()), SkIntToScalar(subset.top()), - &paint); - } - - // show it on screen - surf->draw(canvas, 0, 0, nullptr); -} diff --git a/include/core/SkSurface.h b/include/core/SkSurface.h index 23c3b6a235..0c7d36a264 100644 --- a/include/core/SkSurface.h +++ b/include/core/SkSurface.h @@ -495,17 +495,6 @@ public: */ sk_sp makeImageSnapshot(); - /** - * Like the no-parameter version, this returns an image of the current surface contents. - * This variant takes a rectangle specifying the subset of the surface that is of interest. - * These bounds will be sanitized before being used. - * - If bounds extends beyond the surface, it will be trimmed to just the intersection of - * it and the surface. - * - If bounds does not intersect the surface, then this returns nullptr. - * - If bounds == the surface, then this is the same as calling the no-parameter variant. - */ - sk_sp makeImageSnapshot(const SkIRect& bounds); - /** Draws SkSurface contents to canvas, with its top-left corner at (x, y). If SkPaint paint is not nullptr, apply SkColorFilter, alpha, SkImageFilter, diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp index e4a1986630..1a3eb4312e 100644 --- a/src/image/SkSurface.cpp +++ b/src/image/SkSurface.cpp @@ -167,20 +167,6 @@ sk_sp SkSurface::makeImageSnapshot() { return asSB(this)->refCachedImage(); } -sk_sp SkSurface::makeImageSnapshot(const SkIRect& srcBounds) { - const SkIRect surfBounds = { 0, 0, fWidth, fHeight }; - SkIRect bounds = srcBounds; - if (!bounds.intersect(surfBounds)) { - return nullptr; - } - SkASSERT(!bounds.isEmpty()); - if (bounds == surfBounds) { - return this->makeImageSnapshot(); - } else { - return asSB(this)->onNewImageSnapshot(&bounds); - } -} - sk_sp SkSurface::makeSurface(const SkImageInfo& info) { return asSB(this)->onNewSurface(info); } @@ -279,7 +265,7 @@ protected: sk_sp onNewSurface(const SkImageInfo& info) override { return MakeNull(info.width(), info.height()); } - sk_sp onNewImageSnapshot(const SkIRect* subsetOrNull) override { return nullptr; } + sk_sp onNewImageSnapshot() override { return nullptr; } void onWritePixels(const SkPixmap&, int x, int y) override {} void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) override {} void onCopyOnWrite(ContentChangeMode) override {} diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h index ef09331d40..67d330f9bb 100644 --- a/src/image/SkSurface_Base.h +++ b/src/image/SkSurface_Base.h @@ -37,11 +37,8 @@ public: * This needs to be able to outlive the surface itself (if need be), and * must faithfully represent the current contents, even if the surface * is changed after this called (e.g. it is drawn to via its canvas). - * - * If a subset is specified, the the impl must make a copy, rather than try to wait - * on copy-on-write. */ - virtual sk_sp onNewImageSnapshot(const SkIRect* subset = nullptr) = 0; + virtual sk_sp onNewImageSnapshot() = 0; virtual void onWritePixels(const SkPixmap&, int x, int y) = 0; diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index 41f36a8724..af0b564515 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/src/image/SkSurface_Gpu.cpp @@ -83,7 +83,7 @@ sk_sp SkSurface_Gpu::onNewSurface(const SkImageInfo& info) { origin, &this->props()); } -sk_sp SkSurface_Gpu::onNewImageSnapshot(const SkIRect* subset) { +sk_sp SkSurface_Gpu::onNewImageSnapshot() { GrRenderTargetContext* rtc = fDevice->accessRenderTargetContext(); if (!rtc) { return nullptr; @@ -98,14 +98,10 @@ sk_sp SkSurface_Gpu::onNewImageSnapshot(const SkIRect* subset) { SkBudgeted budgeted = rtc->asSurfaceProxy()->isBudgeted(); sk_sp srcProxy = rtc->asTextureProxyRef(); - - if (subset) { - srcProxy = GrSurfaceProxy::Copy(ctx, rtc->asSurfaceProxy(), rtc->mipMapped(), *subset, - budgeted); - } else if (!srcProxy || rtc->priv().refsWrappedObjects()) { - // If the original render target is a buffer originally created by the client, then we don't - // want to ever retarget the SkSurface at another buffer we create. Force a copy now to avoid - // copy-on-write. + // If the original render target is a buffer originally created by the client, then we don't + // want to ever retarget the SkSurface at another buffer we create. Force a copy now to avoid + // copy-on-write. + if (!srcProxy || rtc->priv().refsWrappedObjects()) { SkASSERT(rtc->origin() == rtc->asSurfaceProxy()->origin()); srcProxy = GrSurfaceProxy::Copy(ctx, rtc->asSurfaceProxy(), rtc->mipMapped(), budgeted); diff --git a/src/image/SkSurface_Gpu.h b/src/image/SkSurface_Gpu.h index d5fea6dcbd..97fe5e5d5a 100644 --- a/src/image/SkSurface_Gpu.h +++ b/src/image/SkSurface_Gpu.h @@ -28,7 +28,7 @@ public: SkCanvas* onNewCanvas() override; sk_sp onNewSurface(const SkImageInfo&) override; - sk_sp onNewImageSnapshot(const SkIRect* subset) override; + sk_sp onNewImageSnapshot() override; void onWritePixels(const SkPixmap&, int x, int y) override; void onCopyOnWrite(ContentChangeMode) override; void onDiscard() override; diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp index 5a4fcd2915..19078239d4 100644 --- a/src/image/SkSurface_Raster.cpp +++ b/src/image/SkSurface_Raster.cpp @@ -21,7 +21,7 @@ public: SkCanvas* onNewCanvas() override; sk_sp onNewSurface(const SkImageInfo&) override; - sk_sp onNewImageSnapshot(const SkIRect* subset) override; + sk_sp onNewImageSnapshot() override; void onWritePixels(const SkPixmap&, int x, int y) override; void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) override; void onCopyOnWrite(ContentChangeMode) override; @@ -98,16 +98,7 @@ void SkSurface_Raster::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, canvas->drawBitmap(fBitmap, x, y, paint); } -sk_sp SkSurface_Raster::onNewImageSnapshot(const SkIRect* subset) { - if (subset) { - SkASSERT(SkIRect::MakeWH(fBitmap.width(), fBitmap.height()).contains(*subset)); - SkBitmap dst; - dst.allocPixels(fBitmap.info().makeWH(subset->width(), subset->height())); - SkAssertResult(fBitmap.readPixels(dst.pixmap(), subset->left(), subset->top())); - dst.setImmutable(); // key, so MakeFromBitmap doesn't make a copy of the buffer - return SkImage::MakeFromBitmap(dst); - } - +sk_sp SkSurface_Raster::onNewImageSnapshot() { SkCopyPixelsMode cpm = kIfMutable_SkCopyPixelsMode; if (fWeOwnThePixels) { // SkImage_raster requires these pixels are immutable for its full lifetime.