From d34528c3576bccef9681e4fe947c825355782a1a Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Mon, 22 Jun 2020 14:51:22 -0400 Subject: [PATCH] Remove SkSpecialImage::makeTextureImage Tweak how SkImageSource works, so that all nodes now remain on the GPU, or go directly there if we've got a GPU-backed skif::Context. Bug: skia:9825 Bug: skia:10202 Change-Id: I35471fd41a00a0a9859eff04c26382e9d2d88a7b Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298347 Commit-Queue: Brian Osman Reviewed-by: Michael Ludwig --- src/core/SkImageFilter.cpp | 9 +-- src/core/SkSpecialImage.cpp | 42 ------------- src/core/SkSpecialImage.h | 7 --- src/effects/imagefilters/SkImageSource.cpp | 4 +- tests/ImageFilterTest.cpp | 16 ++--- tests/SpecialImageTest.cpp | 68 ---------------------- 6 files changed, 13 insertions(+), 133 deletions(-) diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp index 4694a89455..8ba4f47006 100644 --- a/src/core/SkImageFilter.cpp +++ b/src/core/SkImageFilter.cpp @@ -230,14 +230,9 @@ skif::FilterResult SkImageFilter_Base::filterImage(const skif::Con result = this->onFilterImage(context); -#if SK_SUPPORT_GPU - if (context.gpuBacked() && result.image() && !result.image()->isTextureBacked()) { - // Keep the result on the GPU - this is still required for some - // image filters that don't support GPU in all cases - auto asTexture = result.image()->makeTextureImage(context.getContext()); - result = skif::FilterResult(std::move(asTexture), result.layerOrigin()); + if (context.gpuBacked()) { + SkASSERT(!result.image() || result.image()->isTextureBacked()); } -#endif if (context.cache()) { context.cache()->set(key, this, result); diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp index 33002fb079..59d7f0a9aa 100644 --- a/src/core/SkSpecialImage.cpp +++ b/src/core/SkSpecialImage.cpp @@ -85,48 +85,6 @@ SkSpecialImage::SkSpecialImage(const SkIRect& subset, , fUniqueID(kNeedNewImageUniqueID_SpecialImage == uniqueID ? SkNextID::ImageID() : uniqueID) { } -sk_sp SkSpecialImage::makeTextureImage(GrRecordingContext* context) const { -#if SK_SUPPORT_GPU - if (!context) { - return nullptr; - } - if (GrRecordingContext* curContext = as_SIB(this)->onGetContext()) { - return curContext->priv().matches(context) ? sk_ref_sp(this) : nullptr; - } - - SkBitmap bmp; - if (!this->getROPixels(&bmp)) { - return nullptr; - } - - if (bmp.empty()) { - return SkSpecialImage::MakeFromRaster(SkIRect::MakeEmpty(), bmp, &this->props()); - } - - // TODO: this is a tight copy of 'bmp' but it doesn't have to be (given SkSpecialImage's - // semantics). Since this is cached though we would have to bake the fit into the cache key. - auto view = GrMakeCachedBitmapProxyView(context, bmp); - if (!view.proxy()) { - return nullptr; - } - - const SkIRect rect = SkIRect::MakeSize(view.proxy()->dimensions()); - - // GrMakeCachedBitmapProxyView has uploaded only the specified subset of 'bmp' so we need not - // bother with SkBitmap::getSubset - return SkSpecialImage::MakeDeferredFromGpu(context, - rect, - this->uniqueID(), - std::move(view), - SkColorTypeToGrColorType(bmp.colorType()), - sk_ref_sp(this->getColorSpace()), - &this->props(), - this->alphaType()); -#else - return nullptr; -#endif -} - void SkSpecialImage::draw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) const { return as_SIB(this)->onDraw(canvas, x, y, paint); } diff --git a/src/core/SkSpecialImage.h b/src/core/SkSpecialImage.h index 9df11ab04c..0dfed77712 100644 --- a/src/core/SkSpecialImage.h +++ b/src/core/SkSpecialImage.h @@ -61,13 +61,6 @@ public: virtual SkColorType colorType() const = 0; virtual size_t getSize() const = 0; - /** - * Ensures that a special image is backed by a texture (when GrRecordingContext is non-null). - * If no transformation is required, the returned image may be the same as this special image. - * If this special image is from a different GrRecordingContext, this will fail. - */ - sk_sp makeTextureImage(GrRecordingContext*) const; - /** * Draw this SpecialImage into the canvas, automatically taking into account the image's subset */ diff --git a/src/effects/imagefilters/SkImageSource.cpp b/src/effects/imagefilters/SkImageSource.cpp index 7c4d1af053..63adae94bf 100644 --- a/src/effects/imagefilters/SkImageSource.cpp +++ b/src/effects/imagefilters/SkImageSource.cpp @@ -103,8 +103,10 @@ sk_sp SkImageSourceImpl::onFilterImage(const Context& ctx, SkRect dstRect; ctx.ctm().mapRect(&dstRect, fDstRect); + // If we have a context and aren't texture backed, we skip the fast path and draw using the + // slow path below to ensure our output image is texture backed. SkRect bounds = SkRect::MakeIWH(fImage->width(), fImage->height()); - if (fSrcRect == bounds) { + if (fSrcRect == bounds && (!ctx.getContext() || fImage->isTextureBacked())) { int iLeft = dstRect.fLeft; int iTop = dstRect.fTop; // TODO: this seems to be a very noise-prone way to determine this (esp. the floating-point diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp index 21e76ab483..8e78dc1e0e 100644 --- a/tests/ImageFilterTest.cpp +++ b/tests/ImageFilterTest.cpp @@ -513,12 +513,12 @@ static void test_negative_blur_sigma(skiatest::Reporter* reporter, GrContext* co sk_sp positiveFilter(SkImageFilters::Blur(kBlurSigma, kBlurSigma, nullptr)); sk_sp negativeFilter(SkImageFilters::Blur(-kBlurSigma, kBlurSigma, nullptr)); - SkBitmap gradient = make_gradient_circle(kWidth, kHeight); - sk_sp imgSrc(SkSpecialImage::MakeFromRaster(SkIRect::MakeWH(kWidth, kHeight), - gradient)); + sk_sp gradient = SkImage::MakeFromBitmap(make_gradient_circle(kWidth, kHeight)); if (context) { - imgSrc = imgSrc->makeTextureImage(context); + gradient = gradient->makeTextureImage(context); } + sk_sp imgSrc( + SkSpecialImage::MakeFromImage(context, SkIRect::MakeWH(kWidth, kHeight), gradient)); SkIPoint offset; SkImageFilter_Base::Context ctx(SkMatrix::I(), SkIRect::MakeWH(32, 32), nullptr, @@ -603,12 +603,12 @@ static void test_morphology_radius_with_mirror_ctm(skiatest::Reporter* reporter, paint.setColor(SK_ColorWHITE); canvas.drawRect(SkRect::MakeXYWH(kWidth / 4, kHeight / 4, kWidth / 2, kHeight / 2), paint); - - sk_sp imgSrc(SkSpecialImage::MakeFromRaster(SkIRect::MakeWH(kWidth, kHeight), - bitmap)); + sk_sp image = SkImage::MakeFromBitmap(bitmap); if (context) { - imgSrc = imgSrc->makeTextureImage(context); + image = image->makeTextureImage(context); } + sk_sp imgSrc( + SkSpecialImage::MakeFromImage(context, SkIRect::MakeWH(kWidth, kHeight), image)); SkIPoint offset; SkImageFilter_Base::Context ctx(SkMatrix::I(), SkIRect::MakeWH(32, 32), nullptr, diff --git a/tests/SpecialImageTest.cpp b/tests/SpecialImageTest.cpp index d19f355a5e..c6c393ae38 100644 --- a/tests/SpecialImageTest.cpp +++ b/tests/SpecialImageTest.cpp @@ -185,74 +185,6 @@ DEF_TEST(SpecialImage_Image_Legacy, reporter) { test_specialimage_image(reporter); } -static void test_texture_backed(skiatest::Reporter* reporter, - const sk_sp& orig, - const sk_sp& gpuBacked) { - REPORTER_ASSERT(reporter, gpuBacked); - REPORTER_ASSERT(reporter, gpuBacked->isTextureBacked()); - REPORTER_ASSERT(reporter, gpuBacked->uniqueID() == orig->uniqueID()); - REPORTER_ASSERT(reporter, gpuBacked->subset().width() == orig->subset().width() && - gpuBacked->subset().height() == orig->subset().height()); - REPORTER_ASSERT(reporter, gpuBacked->getColorSpace() == orig->getColorSpace()); -} - -// Test out the SkSpecialImage::makeTextureImage entry point -DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SpecialImage_MakeTexture, reporter, ctxInfo) { - GrContext* context = ctxInfo.grContext(); - SkBitmap bm = create_bm(); - - const SkIRect& subset = SkIRect::MakeXYWH(kPad, kPad, kSmallerSize, kSmallerSize); - - { - // raster - sk_sp rasterImage(SkSpecialImage::MakeFromRaster( - SkIRect::MakeWH(kFullSize, - kFullSize), - bm)); - - { - sk_sp fromRaster(rasterImage->makeTextureImage(context)); - test_texture_backed(reporter, rasterImage, fromRaster); - } - - { - sk_sp subRasterImage(rasterImage->makeSubset(subset)); - - sk_sp fromSubRaster(subRasterImage->makeTextureImage(context)); - test_texture_backed(reporter, subRasterImage, fromSubRaster); - } - } - - { - // gpu - GrBitmapTextureMaker maker(context, bm, GrImageTexGenPolicy::kNew_Uncached_Budgeted); - auto view = maker.view(GrMipMapped::kNo); - if (!view) { - return; - } - - sk_sp gpuImage( - SkSpecialImage::MakeDeferredFromGpu(context, - SkIRect::MakeWH(kFullSize, kFullSize), - kNeedNewImageUniqueID_SpecialImage, - std::move(view), - maker.colorType(), - nullptr)); - - { - sk_sp fromGPU(gpuImage->makeTextureImage(context)); - test_texture_backed(reporter, gpuImage, fromGPU); - } - - { - sk_sp subGPUImage(gpuImage->makeSubset(subset)); - - sk_sp fromSubGPU(subGPUImage->makeTextureImage(context)); - test_texture_backed(reporter, subGPUImage, fromSubGPU); - } - } -} - DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SpecialImage_Gpu, reporter, ctxInfo) { GrContext* context = ctxInfo.grContext(); SkBitmap bm = create_bm();