From 5df6db15dc04ea7f8c8fa7a7b58ecb881d7a70fe Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Thu, 16 Feb 2017 13:39:44 -0500 Subject: [PATCH] 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 Commit-Queue: Robert Phillips --- src/core/SkSpecialImage.cpp | 55 ++++++++++++++++++++++++--------------- src/core/SkSpecialImage.h | 9 ++++--- src/effects/SkTileImageFilter.cpp | 2 +- src/image/SkImage.cpp | 22 +++++----------- tests/ImageFilterTest.cpp | 13 ++++++++- tests/SpecialImageTest.cpp | 4 +-- 6 files changed, 61 insertions(+), 44 deletions(-) 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 @@ public: virtual sk_sp onMakeSurface(const SkImageFilter::OutputProperties& outProps, const SkISize& size, SkAlphaType at) const = 0; - virtual sk_sp onMakeTightSubset(const SkIRect& subset) const = 0; + virtual sk_sp onAsImage(const SkIRect* subset) const = 0; virtual sk_sp onMakeTightSurface(const SkImageFilter::OutputProperties& outProps, const SkISize& size, SkAlphaType at) const = 0; @@ -170,10 +171,11 @@ sk_sp SkSpecialImage::makeSubset(const SkIRect& subset) const { return as_SIB(this)->onMakeSubset(subset); } -sk_sp SkSpecialImage::makeTightSubset(const SkIRect& subset) const { - return as_SIB(this)->onMakeTightSubset(subset); +sk_sp 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 @@ public: &this->props()); } - sk_sp onMakeTightSubset(const SkIRect& subset) const override { - SkBitmap subsetBM; + sk_sp 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 onMakeTightSurface(const SkImageFilter::OutputProperties& outProps, @@ -364,7 +370,9 @@ static sk_sp wrap_proxy_in_image(GrContext* context, GrTextureProxy* pr return nullptr; } - return sk_make_sp(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(tex->width(), tex->height(), kNeedNewImageUniqueID, alphaType, sk_ref_sp(tex), std::move(colorSpace), SkBudgeted::kYes); @@ -499,20 +507,25 @@ public: } // TODO: move all the logic here into the subset-flavor GrSurfaceProxy::copy? - sk_sp 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 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 subsetProxy(GrSurfaceProxy::Copy(fContext, fTextureProxy.get(), + *subset, SkBudgeted::kYes)); + + return wrap_proxy_in_image(fContext, subsetProxy.get(), fAlphaType, fColorSpace); } - sk_sp 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 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 @@ public: sk_sp 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 makeTightSubset(const SkIRect& subset) const; + sk_sp 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 @@ sk_sp SkTileImageFilter::onFilterImage(SkSpecialImage* source, // We create an SkImage here b.c. it needs to be a tight fit for the tiling sk_sp 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 @@ sk_sp SkImage::MakeFromPicture(sk_sp picture, const SkISize& matrix, paint, bitDepth, std::move(colorSpace))); } + sk_sp SkImage::makeWithFilter(const SkImageFilter* filter, const SkIRect& subset, const SkIRect& clipBounds, SkIRect* outSubset, SkIPoint* offset) const { @@ -284,32 +285,21 @@ sk_sp SkImage::makeWithFilter(const SkImageFilter* filter, const SkIRec SkImageFilter::OutputProperties outputProperties(colorSpace); SkImageFilter::Context context(SkMatrix::I(), clipBounds, cache.get(), outputProperties); - sk_sp result = - filter->filterImage(srcSpecialImage.get(), context, offset); - + sk_sp 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 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 @@ DEF_TEST(ImageFilterBlurLargeImage, reporter) { } static void test_make_with_filter(skiatest::Reporter* reporter, GrContext* context) { - sk_sp surface(create_surface(context, 100, 100)); + sk_sp surface(create_surface(context, 192, 128)); surface->getCanvas()->clear(SK_ColorRED); SkPaint bluePaint; bluePaint.setColor(SK_ColorBLUE); @@ -1795,6 +1795,17 @@ static void test_make_with_filter(skiatest::Reporter* reporter, GrContext* conte 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 @@ static void test_image(const sk_sp& img, skiatest::Reporter* rep 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 tightImg(img->makeTightSubset(newSubset)); + sk_sp tightImg(img->asImage(&newSubset)); REPORTER_ASSERT(reporter, tightImg->width() == subset.width()); REPORTER_ASSERT(reporter, tightImg->height() == subset.height()); -- 2.7.4