From 14d319b87b64ec3a92e62aaa3c8273c307098bd7 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Thu, 27 Apr 2023 15:43:07 -0400 Subject: [PATCH] Remove Ganesh and Graphite code from SkImage::subset This makes both implementations more consistent and easier to decouple if there is no GPU backend compiled in. Graphite images are no longer uploaded as a texture-backed image if a recorder is passed in and Lazy images are turned into a raster image before being subset (instead of a texture-backed image). In practice, this should only impact skp images (which were further cleaned up in [1]). In some cases, we want to be able to directly create a texture- based subset, so a new static function does that SkImages::SubsetTextureFrom. This currently has a trivial implementation, but we might do something smarter, especially with larger textures. [1] https://skia-review.googlesource.com/c/skia/+/678656 Change-Id: I28f06a96fa451272c77a5fd06e97b2a4ad5b77b3 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/671679 Commit-Queue: Kevin Lubick Reviewed-by: Brian Osman --- docs/examples/Image_encodeToData.cpp | 3 +- docs/examples/Image_encodeToData_2.cpp | 2 +- docs/examples/Image_makeSubset.cpp | 2 +- .../image_subsets_get_different_uids.cpp | 4 +- .../skbug_237_drawImage_with_blur.cpp | 2 +- gm/coordclampshader.cpp | 2 +- gm/crop_imagefilter.cpp | 2 +- gm/crosscontextimage.cpp | 5 +- gm/drawbitmaprect.cpp | 4 +- gm/drawimageset.cpp | 2 +- gm/image.cpp | 4 +- gm/image_pict.cpp | 6 +- gm/imagemasksubset.cpp | 4 +- gm/perspimages.cpp | 3 +- gm/wacky_yuv_formats.cpp | 2 +- gm/yuv420_odd_dim.cpp | 2 +- include/core/SkImage.h | 56 +++++++++++++----- include/gpu/ganesh/SkImageGanesh.h | 17 ++++++ include/gpu/graphite/Image.h | 42 +++++++++++++ relnotes/skimage_subset.md | 14 +++++ src/core/SkReadBuffer.cpp | 2 +- src/gpu/ganesh/image/SkImage_GaneshBase.cpp | 45 ++++++++++++-- src/gpu/ganesh/image/SkImage_GaneshBase.h | 7 ++- .../ganesh/image/SkImage_GaneshFactories.cpp | 2 +- src/gpu/ganesh/image/SkImage_LazyTexture.cpp | 9 +-- src/gpu/ganesh/image/SkImage_LazyTexture.h | 2 +- src/gpu/graphite/Image_Base_Graphite.cpp | 22 +++++-- src/gpu/graphite/Image_Base_Graphite.h | 2 +- src/gpu/graphite/Image_Graphite.cpp | 4 +- src/gpu/graphite/Image_Graphite.h | 5 +- src/gpu/graphite/Image_YUVA_Graphite.h | 2 +- src/image/SkImage.cpp | 38 +++--------- src/image/SkImage_Base.cpp | 18 ++++++ src/image/SkImage_Base.h | 7 ++- src/image/SkImage_Lazy.cpp | 34 ++++------- src/image/SkImage_Lazy.h | 7 ++- src/image/SkImage_Raster.cpp | 16 ++--- src/image/SkImage_Raster.h | 6 +- src/pdf/SkKeyedImage.cpp | 2 +- tests/GrDDLImageTest.cpp | 8 +-- tests/ImageTest.cpp | 10 ++-- tests/graphite/ImageProviderTest.cpp | 59 +++++++++++++------ 42 files changed, 318 insertions(+), 167 deletions(-) create mode 100644 include/gpu/graphite/Image.h create mode 100644 relnotes/skimage_subset.md diff --git a/docs/examples/Image_encodeToData.cpp b/docs/examples/Image_encodeToData.cpp index 853d08ad7f0c..68e1a541409a 100644 --- a/docs/examples/Image_encodeToData.cpp +++ b/docs/examples/Image_encodeToData.cpp @@ -11,7 +11,8 @@ void draw(SkCanvas* canvas) { SkJpegEncoder::Options options; options.fQuality = quality; sk_sp data(SkJpegEncoder::Encode(nullptr, image.get(), options)); - sk_sp filtered = SkImages::DeferredFromEncodedData(data)->makeSubset(subset); + sk_sp filtered = SkImages::DeferredFromEncodedData(data)-> + makeSubset(nullptr, subset); canvas->drawImage(filtered, x, 0); x += 16; } diff --git a/docs/examples/Image_encodeToData_2.cpp b/docs/examples/Image_encodeToData_2.cpp index c74ca328aff8..cbae93eaf8ef 100644 --- a/docs/examples/Image_encodeToData_2.cpp +++ b/docs/examples/Image_encodeToData_2.cpp @@ -12,7 +12,7 @@ void draw(SkCanvas* canvas) { if (!data) { data = SkPngEncoder::Encode(nullptr, image.get(), {}); } - sk_sp eye = SkImages::DeferredFromEncodedData(data)->makeSubset(subset); + sk_sp eye = SkImages::DeferredFromEncodedData(data)->makeSubset(nullptr, subset); canvas->drawImage(eye, 0, 0); } } // END FIDDLE diff --git a/docs/examples/Image_makeSubset.cpp b/docs/examples/Image_makeSubset.cpp index fc5358a26d0d..e8f49b7d911e 100644 --- a/docs/examples/Image_makeSubset.cpp +++ b/docs/examples/Image_makeSubset.cpp @@ -9,7 +9,7 @@ void draw(SkCanvas* canvas) { const int height = 64; for (int y = 0; y < 512; y += height ) { for (int x = 0; x < 512; x += width ) { - sk_sp subset(image->makeSubset({x, y, x + width, y + height})); + sk_sp subset(image->makeSubset(nullptr, {x, y, x + width, y + height})); canvas->drawImage(subset, x * 3 / 2, y * 3 / 2); } } diff --git a/docs/examples/image_subsets_get_different_uids.cpp b/docs/examples/image_subsets_get_different_uids.cpp index cd96bbcf6873..8a1fe838dbf0 100644 --- a/docs/examples/image_subsets_get_different_uids.cpp +++ b/docs/examples/image_subsets_get_different_uids.cpp @@ -5,8 +5,8 @@ REG_FIDDLE(image_subsets_get_different_uids, 128, 128, true, 5) { // image_subsets_get_different_uids void draw(SkCanvas*) { SkIRect r{0, 0, 10, 10}; - auto s1 = image->makeSubset(r); - auto s2 = image->makeSubset(r); + auto s1 = image->makeSubset(nullptr, r); + auto s2 = image->makeSubset(nullptr, r); SkDebugf("%u\n%u\n", s1->uniqueID(), s2->uniqueID()); } } // END FIDDLE diff --git a/docs/examples/skbug_237_drawImage_with_blur.cpp b/docs/examples/skbug_237_drawImage_with_blur.cpp index 74351fe0da5e..dc2da770e503 100644 --- a/docs/examples/skbug_237_drawImage_with_blur.cpp +++ b/docs/examples/skbug_237_drawImage_with_blur.cpp @@ -7,7 +7,7 @@ void draw(SkCanvas* canvas) { paint.setMaskFilter(SkMaskFilter::MakeBlur(kNormal_SkBlurStyle, 5.0f, false)); canvas->clear(0xFF88FF88); canvas->scale(4, 4); - auto subset = image->makeSubset({16, 16, 48, 48}); + auto subset = image->makeSubset(nullptr, {16, 16, 48, 48}); canvas->drawImage(subset, 16, 16, SkSamplingOptions(), &paint); } } // END FIDDLE diff --git a/gm/coordclampshader.cpp b/gm/coordclampshader.cpp index a1e00ceab67e..4b3121528828 100644 --- a/gm/coordclampshader.cpp +++ b/gm/coordclampshader.cpp @@ -21,7 +21,7 @@ DEF_SIMPLE_GM(coordclampshader, canvas, 1074, 795) { return; } // The mandrill_512 image has a bottom row of mostly black pixels. Remove it. - image = image->makeSubset(SkIRect::MakeWH(image->width(), image->height() - 1)); + image = image->makeSubset(nullptr, SkIRect::MakeWH(image->width(), image->height() - 1)); image = image->withDefaultMipmaps(); auto imageShader = image->makeShader(SkSamplingOptions{SkFilterMode::kLinear}); diff --git a/gm/crop_imagefilter.cpp b/gm/crop_imagefilter.cpp index 556026379bb1..0c718667d530 100644 --- a/gm/crop_imagefilter.cpp +++ b/gm/crop_imagefilter.cpp @@ -182,7 +182,7 @@ void draw_example( SkRect clippedContentBounds; if (clippedContentBounds.intersect(contentBounds, kExampleBounds)) { auto contentImage = ToolUtils::MakeTextureImage( - canvas, image->makeSubset(clippedContentBounds.roundOut())); + canvas, image->makeSubset(nullptr, clippedContentBounds.roundOut())); if (contentImage) { SkPaint tiledPaint; tiledPaint.setShader(contentImage->makeShader( diff --git a/gm/crosscontextimage.cpp b/gm/crosscontextimage.cpp index 5d874f3fbf19..1e656f81058c 100644 --- a/gm/crosscontextimage.cpp +++ b/gm/crosscontextimage.cpp @@ -54,8 +54,9 @@ DEF_SIMPLE_GPU_GM_CAN_FAIL(cross_context_image, rContext, canvas, errorMsg, canvas->drawImage(images[i], 0, 0); canvas->translate(0, 256 + 10); - canvas->drawImage(images[i]->makeSubset(SkIRect::MakeXYWH(64, 64, 128, 128), dContext), - 0, 0); + auto subset = images[i]->makeSubset(dContext, SkIRect::MakeXYWH(64, 64, 128, 128)); + SkASSERTF(subset, "subset was null"); + canvas->drawImage(subset, 0, 0); canvas->translate(128, 0); canvas->drawImageRect(images[i], SkRect::MakeWH(128, 128), diff --git a/gm/drawbitmaprect.cpp b/gm/drawbitmaprect.cpp index 91508a81abb8..03b9d19902c6 100644 --- a/gm/drawbitmaprect.cpp +++ b/gm/drawbitmaprect.cpp @@ -145,11 +145,11 @@ static void imagesubsetproc(SkCanvas* canvas, sk_sp image, const SkBitm } auto direct = GrAsDirectContext(canvas->recordingContext()); - if (sk_sp subset = image->makeSubset(srcR, direct)) { + if (sk_sp subset = image->makeSubset(direct, srcR)) { canvas->drawImageRect(subset, dstR, sampling, paint); } #if defined(SK_GRAPHITE) - if (sk_sp subset = image->makeSubset(srcR, canvas->recorder())) { + if (sk_sp subset = image->makeSubset(canvas->recorder(), srcR, {})) { canvas->drawImageRect(subset, dstR, sampling, paint); } #endif diff --git a/gm/drawimageset.cpp b/gm/drawimageset.cpp index feec60224a85..1d9e27aee1e1 100644 --- a/gm/drawimageset.cpp +++ b/gm/drawimageset.cpp @@ -92,7 +92,7 @@ static void make_image_tiles(int tileW, int tileH, int m, int n, const SkColor c subset.fBottom = h; set[y * m + x].fAAFlags |= SkCanvas::kBottom_QuadAAFlag; } - set[y * m + x].fImage = fullImage->makeSubset(subset); + set[y * m + x].fImage = fullImage->makeSubset(nullptr, subset); set[y * m + x].fSrcRect = SkRect::MakeXYWH(x == 0 ? 0 : 1, y == 0 ? 0 : 1, tileW, tileH); set[y * m + x].fDstRect = SkRect::MakeXYWH(x * tileW, y * tileH, tileW, tileH); diff --git a/gm/image.cpp b/gm/image.cpp index 47e3d2cd3966..4c3775fdcd40 100644 --- a/gm/image.cpp +++ b/gm/image.cpp @@ -508,11 +508,11 @@ DEF_SIMPLE_GM_CAN_FAIL(image_subset, canvas, errorMsg, 440, 220) { #if defined(SK_GRAPHITE) if (recorder) { - subset = img->makeSubset({100, 100, 200, 200}, recorder); + subset = img->makeSubset(recorder, {100, 100, 200, 200}, {}); } else #endif { - subset = img->makeSubset({100, 100, 200, 200}, dContext); + subset = img->makeSubset(dContext, {100, 100, 200, 200}); } canvas->drawImage(subset, 220, 10); diff --git a/gm/image_pict.cpp b/gm/image_pict.cpp index cb22a132cfca..12cb212dfbc9 100644 --- a/gm/image_pict.cpp +++ b/gm/image_pict.cpp @@ -317,16 +317,16 @@ class ImageCacheratorGM : public skiagm::GM { auto textureGen = std::unique_ptr( static_cast(gen.release())); fImageSubset = SkImages::DeferredFromTextureGenerator(std::move(textureGen)) - ->makeSubset(subset, dContext); + ->makeSubset(dContext, subset); } else { fImageSubset = SkImages::DeferredFromGenerator(std::move(gen)) - ->makeSubset(subset, dContext); + ->makeSubset(dContext, subset); } } else { #if defined(SK_GRAPHITE) auto recorder = canvas->recorder(); fImageSubset = SkImages::DeferredFromGenerator(std::move(gen)) - ->makeSubset(subset, recorder); + ->makeSubset(recorder, subset, {}); #endif } if (!fImageSubset) { diff --git a/gm/imagemasksubset.cpp b/gm/imagemasksubset.cpp index 5921e19c5f03..12c6a443691e 100644 --- a/gm/imagemasksubset.cpp +++ b/gm/imagemasksubset.cpp @@ -94,10 +94,10 @@ DEF_SIMPLE_GM(imagemasksubset, canvas, 480, 480) { sk_sp subset; if (auto direct = GrAsDirectContext(canvas->recordingContext())) { - subset = image->makeSubset(kSubset, direct); + subset = image->makeSubset(direct, kSubset); } else { #if defined(SK_GRAPHITE) - subset = image->makeSubset(kSubset, canvas->recorder()); + subset = image->makeSubset(canvas->recorder(), kSubset, {}); #endif } diff --git a/gm/perspimages.cpp b/gm/perspimages.cpp index b3bc19736065..3d5f9c4c16e3 100644 --- a/gm/perspimages.cpp +++ b/gm/perspimages.cpp @@ -26,7 +26,8 @@ using namespace skia_private; static sk_sp make_image1() { return GetResourceAsImage("images/mandrill_128.png"); } static sk_sp make_image2() { - return GetResourceAsImage("images/brickwork-texture.jpg")->makeSubset({0, 0, 128, 128}); + return GetResourceAsImage("images/brickwork-texture.jpg")-> + makeSubset(nullptr, {0, 0, 128, 128}); } namespace skiagm { diff --git a/gm/wacky_yuv_formats.cpp b/gm/wacky_yuv_formats.cpp index d9bee5d26e9b..243d90d63589 100644 --- a/gm/wacky_yuv_formats.cpp +++ b/gm/wacky_yuv_formats.cpp @@ -1134,7 +1134,7 @@ class YUVMakeColorSpaceGM : public GM { y += kTileWidthHeight + kPad; SkIRect bounds = SkIRect::MakeWH(kTileWidthHeight / 2, kTileWidthHeight / 2); - auto subset = yuv->makeSubset(bounds, dContext); + auto subset = SkImages::SubsetTextureFrom(dContext, yuv.get(), bounds); SkASSERT(subset); canvas->drawImage(subset, x, y); y += kTileWidthHeight + kPad; diff --git a/gm/yuv420_odd_dim.cpp b/gm/yuv420_odd_dim.cpp index 8baec4a9de34..c3d7b6f798a9 100644 --- a/gm/yuv420_odd_dim.cpp +++ b/gm/yuv420_odd_dim.cpp @@ -102,7 +102,7 @@ DEF_SIMPLE_GM_CAN_FAIL(yuv420_odd_dim_repeat, canvas, errMsg, // Make sure the image is odd dimensioned. int w = image->width() & 0b1 ? image->width() : image->width() - 1; int h = image->height() & 0b1 ? image->height() : image->height() - 1; - image = image->makeSubset(SkIRect::MakeWH(w, h)); + image = image->makeSubset(nullptr, SkIRect::MakeWH(w, h)); auto [planes, yuvaInfo] = sk_gpu_test::MakeYUVAPlanesAsA8(image.get(), kJPEG_SkYUVColorSpace, diff --git a/include/core/SkImage.h b/include/core/SkImage.h index 1dc696d86102..1533a230da42 100644 --- a/include/core/SkImage.h +++ b/include/core/SkImage.h @@ -680,19 +680,27 @@ class SK_API SkImage : public SkRefCnt { Returns nullptr if any of the following are true: - Subset is empty - Subset is not contained inside the image's bounds - - Pixels in the image could not be read or copied + - Pixels in the source image could not be read or copied + - This image is texture-backed and the provided context is null or does not match + the source image's context. - If this image is texture-backed, the context parameter is required and must match the - context of the source image. If the context parameter is provided, and the image is - raster-backed, the subset will be converted to texture-backed. + If the source image was texture-backed, the resulting image will be texture-backed also. + Otherwise, the returned image will be raster-backed. + @param direct the GrDirectContext of the source image (nullptr is ok if the source image + is not texture-backed). @param subset bounds of returned SkImage - @param context the GrDirectContext in play, if it exists @return the subsetted image, or nullptr example: https://fiddle.skia.org/c/@Image_makeSubset */ - sk_sp makeSubset(const SkIRect& subset, GrDirectContext* direct = nullptr) const; + virtual sk_sp makeSubset(GrDirectContext* direct, const SkIRect& subset) const = 0; + +#if !defined(SK_DISABLE_LEGACY_IMAGE_SUBSET_METHODS) + sk_sp makeSubset(const SkIRect& subset, GrDirectContext* direct = nullptr) const { + return this->makeSubset(direct, subset); + } +#endif /** * Returns true if the image has mipmap levels. @@ -882,20 +890,29 @@ class SK_API SkImage : public SkRefCnt { - Subset is empty - Subset is not contained inside the image's bounds - Pixels in the image could not be read or copied + - This image is texture-backed and the provided context is null or does not match + the source image's context. - If this image is texture-backed, the recorder parameter is required. - If the recorder parameter is provided, and the image is raster-backed, the subset will - be converted to texture-backed. + If the source image was texture-backed, the resulting image will be texture-backed also. + Otherwise, the returned image will be raster-backed. + @param recorder the recorder of the source image (nullptr is ok if the + source image was texture-backed). @param subset bounds of returned SkImage - @param recorder the recorder in which to create the new image - @param RequiredImageProperties properties the returned SkImage must possess (e.g., - mipmaps) + @param RequiredImageProperties properties the returned SkImage must possess (e.g. mipmaps) @return the subsetted image, or nullptr */ + sk_sp makeSubset(skgpu::graphite::Recorder*, + const SkIRect& subset, + RequiredImageProperties) const; + +#if !defined(SK_DISABLE_LEGACY_IMAGE_SUBSET_METHODS) sk_sp makeSubset(const SkIRect& subset, - skgpu::graphite::Recorder*, - RequiredImageProperties = {}) const; + skgpu::graphite::Recorder* r, + RequiredImageProperties props = {}) const { + return this->makeSubset(r, subset, props); + } +#endif /** Creates SkImage in target SkColorSpace. Returns nullptr if SkImage could not be created. @@ -948,7 +965,7 @@ class SK_API SkImage : public SkRefCnt { example: https://fiddle.skia.org/c/@Image_makeNonTextureImage */ - sk_sp makeNonTextureImage() const; + sk_sp makeNonTextureImage(GrDirectContext* = nullptr) const; /** Returns raster image. Copies SkImage backed by GPU texture into CPU memory, or decodes SkImage from lazy image. Returns original SkImage if decoded in @@ -963,7 +980,14 @@ class SK_API SkImage : public SkRefCnt { example: https://fiddle.skia.org/c/@Image_makeRasterImage */ - sk_sp makeRasterImage(CachingHint cachingHint = kDisallow_CachingHint) const; + sk_sp makeRasterImage(GrDirectContext*, + CachingHint cachingHint = kDisallow_CachingHint) const; + +#if !defined(SK_IMAGE_READ_PIXELS_DISABLE_LEGACY_API) + sk_sp makeRasterImage(CachingHint cachingHint = kDisallow_CachingHint) const { + return this->makeRasterImage(nullptr, cachingHint); + } +#endif /** Creates filtered SkImage. filter processes original SkImage, potentially changing color, position, and size. subset is the bounds of original SkImage processed diff --git a/include/gpu/ganesh/SkImageGanesh.h b/include/gpu/ganesh/SkImageGanesh.h index 296d90d17e3f..b6723d04e145 100644 --- a/include/gpu/ganesh/SkImageGanesh.h +++ b/include/gpu/ganesh/SkImageGanesh.h @@ -32,6 +32,7 @@ class SkYUVAPixmaps; enum SkAlphaType : int; enum SkColorType : int; enum class SkTextureCompressionType; +struct SkIRect; struct SkISize; /** @@ -380,6 +381,22 @@ inline bool GetBackendTextureFromImage(GrDirectContext* context, backendTextureReleaseProc); } +/** Returns subset of this image as a texture-backed image. + + Returns nullptr if any of the following are true: + - Subset is empty + - Subset is not contained inside the image's bounds + - Pixels in the source image could not be read or copied + - The source image is texture-backed and context does not match the source image's context. + + @param context the non-null GrDirectContext to which the subset should be uploaded. + @param subset bounds of returned SkImage + @return the subsetted image, uploaded as a texture, or nullptr +*/ +SK_API sk_sp SubsetTextureFrom(GrDirectContext* context, + const SkImage* img, + const SkIRect& subset); + } // namespace SkImages #endif diff --git a/include/gpu/graphite/Image.h b/include/gpu/graphite/Image.h new file mode 100644 index 000000000000..e669a2b2806b --- /dev/null +++ b/include/gpu/graphite/Image.h @@ -0,0 +1,42 @@ +/* + * Copyright 2023 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef skgpu_graphite_Image_DEFINED +#define skgpu_graphite_Image_DEFINED + +#include "include/core/SkImage.h" +#include "include/core/SkRefCnt.h" + +struct SkIRect; + +namespace skgpu::graphite { + class Recorder; +} + +namespace SkImages { +/** Returns subset of this image as a texture-backed image. + + Returns nullptr if any of the following are true: + - Subset is empty + - Subset is not contained inside the image's bounds + - Pixels in the source image could not be read or copied + - The source image is texture-backed and context does not match the source image's context. + + @param recorder the non-null recorder in which to create the new image. + @param img Source image + @param subset bounds of returned SkImage + @param props properties the returned SkImage must possess (e.g. mipmaps) + @return the subsetted image, uploaded as a texture, or nullptr +*/ +SK_API sk_sp SubsetTextureFrom(skgpu::graphite::Recorder* recorder, + const SkImage* img, + const SkIRect& subset, + SkImage::RequiredImageProperties props = {}); +} // namespace SkImages + + +#endif // skgpu_graphite_Image_DEFINED diff --git a/relnotes/skimage_subset.md b/relnotes/skimage_subset.md new file mode 100644 index 000000000000..fee74ddf6a81 --- /dev/null +++ b/relnotes/skimage_subset.md @@ -0,0 +1,14 @@ +`SkImage::subset` now takes a `GrDirectContext*` as its first parameter (this can be `nullptr` for +non-gpu backed images. Images which are backed by a codec or picture will not be turned into a GPU +texture before being read. This should only impact picture-backed images, which may not be read +correctly if the picture contain nested texture-backed images itself. To force a conversion to +a texture, clients should call `SkImages::TextureFromImage`, passing in the image, and then call +subset on the result. Documentation has been clarified that `SkImage::subset` will return a raster- +backed image if the source is not backed by a texture, and texture-otherwise. + +`SkImages::SubsetTextureFrom` has been added to subset an image and explicitly return a texture- +backed image. This allows some optimizations, especially for large images that exceed a maximum +texture size of a GPU. + +`SkImage::makeRasterImage` and `SkImage::makeNonTextureImage` now take a `GrDirectContext*` which +clients should supply for reading-back pixels from texture-backed images. \ No newline at end of file diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp index db8b82ed9d2a..b86c609d8595 100644 --- a/src/core/SkReadBuffer.cpp +++ b/src/core/SkReadBuffer.cpp @@ -412,7 +412,7 @@ sk_sp SkReadBuffer::readImage() { SkIRect subset; this->readIRect(&subset); if (image) { - image = image->makeSubset(subset); + image = image->makeSubset(nullptr, subset); } } diff --git a/src/gpu/ganesh/image/SkImage_GaneshBase.cpp b/src/gpu/ganesh/image/SkImage_GaneshBase.cpp index 36c9b54f8196..4dbc6269f5a2 100644 --- a/src/gpu/ganesh/image/SkImage_GaneshBase.cpp +++ b/src/gpu/ganesh/image/SkImage_GaneshBase.cpp @@ -14,6 +14,7 @@ #include "include/core/SkImageInfo.h" #include "include/core/SkPixmap.h" #include "include/core/SkPromiseImageTexture.h" +#include "include/core/SkRect.h" #include "include/core/SkSize.h" #include "include/gpu/GpuTypes.h" #include "include/gpu/GrBackendSurface.h" @@ -47,7 +48,6 @@ class GrContextThreadSafeProxy; enum SkColorType : int; -struct SkIRect; #if defined(SK_GRAPHITE) #include "src/gpu/graphite/Log.h" @@ -159,12 +159,34 @@ bool SkImage_GaneshBase::getROPixels(GrDirectContext* dContext, return true; } -sk_sp SkImage_GaneshBase::onMakeSubset(const SkIRect& subset, - GrDirectContext* direct) const { +sk_sp SkImage_GaneshBase::makeSubset(GrDirectContext* direct, + const SkIRect& subset) const { if (!fContext->priv().matches(direct)) { return nullptr; } + if (subset.isEmpty()) { + return nullptr; + } + + const SkIRect bounds = SkIRect::MakeWH(this->width(), this->height()); + if (!bounds.contains(subset)) { + return nullptr; + } + + // optimization : return self if the subset == our bounds + if (bounds == subset) { + return sk_ref_sp(const_cast(this)); + } + + return this->onMakeSubset(direct, subset); +} + +sk_sp SkImage_GaneshBase::onMakeSubset(GrDirectContext* direct, + const SkIRect& subset) const { + if (!fContext->priv().matches(direct)) { + return nullptr; + } auto [view, ct] = skgpu::ganesh::AsView(direct, this, skgpu::Mipmapped::kNo); SkASSERT(view); SkASSERT(ct == SkColorTypeToGrColorType(this->colorType())); @@ -195,8 +217,8 @@ sk_sp SkImage_GaneshBase::onMakeTextureImage(skgpu::graphite::Recorder* return nullptr; } -sk_sp SkImage_GaneshBase::onMakeSubset(const SkIRect&, - skgpu::graphite::Recorder*, +sk_sp SkImage_GaneshBase::onMakeSubset(skgpu::graphite::Recorder*, + const SkIRect&, RequiredImageProperties) const { SKGPU_LOG_W("Cannot convert Ganesh-backed image to Graphite"); return nullptr; @@ -361,3 +383,16 @@ sk_sp SkImage_GaneshBase::MakePromiseImageLazyProxy( return GrProxyProvider::CreatePromiseProxy( tsp, std::move(callback), backendFormat, dimensions, mipmapped); } + +namespace SkImages { +sk_sp SubsetTextureFrom(GrDirectContext* context, + const SkImage* img, + const SkIRect& subset) { + if (context == nullptr || img == nullptr) { + return nullptr; + } + auto subsetImg = img->makeSubset(context, subset); + return SkImages::TextureFromImage(context, subsetImg.get()); +} + +} diff --git a/src/gpu/ganesh/image/SkImage_GaneshBase.h b/src/gpu/ganesh/image/SkImage_GaneshBase.h index 68802d6567d5..c5a8f2f995ad 100644 --- a/src/gpu/ganesh/image/SkImage_GaneshBase.h +++ b/src/gpu/ganesh/image/SkImage_GaneshBase.h @@ -54,11 +54,12 @@ class SkImage_GaneshBase : public SkImage_Base { // From SkImage.h bool isValid(GrRecordingContext*) const final; + sk_sp makeSubset(GrDirectContext* direct, const SkIRect& subset) const final; // From SkImage_Base.h bool getROPixels(GrDirectContext*, SkBitmap*, CachingHint) const final; - sk_sp onMakeSubset(const SkIRect& subset, GrDirectContext*) const final; + sk_sp onMakeSubset(GrDirectContext*, const SkIRect& subset) const final; bool onReadPixels(GrDirectContext* dContext, const SkImageInfo& dstInfo, @@ -110,8 +111,8 @@ class SkImage_GaneshBase : public SkImage_Base { #if defined(SK_GRAPHITE) sk_sp onMakeTextureImage(skgpu::graphite::Recorder*, RequiredImageProperties) const final; - sk_sp onMakeSubset(const SkIRect& subset, - skgpu::graphite::Recorder*, + sk_sp onMakeSubset(skgpu::graphite::Recorder*, + const SkIRect& subset, RequiredImageProperties) const final; sk_sp onMakeColorTypeAndColorSpace(SkColorType, sk_sp, diff --git a/src/gpu/ganesh/image/SkImage_GaneshFactories.cpp b/src/gpu/ganesh/image/SkImage_GaneshFactories.cpp index 5d7710edb11e..f37f803db054 100644 --- a/src/gpu/ganesh/image/SkImage_GaneshFactories.cpp +++ b/src/gpu/ganesh/image/SkImage_GaneshFactories.cpp @@ -85,7 +85,7 @@ bool MakeBackendTextureFromImage(GrDirectContext* direct, // image is not unique, or if the texture wraps an external object. if (!image->unique() || !texture->unique() || texture->resourcePriv().refsWrappedObjects()) { // onMakeSubset will always copy the image. - image = as_IB(image)->onMakeSubset(image->bounds(), direct); + image = as_IB(image)->onMakeSubset(direct, image->bounds()); if (!image) { return false; } diff --git a/src/gpu/ganesh/image/SkImage_LazyTexture.cpp b/src/gpu/ganesh/image/SkImage_LazyTexture.cpp index 98013d944a1d..97aa42b46f24 100644 --- a/src/gpu/ganesh/image/SkImage_LazyTexture.cpp +++ b/src/gpu/ganesh/image/SkImage_LazyTexture.cpp @@ -30,10 +30,11 @@ enum class GrColorType; -sk_sp SkImage_LazyTexture::onMakeSubset(const SkIRect& subset, - GrDirectContext* direct) const { - auto pixels = direct ? SkImages::TextureFromImage(direct, this) : this->makeRasterImage(); - return pixels ? pixels->makeSubset(subset, direct) : nullptr; +sk_sp SkImage_LazyTexture::onMakeSubset(GrDirectContext* direct, + const SkIRect& subset) const { + auto pixels = direct ? SkImages::TextureFromImage(direct, this) : + this->makeRasterImage(nullptr); + return pixels ? pixels->makeSubset(direct, subset) : nullptr; } bool SkImage_LazyTexture::readPixelsProxy(GrDirectContext* ctx, const SkPixmap& pixmap) const { diff --git a/src/gpu/ganesh/image/SkImage_LazyTexture.h b/src/gpu/ganesh/image/SkImage_LazyTexture.h index e304b9fe89b2..4f887ca2ef6d 100644 --- a/src/gpu/ganesh/image/SkImage_LazyTexture.h +++ b/src/gpu/ganesh/image/SkImage_LazyTexture.h @@ -22,7 +22,7 @@ class SkImage_LazyTexture final : public SkImage_Lazy { bool readPixelsProxy(GrDirectContext*, const SkPixmap&) const override; - sk_sp onMakeSubset(const SkIRect&, GrDirectContext*) const override; + sk_sp onMakeSubset(GrDirectContext*, const SkIRect&) const override; }; #endif diff --git a/src/gpu/graphite/Image_Base_Graphite.cpp b/src/gpu/graphite/Image_Base_Graphite.cpp index 758790af7575..248a89ba1466 100644 --- a/src/gpu/graphite/Image_Base_Graphite.cpp +++ b/src/gpu/graphite/Image_Base_Graphite.cpp @@ -8,11 +8,12 @@ #include "src/gpu/graphite/Image_Base_Graphite.h" #include "include/core/SkColorSpace.h" +#include "include/gpu/graphite/Image.h" #include "src/gpu/graphite/Log.h" namespace skgpu::graphite { -sk_sp Image_Base::onMakeSubset(const SkIRect&, GrDirectContext*) const { +sk_sp Image_Base::onMakeSubset(GrDirectContext*, const SkIRect&) const { SKGPU_LOG_W("Cannot convert Graphite-backed image to Ganesh"); return nullptr; } @@ -50,8 +51,8 @@ void Image_Base::onAsyncRescaleAndReadPixelsYUV420(SkYUVColorSpace yuvColorSpace using namespace skgpu::graphite; -sk_sp SkImage::makeSubset(const SkIRect& subset, - skgpu::graphite::Recorder* recorder, +sk_sp SkImage::makeSubset(skgpu::graphite::Recorder* recorder, + const SkIRect& subset, RequiredImageProperties requiredProps) const { if (subset.isEmpty()) { return nullptr; @@ -62,5 +63,18 @@ sk_sp SkImage::makeSubset(const SkIRect& subset, return nullptr; } - return as_IB(this)->onMakeSubset(subset, recorder, requiredProps); + return as_IB(this)->onMakeSubset(recorder, subset, requiredProps); } + +namespace SkImages { +sk_sp SubsetTextureFrom(skgpu::graphite::Recorder* recorder, + const SkImage* img, + const SkIRect& subset, + SkImage::RequiredImageProperties props) { + if (!recorder || !img) { + return nullptr; + } + auto subsetImg = img->makeSubset(recorder, subset, props); + return subsetImg->makeTextureImage(recorder, props); +} +} // namespace SkImages diff --git a/src/gpu/graphite/Image_Base_Graphite.h b/src/gpu/graphite/Image_Base_Graphite.h index cccedda9f2bf..3c16dbdbd22e 100644 --- a/src/gpu/graphite/Image_Base_Graphite.h +++ b/src/gpu/graphite/Image_Base_Graphite.h @@ -39,7 +39,7 @@ class Image_Base : public SkImage_Base { SkBitmap*, CachingHint = kAllow_CachingHint) const override { return false; } - sk_sp onMakeSubset(const SkIRect&, GrDirectContext*) const override; + sk_sp onMakeSubset(GrDirectContext*, const SkIRect&) const override; sk_sp onMakeColorTypeAndColorSpace(SkColorType, sk_sp, diff --git a/src/gpu/graphite/Image_Graphite.cpp b/src/gpu/graphite/Image_Graphite.cpp index e941ce020eb8..dddd0c70e21a 100644 --- a/src/gpu/graphite/Image_Graphite.cpp +++ b/src/gpu/graphite/Image_Graphite.cpp @@ -37,8 +37,8 @@ Image::Image(TextureProxyView view, Image::~Image() {} -sk_sp Image::onMakeSubset(const SkIRect& subset, - Recorder* recorder, +sk_sp Image::onMakeSubset(Recorder* recorder, + const SkIRect& subset, RequiredImageProperties requiredProps) const { const SkIRect bounds = SkIRect::MakeWH(this->width(), this->height()); diff --git a/src/gpu/graphite/Image_Graphite.h b/src/gpu/graphite/Image_Graphite.h index 0be75b6c2f41..8012fcda4ef3 100644 --- a/src/gpu/graphite/Image_Graphite.h +++ b/src/gpu/graphite/Image_Graphite.h @@ -31,8 +31,6 @@ class Image final : public Image_Base { return fTextureProxyView.proxy()->mipmapped() == skgpu::Mipmapped::kYes; } - using Image_Base::onMakeSubset; - SkImage_Base::Type type() const override { return SkImage_Base::Type::kGraphite; } sk_sp onReinterpretColorSpace(sk_sp) const override; @@ -49,7 +47,8 @@ class Image final : public Image_Base { private: sk_sp onMakeTextureImage(Recorder*, RequiredImageProperties) const override; sk_sp copyImage(const SkIRect& subset, Recorder*, RequiredImageProperties) const; - sk_sp onMakeSubset(const SkIRect&, Recorder*, RequiredImageProperties) const override; + using Image_Base::onMakeSubset; + sk_sp onMakeSubset(Recorder*, const SkIRect&, RequiredImageProperties) const override; using Image_Base::onMakeColorTypeAndColorSpace; sk_sp onMakeColorTypeAndColorSpace(SkColorType targetCT, sk_sp targetCS, diff --git a/src/gpu/graphite/Image_YUVA_Graphite.h b/src/gpu/graphite/Image_YUVA_Graphite.h index aa4a88bc9692..6c8d96d09fad 100644 --- a/src/gpu/graphite/Image_YUVA_Graphite.h +++ b/src/gpu/graphite/Image_YUVA_Graphite.h @@ -39,7 +39,7 @@ class Image_YUVA final : public Image_Base { return nullptr; } using Image_Base::onMakeSubset; - sk_sp onMakeSubset(const SkIRect&, Recorder*, RequiredImageProperties) const override { + sk_sp onMakeSubset(Recorder*, const SkIRect&, RequiredImageProperties) const override { return nullptr; } using Image_Base::onMakeColorTypeAndColorSpace; diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index 5c37cc3a9ac3..d809bfd1d8aa 100644 --- a/src/image/SkImage.cpp +++ b/src/image/SkImage.cpp @@ -203,32 +203,6 @@ sk_sp SkImage::refEncodedData() const { /////////////////////////////////////////////////////////////////////////////////////////////////// -sk_sp SkImage::makeSubset(const SkIRect& subset, GrDirectContext* direct) const { - if (subset.isEmpty()) { - return nullptr; - } - - const SkIRect bounds = SkIRect::MakeWH(this->width(), this->height()); - if (!bounds.contains(subset)) { - return nullptr; - } - -#if defined(SK_GANESH) - auto myContext = as_IB(this)->context(); - // This check is also performed in the subclass, but we do it here for the short-circuit below. - if (myContext && !myContext->priv().matches(direct)) { - return nullptr; - } -#endif - - // optimization : return self if the subset == our bounds - if (bounds == subset) { - return sk_ref_sp(const_cast(this)); - } - - return as_IB(this)->onMakeSubset(subset, direct); -} - bool SkImage::readPixels(GrDirectContext* dContext, const SkPixmap& pmap, int srcX, int srcY, CachingHint chint) const { return this->readPixels(dContext, pmap.info(), pmap.writable_addr(), pmap.rowBytes(), srcX, @@ -366,14 +340,14 @@ sk_sp SkImage::reinterpretColorSpace(sk_sp target) const return as_IB(this)->onReinterpretColorSpace(std::move(target)); } -sk_sp SkImage::makeNonTextureImage() const { +sk_sp SkImage::makeNonTextureImage(GrDirectContext* dContext) const { if (!this->isTextureBacked()) { return sk_ref_sp(const_cast(this)); } - return this->makeRasterImage(); + return this->makeRasterImage(dContext, kDisallow_CachingHint); } -sk_sp SkImage::makeRasterImage(CachingHint chint) const { +sk_sp SkImage::makeRasterImage(GrDirectContext* dContext, CachingHint chint) const { SkPixmap pm; if (this->peekPixels(&pm)) { return sk_ref_sp(const_cast(this)); @@ -385,8 +359,10 @@ sk_sp SkImage::makeRasterImage(CachingHint chint) const { return nullptr; } - // Context TODO: Elevate GrDirectContext requirement to public API. - auto dContext = as_IB(this)->directContext(); + if (!dContext) { + // Try to get the saved context if the client didn't pass it in (but they really should). + dContext = as_IB(this)->directContext(); + } sk_sp data = SkData::MakeUninitialized(size); pm = {fInfo.makeColorSpace(nullptr), data->writable_data(), fInfo.minRowBytes()}; if (!this->readPixels(dContext, pm, 0, 0, chint)) { diff --git a/src/image/SkImage_Base.cpp b/src/image/SkImage_Base.cpp index 838c61516201..7007dd46e13a 100644 --- a/src/image/SkImage_Base.cpp +++ b/src/image/SkImage_Base.cpp @@ -87,6 +87,24 @@ bool SkImage_Base::onAsLegacyBitmap(GrDirectContext* dContext, SkBitmap* bitmap) return true; } +sk_sp SkImage_Base::makeSubset(GrDirectContext* direct, const SkIRect& subset) const { + if (subset.isEmpty()) { + return nullptr; + } + + const SkIRect bounds = SkIRect::MakeWH(this->width(), this->height()); + if (!bounds.contains(subset)) { + return nullptr; + } + + // optimization : return self if the subset == our bounds + if (bounds == subset) { + return sk_ref_sp(const_cast(this)); + } + + return as_IB(this)->onMakeSubset(direct, subset); +} + void SkImage_Base::onAsyncRescaleAndReadPixelsYUV420(SkYUVColorSpace, sk_sp dstColorSpace, SkIRect srcRect, diff --git a/src/image/SkImage_Base.h b/src/image/SkImage_Base.h index b94ed99d57c4..4d8acf13c670 100644 --- a/src/image/SkImage_Base.h +++ b/src/image/SkImage_Base.h @@ -46,6 +46,7 @@ class SkImage_Base : public SkImage { ~SkImage_Base() override; // From SkImage.h + sk_sp makeSubset(GrDirectContext* direct, const SkIRect& subset) const override; size_t textureSize() const override { return 0; } #if defined(SK_GRAPHITE) sk_sp makeTextureImage(skgpu::graphite::Recorder*, @@ -119,7 +120,7 @@ class SkImage_Base : public SkImage { virtual bool getROPixels(GrDirectContext*, SkBitmap*, CachingHint = kAllow_CachingHint) const = 0; - virtual sk_sp onMakeSubset(const SkIRect&, GrDirectContext*) const = 0; + virtual sk_sp onMakeSubset(GrDirectContext*, const SkIRect&) const = 0; virtual sk_sp onRefEncoded() const { return nullptr; } @@ -184,8 +185,8 @@ class SkImage_Base : public SkImage { #if defined(SK_GRAPHITE) virtual sk_sp onMakeTextureImage(skgpu::graphite::Recorder*, RequiredImageProperties) const = 0; - virtual sk_sp onMakeSubset(const SkIRect&, - skgpu::graphite::Recorder*, + virtual sk_sp onMakeSubset(skgpu::graphite::Recorder*, + const SkIRect&, RequiredImageProperties) const = 0; virtual sk_sp onMakeColorTypeAndColorSpace(SkColorType, sk_sp, diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp index 9da8ecfda42d..37f8a9df70cb 100644 --- a/src/image/SkImage_Lazy.cpp +++ b/src/image/SkImage_Lazy.cpp @@ -20,10 +20,6 @@ #include "src/core/SkResourceCache.h" #include "src/core/SkYUVPlanesCache.h" -#if defined(SK_GANESH) -#include "include/gpu/ganesh/SkImageGanesh.h" -#endif - #if defined(SK_GRAPHITE) #include "src/gpu/graphite/TextureUtils.h" #endif @@ -190,30 +186,26 @@ bool SkImage_Lazy::isValid(GrRecordingContext* context) const { return generator->isValid(context); } -sk_sp SkImage_Lazy::onMakeSubset(const SkIRect& subset, GrDirectContext* direct) const { - // TODO: can we do this more efficiently, by telling the generator we want to - // "realize" a subset? -#if defined(SK_GANESH) - auto pixels = direct ? SkImages::TextureFromImage(direct, this) : this->makeRasterImage(); -#else - auto pixels = this->makeRasterImage(); -#endif - return pixels ? pixels->makeSubset(subset, direct) : nullptr; +sk_sp SkImage_Lazy::onMakeSubset(GrDirectContext*, const SkIRect& subset) const { + // neither picture-backed nor codec-backed lazy images need the context to do readbacks. + // The subclass for cross-context images *does* use the direct context. + auto pixels = this->makeRasterImage(nullptr); + return pixels ? pixels->makeSubset(nullptr, subset) : nullptr; } #if defined(SK_GRAPHITE) -sk_sp SkImage_Lazy::onMakeSubset(const SkIRect& subset, - skgpu::graphite::Recorder* recorder, - RequiredImageProperties requiredProperties) const { +sk_sp SkImage_Lazy::onMakeSubset(skgpu::graphite::Recorder*, + const SkIRect& subset, + RequiredImageProperties props) const { // TODO: can we do this more efficiently, by telling the generator we want to // "realize" a subset? - - sk_sp nonLazyImg = recorder ? this->makeTextureImage(recorder, requiredProperties) - : this->makeRasterImage(); - - return nonLazyImg ? nonLazyImg->makeSubset(subset, recorder, requiredProperties) : nullptr; + sk_sp nonLazyImg = this->makeRasterImage(nullptr); + if (!nonLazyImg) { + return nullptr; + } + return nonLazyImg->makeSubset(nullptr, subset, props); } #endif // SK_GRAPHITE diff --git a/src/image/SkImage_Lazy.h b/src/image/SkImage_Lazy.h index 561e98e3f25f..82f547a3b1af 100644 --- a/src/image/SkImage_Lazy.h +++ b/src/image/SkImage_Lazy.h @@ -60,10 +60,10 @@ class SkImage_Lazy : public SkImage_Base { bool onReadPixels(GrDirectContext*, const SkImageInfo&, void*, size_t, int srcX, int srcY, CachingHint) const override; sk_sp onRefEncoded() const override; - sk_sp onMakeSubset(const SkIRect&, GrDirectContext*) const override; + sk_sp onMakeSubset(GrDirectContext*, const SkIRect&) const override; #if defined(SK_GRAPHITE) - sk_sp onMakeSubset(const SkIRect&, - skgpu::graphite::Recorder*, + sk_sp onMakeSubset(skgpu::graphite::Recorder*, + const SkIRect&, RequiredImageProperties) const override; sk_sp onMakeColorTypeAndColorSpace(SkColorType targetCT, sk_sp targetCS, @@ -110,6 +110,7 @@ class SkImage_Lazy : public SkImage_Base { mutable SkIDChangeListener::List fUniqueIDListeners; }; +// Ref-counted tuple(SkImageGenerator, SkMutex) which allows sharing one generator among N images class SharedGenerator final : public SkNVRefCnt { public: static sk_sp Make(std::unique_ptr gen); diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp index ac0704443c9b..fb7828e11515 100644 --- a/src/image/SkImage_Raster.cpp +++ b/src/image/SkImage_Raster.cpp @@ -112,7 +112,7 @@ static SkBitmap copy_bitmap_subset(const SkBitmap& orig, const SkIRect& subset) return bitmap; } -sk_sp SkImage_Raster::onMakeSubset(const SkIRect& subset, GrDirectContext*) const { +sk_sp SkImage_Raster::onMakeSubset(GrDirectContext*, const SkIRect& subset) const { SkBitmap copy = copy_bitmap_subset(fBitmap, subset); if (copy.isNull()) { return nullptr; @@ -139,8 +139,8 @@ static sk_sp copy_mipmaps(const SkBitmap& src, SkMipmap* srcMips) { return dst; } -sk_sp SkImage_Raster::onMakeSubset(const SkIRect& subset, - skgpu::graphite::Recorder* recorder, +sk_sp SkImage_Raster::onMakeSubset(skgpu::graphite::Recorder* recorder, + const SkIRect& subset, RequiredImageProperties requiredProperties) const { sk_sp img; @@ -168,15 +168,7 @@ sk_sp SkImage_Raster::onMakeSubset(const SkIRect& subset, } } - if (!img) { - return nullptr; - } - - if (recorder) { - return img->makeTextureImage(recorder, requiredProperties); - } else { - return img; - } + return img; } #endif // SK_GRAPHITE diff --git a/src/image/SkImage_Raster.h b/src/image/SkImage_Raster.h index d584089f2dc4..c6cd812bfb55 100644 --- a/src/image/SkImage_Raster.h +++ b/src/image/SkImage_Raster.h @@ -59,10 +59,10 @@ class SkImage_Raster : public SkImage_Base { const SkBitmap* onPeekBitmap() const override { return &fBitmap; } bool getROPixels(GrDirectContext*, SkBitmap*, CachingHint) const override; - sk_sp onMakeSubset(const SkIRect&, GrDirectContext*) const override; + sk_sp onMakeSubset(GrDirectContext*, const SkIRect&) const override; #if defined(SK_GRAPHITE) - sk_sp onMakeSubset(const SkIRect&, - skgpu::graphite::Recorder*, + sk_sp onMakeSubset(skgpu::graphite::Recorder*, + const SkIRect&, RequiredImageProperties) const override; #endif diff --git a/src/pdf/SkKeyedImage.cpp b/src/pdf/SkKeyedImage.cpp index 7b733d072a86..e502e22e695b 100644 --- a/src/pdf/SkKeyedImage.cpp +++ b/src/pdf/SkKeyedImage.cpp @@ -33,7 +33,7 @@ SkKeyedImage::SkKeyedImage(const SkBitmap& bm) : fImage(bm.asImage()) { SkKeyedImage SkKeyedImage::subset(SkIRect subset) const { SkKeyedImage img; if (fImage && subset.intersect(fImage->bounds())) { - img.fImage = fImage->makeSubset(subset); + img.fImage = fImage->makeSubset(nullptr, subset); if (img.fImage) { img.fKey = {subset.makeOffset(fKey.fSubset.topLeft()), fKey.fID}; } diff --git a/tests/GrDDLImageTest.cpp b/tests/GrDDLImageTest.cpp index e65c1c0e85da..cb8ec2716814 100644 --- a/tests/GrDDLImageTest.cpp +++ b/tests/GrDDLImageTest.cpp @@ -48,11 +48,11 @@ DEF_GANESH_TEST(GrDDLImage_MakeSubset, reporter, options, CtsEnforcement::kApiLe REPORTER_ASSERT(reporter, rasterImg->isValid(static_cast(nullptr))); // raster + context: - auto subImg1 = rasterImg->makeSubset(subsetBounds, dContext); + auto subImg1 = rasterImg->makeSubset(dContext, subsetBounds); REPORTER_ASSERT(reporter, subImg1->isValid(dContext)); // raster + no context: - auto subImg2 = rasterImg->makeSubset(subsetBounds); + auto subImg2 = rasterImg->makeSubset(nullptr, subsetBounds); REPORTER_ASSERT(reporter, subImg2->isValid(static_cast(nullptr))); // Texture image: @@ -73,11 +73,11 @@ DEF_GANESH_TEST(GrDDLImage_MakeSubset, reporter, options, CtsEnforcement::kApiLe REPORTER_ASSERT(reporter, gpuImage->isValid(dContext)); // gpu image + context: - auto subImg5 = gpuImage->makeSubset(subsetBounds, dContext); + auto subImg5 = gpuImage->makeSubset(dContext, subsetBounds); REPORTER_ASSERT(reporter, subImg5->isValid(dContext)); // gpu image + nullptr: - REPORTER_ASSERT(reporter, !gpuImage->makeSubset(subsetBounds)); + REPORTER_ASSERT(reporter, !gpuImage->makeSubset(nullptr, subsetBounds)); dContext->flush(); dContext->submit(true); diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp index 0faccdac39f7..7bdaf1544aba 100644 --- a/tests/ImageTest.cpp +++ b/tests/ImageTest.cpp @@ -230,7 +230,7 @@ static void test_encode(skiatest::Reporter* reporter, GrDirectContext* dContext, // Now see if we can instantiate an image from a subset of the surface/origEncoded - decoded = SkImages::DeferredFromEncodedData(origEncoded)->makeSubset(ir); + decoded = SkImages::DeferredFromEncodedData(origEncoded)->makeSubset(nullptr, ir); REPORTER_ASSERT(reporter, decoded); assert_equal(reporter, dContext, image, &ir, decoded.get()); } @@ -1000,7 +1000,7 @@ static void test_cross_context_image(skiatest::Reporter* reporter, const GrConte // 3) Create image, draw, free image, flush // ... and then repeat the last two patterns with drawing on a second* context: // 4) Create image, draw*, flush*, free image - // 5) Create image, draw*, free iamge, flush* + // 5) Create image, draw*, free image, flush* // Case #1: Create image, free image { @@ -1089,7 +1089,7 @@ static void test_cross_context_image(skiatest::Reporter* reporter, const GrConte GrRecordingContextPriv::AutoSuppressWarningMessages aswm(otherCtx); testContext->makeCurrent(); - sk_sp refImg(imageMaker(dContext)); + sk_sp refImg(imageMaker(dContext)); GrSurfaceProxyView view, otherView, viewSecondRef; // Any context should be able to borrow the texture at this point @@ -1663,7 +1663,7 @@ DEF_TEST(image_subset_encode_skbug_7752, reporter) { REPORTER_ASSERT(reporter, ToolUtils::equal_pixels(img.get(), img2.get())); }; check_roundtrip(image); // should trivially pass - check_roundtrip(image->makeSubset({0, 0, W/2, H/2})); - check_roundtrip(image->makeSubset({W/2, H/2, W, H})); + check_roundtrip(image->makeSubset(nullptr, {0, 0, W/2, H/2})); + check_roundtrip(image->makeSubset(nullptr, {W/2, H/2, W, H})); check_roundtrip(image->makeColorSpace(SkColorSpace::MakeSRGBLinear())); } diff --git a/tests/graphite/ImageProviderTest.cpp b/tests/graphite/ImageProviderTest.cpp index 25d2016873da..55724269d5a8 100644 --- a/tests/graphite/ImageProviderTest.cpp +++ b/tests/graphite/ImageProviderTest.cpp @@ -12,6 +12,7 @@ #include "include/core/SkPictureRecorder.h" #include "include/core/SkSpan.h" #include "include/gpu/graphite/Context.h" +#include "include/gpu/graphite/Image.h" #include "include/gpu/graphite/Recording.h" #include "src/core/SkMipmapBuilder.h" #include "src/gpu/graphite/Caps.h" @@ -328,29 +329,32 @@ DEF_GRAPHITE_TEST_FOR_RENDERING_CONTEXTS(ImageProviderTest_Graphite_Testing, rep // works as expected. DEF_GRAPHITE_TEST_FOR_RENDERING_CONTEXTS(Make_TextureImage_Subset_Test, reporter, context) { static const struct { + std::string name; // Some of the factories don't correctly create mipmaps through makeTextureImage // bc Graphite's mipmap regeneration isn't implemented yet (b/238754357). bool fMipmapsBlockedByBug; FactoryT fFactory; } testcases[] = { - { false, create_raster_backed_image_no_mipmaps }, - { false, create_raster_backed_image_with_mipmaps }, - { false, create_gpu_backed_image_no_mipmaps }, - { false, create_gpu_backed_image_with_mipmaps }, - { true, create_picture_backed_image }, - { true, create_bitmap_generator_backed_image }, + { "raster_no_mips", false, create_raster_backed_image_no_mipmaps }, + { "raster_with_mips", false, create_raster_backed_image_with_mipmaps }, + { "texture_no_mips", false, create_gpu_backed_image_no_mipmaps }, + { "texture_with_mips", false, create_gpu_backed_image_with_mipmaps }, + { "picture_backed", true, create_picture_backed_image }, + { "image_generator", true, create_bitmap_generator_backed_image }, }; const SkIRect kFakeSubset = SkIRect::MakeWH(kImageSize.width(), kImageSize.height()); const SkIRect kTrueSubset = kFakeSubset.makeInset(4, 4); - std::unique_ptr recorder = context->makeRecorder(); + std::unique_ptr recorderUP = context->makeRecorder(); + auto recorder = recorderUP.get(); for (auto test : testcases) { - sk_sp orig = test.fFactory(recorder.get()); - + sk_sp orig = test.fFactory(recorder); + skiatest::ReporterContext subtest(reporter, test.name); for (Mipmapped mm : { Mipmapped::kNo, Mipmapped::kYes }) { - sk_sp i = orig->makeTextureImage(recorder.get(), { mm }); + skiatest::ReporterContext subtest2(reporter, SkStringPrintf("mipmaps: %d", (int) mm)); + sk_sp i = orig->makeTextureImage(recorder, { mm }); // makeTextureImage has an optimization which allows Mipmaps on an Image if it // would take extra work to remove them. @@ -362,28 +366,45 @@ DEF_GRAPHITE_TEST_FOR_RENDERING_CONTEXTS(Make_TextureImage_Subset_Test, reporter (i->hasMipmaps() && mipmapOptAllowed)); } - i = orig->makeSubset(kTrueSubset, recorder.get(), { mm }); + // SkImage::makeSubset should "leave an image where it is", that is, return a + // texture backed image iff the original image was texture backed. Otherwise, + // it will return a raster image. + i = orig->makeSubset(recorder, kTrueSubset, { mm }); + REPORTER_ASSERT(reporter, orig->isTextureBacked() == i->isTextureBacked(), + "orig texture status %d != subset texture status %d", + orig->isTextureBacked(), i->isTextureBacked()); + if (i->isTextureBacked()) { + REPORTER_ASSERT(reporter, i->dimensions() == kTrueSubset.size()); + REPORTER_ASSERT(reporter, i->hasMipmaps() == (mm == Mipmapped::kYes)); + } + + i = orig->makeSubset(recorder, kFakeSubset, { mm }); + REPORTER_ASSERT(reporter, orig->isTextureBacked() == i->isTextureBacked(), + "orig texture status %d != subset texture status %d", + orig->isTextureBacked(), i->isTextureBacked()); + if (i->isTextureBacked()) { + REPORTER_ASSERT(reporter, i->dimensions() == kFakeSubset.size()); + REPORTER_ASSERT(reporter, i->hasMipmaps() == (mm == Mipmapped::kYes) || + (i->hasMipmaps() && mipmapOptAllowed)); + } + + // SubsetTextureFrom should always return a texture-backed image + i = SkImages::SubsetTextureFrom(recorder, orig.get(), kTrueSubset, { mm }); REPORTER_ASSERT(reporter, i->isTextureBacked()); REPORTER_ASSERT(reporter, i->dimensions() == kTrueSubset.size()); REPORTER_ASSERT(reporter, i->hasMipmaps() == (mm == Mipmapped::kYes)); - i = orig->makeSubset(kFakeSubset, recorder.get(), { mm }); - REPORTER_ASSERT(reporter, i->isTextureBacked()); - REPORTER_ASSERT(reporter, i->dimensions() == kFakeSubset.size()); - REPORTER_ASSERT(reporter, i->hasMipmaps() == (mm == Mipmapped::kYes) || - (i->hasMipmaps() && mipmapOptAllowed)); - if (!orig->isTextureBacked()) { i = orig->makeTextureImage(nullptr, { mm }); REPORTER_ASSERT(reporter, !i); // Make sure makeSubset w/o a recorder works as expected - i = orig->makeSubset(kTrueSubset, nullptr, { mm }); + i = orig->makeSubset(nullptr, kTrueSubset, { mm }); REPORTER_ASSERT(reporter, !i->isTextureBacked()); REPORTER_ASSERT(reporter, i->dimensions() == kTrueSubset.size()); REPORTER_ASSERT(reporter, i->hasMipmaps() == (mm == Mipmapped::kYes)); - i = orig->makeSubset(kFakeSubset, nullptr, { mm }); + i = orig->makeSubset(nullptr, kFakeSubset, { mm }); REPORTER_ASSERT(reporter, !i->isTextureBacked()); REPORTER_ASSERT(reporter, i->dimensions() == kFakeSubset.size()); REPORTER_ASSERT(reporter, i->hasMipmaps() == (mm == Mipmapped::kYes));