Replace SkSpecialImage::makeTightSubset with asImage
This should allow the relanding of:
https://skia-review.googlesource.com/c/8450/ (Remove asTextureRef from SkSpecialImage & update effects accordingly (take 2))
Change-Id: I517af11036e3f44a280bbe6b9275f6c38f4a86a4
Reviewed-on: https://skia-review.googlesource.com/8498
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp
index cef87b7..8497e79 100644
--- a/src/core/SkSpecialImage.cpp
+++ b/src/core/SkSpecialImage.cpp
@@ -18,6 +18,7 @@
#if SK_SUPPORT_GPU
#include "GrContext.h"
#include "GrSurfaceContext.h"
+#include "GrSurfaceProxyPriv.h"
#include "GrTexture.h"
#include "GrSamplerParams.h"
#include "GrTextureProxy.h"
@@ -59,7 +60,7 @@
virtual sk_sp<SkSpecialSurface> onMakeSurface(const SkImageFilter::OutputProperties& outProps,
const SkISize& size, SkAlphaType at) const = 0;
- virtual sk_sp<SkImage> onMakeTightSubset(const SkIRect& subset) const = 0;
+ virtual sk_sp<SkImage> onAsImage(const SkIRect* subset) const = 0;
virtual sk_sp<SkSurface> onMakeTightSurface(const SkImageFilter::OutputProperties& outProps,
const SkISize& size, SkAlphaType at) const = 0;
@@ -170,10 +171,11 @@
return as_SIB(this)->onMakeSubset(subset);
}
-sk_sp<SkImage> SkSpecialImage::makeTightSubset(const SkIRect& subset) const {
- return as_SIB(this)->onMakeTightSubset(subset);
+sk_sp<SkImage> SkSpecialImage::asImage(const SkIRect* subset) const {
+ return as_SIB(this)->onAsImage(subset);
}
+
#ifdef SK_DEBUG
static bool rect_fits(const SkIRect& rect, int width, int height) {
if (0 == width && 0 == height) {
@@ -301,14 +303,18 @@
&this->props());
}
- sk_sp<SkImage> onMakeTightSubset(const SkIRect& subset) const override {
- SkBitmap subsetBM;
+ sk_sp<SkImage> onAsImage(const SkIRect* subset) const override {
+ if (subset) {
+ SkBitmap subsetBM;
- if (!fBitmap.extractSubset(&subsetBM, subset)) {
- return nullptr;
+ if (!fBitmap.extractSubset(&subsetBM, *subset)) {
+ return nullptr;
+ }
+
+ return SkImage::MakeFromBitmap(subsetBM);
}
- return SkImage::MakeFromBitmap(subsetBM);
+ return SkImage::MakeFromBitmap(fBitmap);
}
sk_sp<SkSurface> onMakeTightSurface(const SkImageFilter::OutputProperties& outProps,
@@ -364,7 +370,9 @@
return nullptr;
}
- return sk_make_sp<SkImage_Gpu>(proxy->width(), proxy->height(),
+ // Note that we're explicitly using the GrTexture's width & height here b.c. SkImages
+ // must be tight.
+ return sk_make_sp<SkImage_Gpu>(tex->width(), tex->height(),
kNeedNewImageUniqueID, alphaType,
sk_ref_sp(tex),
std::move(colorSpace), SkBudgeted::kYes);
@@ -499,20 +507,25 @@
}
// TODO: move all the logic here into the subset-flavor GrSurfaceProxy::copy?
- sk_sp<SkImage> onMakeTightSubset(const SkIRect& subset) const override {
- // TODO: this is problematic since the surfaceProxy could be loose
- if (0 == subset.fLeft && 0 == subset.fTop &&
- fTextureProxy->width() == subset.width() &&
- fTextureProxy->height() == subset.height()) {
- // The existing GrTexture is already tight so reuse it in the SkImage
- return wrap_proxy_in_image(fContext, fTextureProxy.get(),
- fAlphaType, fColorSpace);
+ sk_sp<SkImage> onAsImage(const SkIRect* subset) const override {
+ if (subset) {
+ // TODO: if this becomes a bottle neck we could base this logic on what the size
+ // will be when it is finally instantiated - but that is more fraught.
+ if (//fSurfaceProxy->priv().isExact() &&
+ 0 == subset->fLeft && 0 == subset->fTop &&
+ fTextureProxy->width() == subset->width() &&
+ fTextureProxy->height() == subset->height()) {
+ // The existing GrTexture is already tight so reuse it in the SkImage
+ return wrap_proxy_in_image(fContext, fTextureProxy.get(), fAlphaType, fColorSpace);
+ }
+
+ sk_sp<GrTextureProxy> subsetProxy(GrSurfaceProxy::Copy(fContext, fTextureProxy.get(),
+ *subset, SkBudgeted::kYes));
+
+ return wrap_proxy_in_image(fContext, subsetProxy.get(), fAlphaType, fColorSpace);
}
- sk_sp<GrTextureProxy> subsetProxy(GrSurfaceProxy::Copy(fContext, fTextureProxy.get(),
- subset, SkBudgeted::kYes));
-
- return wrap_proxy_in_image(fContext, subsetProxy.get(), fAlphaType, fColorSpace);
+ return wrap_proxy_in_image(fContext, fTextureProxy.get(), fAlphaType, fColorSpace);
}
sk_sp<SkSurface> onMakeTightSurface(const SkImageFilter::OutputProperties& outProps,
diff --git a/src/core/SkSpecialImage.h b/src/core/SkSpecialImage.h
index 1c9c5a1..a837683 100644
--- a/src/core/SkSpecialImage.h
+++ b/src/core/SkSpecialImage.h
@@ -117,11 +117,14 @@
sk_sp<SkSpecialImage> makeSubset(const SkIRect& subset) const;
/**
- * Extract a subset of this special image and return it as an SkImage.
+ * Create an SkImage from the contents of this special image optionally extracting a subset.
* It may or may not point to the same backing memory.
- * TODO: switch this to makeSurface once we resolved the naming issue
+ * Note: when no 'subset' parameter is specified the the entire SkSpecialImage will be
+ * returned - including whatever extra padding may have resulted from a loose fit!
+ * When the 'subset' parameter is specified the returned image will be tight even if that
+ * entails a copy!
*/
- sk_sp<SkImage> makeTightSubset(const SkIRect& subset) const;
+ sk_sp<SkImage> asImage(const SkIRect* subset = nullptr) const;
// TODO: hide this when GrLayerHoister uses SkSpecialImages more fully (see skbug.com/5063)
/**
diff --git a/src/effects/SkTileImageFilter.cpp b/src/effects/SkTileImageFilter.cpp
index a140db2..b36f742 100644
--- a/src/effects/SkTileImageFilter.cpp
+++ b/src/effects/SkTileImageFilter.cpp
@@ -73,7 +73,7 @@
// We create an SkImage here b.c. it needs to be a tight fit for the tiling
sk_sp<SkImage> subset;
if (inputBounds.contains(srcIRect)) {
- subset = input->makeTightSubset(srcIRect);
+ subset = input->asImage(&srcIRect);
if (!subset) {
return nullptr;
}
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index 2ae01b8..48d9ad3 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -266,6 +266,7 @@
matrix, paint, bitDepth,
std::move(colorSpace)));
}
+
sk_sp<SkImage> SkImage::makeWithFilter(const SkImageFilter* filter, const SkIRect& subset,
const SkIRect& clipBounds, SkIRect* outSubset,
SkIPoint* offset) const {
@@ -284,32 +285,21 @@
SkImageFilter::OutputProperties outputProperties(colorSpace);
SkImageFilter::Context context(SkMatrix::I(), clipBounds, cache.get(), outputProperties);
- sk_sp<SkSpecialImage> result =
- filter->filterImage(srcSpecialImage.get(), context, offset);
-
+ sk_sp<SkSpecialImage> result = filter->filterImage(srcSpecialImage.get(), context, offset);
if (!result) {
return nullptr;
}
- SkIRect fullSize = SkIRect::MakeWH(result->width(), result->height());
-#if SK_SUPPORT_GPU
- if (result->isTextureBacked()) {
- GrContext* context = result->getContext();
- sk_sp<GrTexture> texture = result->asTextureRef(context);
- if (!texture) {
- return nullptr;
- }
- fullSize = SkIRect::MakeWH(texture->width(), texture->height());
- }
-#endif
*outSubset = SkIRect::MakeWH(result->width(), result->height());
if (!outSubset->intersect(clipBounds.makeOffset(-offset->x(), -offset->y()))) {
return nullptr;
}
offset->fX += outSubset->x();
offset->fY += outSubset->y();
- // This isn't really a "tight" subset, but includes any texture padding.
- return result->makeTightSubset(fullSize);
+
+ // Note that here we're returning the special image's entire backing store, loose padding
+ // and all!
+ return result->asImage();
}
bool SkImage::isLazyGenerated() const {
diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index 28612e8..79f34f7 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -1750,7 +1750,7 @@
}
static void test_make_with_filter(skiatest::Reporter* reporter, GrContext* context) {
- sk_sp<SkSurface> surface(create_surface(context, 100, 100));
+ sk_sp<SkSurface> surface(create_surface(context, 192, 128));
surface->getCanvas()->clear(SK_ColorRED);
SkPaint bluePaint;
bluePaint.setColor(SK_ColorBLUE);
@@ -1795,6 +1795,17 @@
SkIRect destRect = SkIRect::MakeXYWH(offset.x(), offset.y(),
outSubset.width(), outSubset.height());
REPORTER_ASSERT(reporter, clipBounds.contains(destRect));
+
+ // In GPU-mode, this case creates a special image with a backing size that differs from
+ // the content size
+ {
+ clipBounds.setXYWH(0, 0, 170, 100);
+ subset.setXYWH(0, 0, 160, 90);
+
+ filter = SkXfermodeImageFilter::Make(SkBlendMode::kSrc, nullptr);
+ result = sourceImage->makeWithFilter(filter.get(), subset, clipBounds, &outSubset, &offset);
+ REPORTER_ASSERT(reporter, result);
+ }
}
DEF_TEST(ImageFilterMakeWithFilter, reporter) {
diff --git a/tests/SpecialImageTest.cpp b/tests/SpecialImageTest.cpp
index e2e4990..fc59f05 100644
--- a/tests/SpecialImageTest.cpp
+++ b/tests/SpecialImageTest.cpp
@@ -110,11 +110,11 @@
kSmallerSize+kPad));
//--------------
- // Test that makeTightSubset & makeTightSurface return appropriately sized objects
+ // Test that asImage & makeTightSurface return appropriately sized objects
// of the correct backing type
SkIRect newSubset = SkIRect::MakeWH(subset.width(), subset.height());
{
- sk_sp<SkImage> tightImg(img->makeTightSubset(newSubset));
+ sk_sp<SkImage> tightImg(img->asImage(&newSubset));
REPORTER_ASSERT(reporter, tightImg->width() == subset.width());
REPORTER_ASSERT(reporter, tightImg->height() == subset.height());