Remove SkSpecialImage::makeSurface

All uses can be replaced by the more consistent and concise skif::Context::makeSurface. Bug: skia:9279 Change-Id: I0328f827440c9375b22d53793bb8ee29e9c35117 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/683356 Commit-Queue: Michael Ludwig <[email protected]> Reviewed-by: Robert Phillips <[email protected]>
armansito · Apr 28, 2023 · 04c8a65 · 04c8a65
1 parent 220047b
commit 04c8a65
Show file tree

Hide file tree

Showing 15 changed files with 64 additions and 119 deletions.
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
@@ -354,7 +354,7 @@ void SkBaseDevice::drawFilteredImage(const skif::Mapping& mapping,
                                              cache.get()});
 
     SkIPoint offset;
-    sk_sp<SkSpecialImage> result = as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset);
+    sk_sp<SkSpecialImage> result = as_IFB(filter)->filterImage(ctx).imageAndOffset(ctx, &offset);
     if (result) {
         SkMatrix deviceMatrixWithOffset = mapping.layerToDevice();
         deviceMatrixWithOffset.preTranslate(offset.fX, offset.fY);

diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
@@ -627,26 +627,22 @@ sk_sp<SkSpecialImage> SkImageFilter_Base::DrawWithFP(GrRecordingContext* rContex
                                                surfaceProps);
 }
 
-sk_sp<SkSpecialImage> SkImageFilter_Base::ImageToColorSpace(SkSpecialImage* src,
-                                                            SkColorType colorType,
-                                                            SkColorSpace* colorSpace,
-                                                            const SkSurfaceProps& surfaceProps) {
+sk_sp<SkSpecialImage> SkImageFilter_Base::ImageToColorSpace(const skif::Context& ctx,
+                                                            SkSpecialImage* src) {
     // There are several conditions that determine if we actually need to convert the source to the
     // destination's color space. Rather than duplicate that logic here, just try to make an xform
     // object. If that produces something, then both are tagged, and the source is in a different
     // gamut than the dest. There is some overhead to making the xform, but those are cached, and
     // if we get one back, that means we're about to use it during the conversion anyway.
     auto colorSpaceXform = GrColorSpaceXform::Make(src->getColorSpace(),  src->alphaType(),
-                                                   colorSpace, kPremul_SkAlphaType);
+                                                   ctx.colorSpace(), kPremul_SkAlphaType);
 
     if (!colorSpaceXform) {
         // No xform needed, just return the original image
         return sk_ref_sp(src);
     }
 
-    sk_sp<SkSpecialSurface> surf(src->makeSurface(colorType, colorSpace,
-                                                  SkISize::Make(src->width(), src->height()),
-                                                  kPremul_SkAlphaType, surfaceProps));
+    sk_sp<SkSpecialSurface> surf = ctx.makeSurface(src->dimensions());
     if (!surf) {
         return sk_ref_sp(src);
     }

diff --git a/src/core/SkImageFilterTypes.cpp b/src/core/SkImageFilterTypes.cpp
@@ -146,17 +146,17 @@ sk_sp<SkSpecialSurface> Context::makeSurface(const SkISize& size,
         // FIXME: Context should also store a surface origin that matches the source origin
         return SkSpecialSurface::MakeRenderTarget(fGaneshContext,
                                                   imageInfo,
-                                                  fInfo.fSurfaceProps,
+                                                  *props,
                                                   fGaneshOrigin);
     } else
 #endif
 #if defined(SK_GRAPHITE)
     if (fGraphiteRecorder) {
-        return SkSpecialSurface::MakeGraphite(fGraphiteRecorder, imageInfo, fInfo.fSurfaceProps);
+        return SkSpecialSurface::MakeGraphite(fGraphiteRecorder, imageInfo, *props);
     } else
 #endif
     {
-        return SkSpecialSurface::MakeRaster(imageInfo, fInfo.fSurfaceProps);
+        return SkSpecialSurface::MakeRaster(imageInfo, *props);
     }
 }
 
@@ -315,8 +315,8 @@ bool LayerSpace<SkMatrix>::inverseMapRect(const LayerSpace<SkIRect>& r,
     }
 }
 
-sk_sp<SkSpecialImage> FilterResult::imageAndOffset(SkIPoint* offset) const {
-    auto [image, origin] = this->resolve(fLayerBounds);
+sk_sp<SkSpecialImage> FilterResult::imageAndOffset(const Context& ctx, SkIPoint* offset) const {
+    auto [image, origin] = this->resolve(ctx, fLayerBounds);
     *offset = SkIPoint(origin);
     return image;
 }
@@ -334,7 +334,7 @@ FilterResult FilterResult::applyCrop(const Context& ctx,
     if (is_nearly_integer_translation(fTransform)) {
         // We can lift the crop to earlier in the order of operations and apply it to the image
         // subset directly, which is handled inside this resolve() call.
-        return this->resolve(tightBounds);
+        return this->resolve(ctx, tightBounds);
     } else {
         // Otherwise cropping is the final operation to the FilterResult's image and can always be
         // applied by adjusting the layer bounds.
@@ -444,7 +444,7 @@ FilterResult FilterResult::applyTransform(const Context& ctx,
         // correctly evaluated. 'nextSampling' will always be 'sampling'.
         LayerSpace<SkIRect> tightBounds;
         if (transform.inverseMapRect(ctx.desiredOutput(), &tightBounds)) {
-            transformed = this->resolve(tightBounds);
+            transformed = this->resolve(ctx, tightBounds);
         }
 
         if (!transformed.fImage) {
@@ -470,6 +470,7 @@ FilterResult FilterResult::applyTransform(const Context& ctx,
 }
 
 std::pair<sk_sp<SkSpecialImage>, LayerSpace<SkIPoint>> FilterResult::resolve(
+        const Context& ctx,
         LayerSpace<SkIRect> dstBounds) const {
     // TODO(michaelludwig): Only valid for kDecal, although kClamp would only need 1 extra
     // pixel of padding so some restriction could happen. We also should skip the intersection if
@@ -504,10 +505,9 @@ std::pair<sk_sp<SkSpecialImage>, LayerSpace<SkIPoint>> FilterResult::resolve(
         return {fImage->makeSubset(subset), imageBounds.topLeft()};
     } // else fall through and attempt a draw
 
-    sk_sp<SkSpecialSurface> surface = fImage->makeSurface(fImage->colorType(),
-                                                          fImage->getColorSpace(),
-                                                          SkISize(dstBounds.size()),
-                                                          kPremul_SkAlphaType, {});
+    // Don't use context properties to avoid DMSAA on internal stages of filter evaluation.
+    SkSurfaceProps props = {};
+    sk_sp<SkSpecialSurface> surface = ctx.makeSurface(SkISize(dstBounds.size()), &props);
     if (!surface) {
         return {nullptr, {}};
     }

diff --git a/src/core/SkImageFilterTypes.h b/src/core/SkImageFilterTypes.h
@@ -687,13 +687,13 @@ class FilterResult {
     // SkImageFilter_Base::filterImage() have been updated to work in the new type system
     // (which comes later as SkDevice, SkCanvas, etc. need to be modified, and coordinate space
     // tagging needs to be added).
-    sk_sp<SkSpecialImage> imageAndOffset(SkIPoint* offset) const;
+    sk_sp<SkSpecialImage> imageAndOffset(const Context& ctx, SkIPoint* offset) const;
 
 private:
     // Renders this FilterResult into a new, but visually equivalent, image that fills 'dstBounds',
     // has nearest-neighbor sampling, and a transform that just translates by 'dstBounds' TL corner.
     std::pair<sk_sp<SkSpecialImage>, LayerSpace<SkIPoint>>
-    resolve(LayerSpace<SkIRect> dstBounds) const;
+    resolve(const Context& ctx, LayerSpace<SkIRect> dstBounds) const;
 
     // The effective image of a FilterResult is 'fImage' sampled by 'fSamplingOptions' and
     // respecting 'fTileMode' (on the SkSpecialImage's subset), transformed by 'fTransform', clipped

diff --git a/src/core/SkImageFilter_Base.h b/src/core/SkImageFilter_Base.h
@@ -217,7 +217,7 @@ class SkImageFilter_Base : public SkImageFilter {
 
     // DEPRECRATED - Call the Context-only filterInput()
     sk_sp<SkSpecialImage> filterInput(int index, const Context& ctx, SkIPoint* offset) const {
-        return this->filterInput(index, ctx).imageAndOffset(offset);
+        return this->filterInput(index, ctx).imageAndOffset(ctx, offset);
     }
 
     // Helper function to visit each of this filter's child filters and call their
@@ -314,14 +314,11 @@ class SkImageFilter_Base : public SkImageFilter {
                                             GrProtected isProtected = GrProtected::kNo);
 
     /**
-     *  Returns a version of the passed-in image (possibly the original), that is in a colorspace
-     *  with the same gamut as the one from the OutputProperties. This allows filters that do many
+     *  Returns a version of the passed-in image (possibly the original), that is in the Context's
+     *  colorspace and color type. This allows filters that do many
      *  texture samples to guarantee that any color space conversion has happened before running.
      */
-    static sk_sp<SkSpecialImage> ImageToColorSpace(SkSpecialImage* src,
-                                                   SkColorType colorType,
-                                                   SkColorSpace* colorSpace,
-                                                   const SkSurfaceProps&);
+    static sk_sp<SkSpecialImage> ImageToColorSpace(const skif::Context& ctx, SkSpecialImage* src);
 #endif
 
     // If 'srcBounds' will sample outside the border of 'originalSrcBounds' (i.e., the sample

diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp
@@ -48,14 +48,6 @@ SkSpecialImage::SkSpecialImage(const SkIRect& subset,
     , fProps(props) {
 }
 
-sk_sp<SkSpecialSurface> SkSpecialImage::makeSurface(SkColorType colorType,
-                                                    const SkColorSpace* colorSpace,
-                                                    const SkISize& size,
-                                                    SkAlphaType at,
-                                                    const SkSurfaceProps& props) const {
-    return this->onMakeSurface(colorType, colorSpace, size, at, props);
-}
-
 sk_sp<SkSurface> SkSpecialImage::makeTightSurface(SkColorType colorType,
                                                   const SkColorSpace* colorSpace,
                                                   const SkISize& size,
@@ -185,15 +177,6 @@ class SkSpecialImage_Raster final : public SkSpecialImage {
     }
 #endif
 
-    sk_sp<SkSpecialSurface> onMakeSurface(SkColorType colorType, const SkColorSpace* colorSpace,
-                                          const SkISize& size, SkAlphaType at,
-                                          const SkSurfaceProps& props) const override {
-        // Ignore the requested color type, the raster backend currently only supports N32
-        colorType = kN32_SkColorType;   // TODO: find ways to allow f16
-        SkImageInfo info = SkImageInfo::Make(size, colorType, at, sk_ref_sp(colorSpace));
-        return SkSpecialSurface::MakeRaster(info, props);
-    }
-
     sk_sp<SkSpecialImage> onMakeSubset(const SkIRect& subset) const override {
         // No need to extract subset, onGetROPixels handles that when needed
         return SkSpecialImage::MakeFromRaster(subset, fBitmap, this->props());
@@ -344,18 +327,6 @@ class SkSpecialImage_Gpu final : public SkSpecialImage {
         return false;
     }
 
-    sk_sp<SkSpecialSurface> onMakeSurface(SkColorType colorType, const SkColorSpace* colorSpace,
-                                          const SkISize& size, SkAlphaType at,
-                                          const SkSurfaceProps& props) const override {
-        if (!fContext) {
-            return nullptr;
-        }
-
-        SkImageInfo ii = SkImageInfo::Make(size, colorType, at, sk_ref_sp(colorSpace));
-
-        return SkSpecialSurface::MakeRenderTarget(fContext, ii, props, fView.origin());
-    }
-
     sk_sp<SkSpecialImage> onMakeSubset(const SkIRect& subset) const override {
         return SkSpecialImage::MakeDeferredFromGpu(fContext,
                                                    subset,

diff --git a/src/core/SkSpecialImage.h b/src/core/SkSpecialImage.h
@@ -116,15 +116,6 @@ class SkSpecialImage : public SkRefCnt {
                                               const SkSurfaceProps&);
 #endif
 
-    /**
-     *  Create a new special surface with a backend that is compatible with this special image.
-     */
-    sk_sp<SkSpecialSurface> makeSurface(SkColorType,
-                                        const SkColorSpace*,
-                                        const SkISize& size,
-                                        SkAlphaType,
-                                        const SkSurfaceProps&) const;
-
     /**
      * Create a new surface with a backend that is compatible with this special image.
      * TODO: switch this to makeSurface once we resolved the naming issue
@@ -230,12 +221,6 @@ class SkSpecialImage : public SkRefCnt {
     // from the content rect by the non-virtual makeSubset().
     virtual sk_sp<SkSpecialImage> onMakeSubset(const SkIRect& subset) const = 0;
 
-    virtual sk_sp<SkSpecialSurface> onMakeSurface(SkColorType colorType,
-                                                  const SkColorSpace* colorSpace,
-                                                  const SkISize& size,
-                                                  SkAlphaType at,
-                                                  const SkSurfaceProps&) const = 0;
-
     // This subset (when not null) is relative to the backing store's coordinate frame, it has
     // already been mapped from the content rect by the non-virtual asImage().
     virtual sk_sp<SkImage> onAsImage(const SkIRect* subset) const = 0;

diff --git a/src/effects/imagefilters/SkBlurImageFilter.cpp b/src/effects/imagefilters/SkBlurImageFilter.cpp
@@ -963,8 +963,7 @@ sk_sp<SkSpecialImage> SkBlurImageFilter::onFilterImage(const Context& ctx,
     if (ctx.gpuBacked()) {
         // Ensure the input is in the destination's gamut. This saves us from having to do the
         // xform during the filter itself.
-        input = ImageToColorSpace(input.get(), ctx.colorType(), ctx.colorSpace(),
-                                  ctx.surfaceProps());
+        input = ImageToColorSpace(ctx, input.get());
         result = this->gpuFilter(ctx, sigma, input, inputBounds, dstBounds, inputOffset,
                                  &resultOffset);
     } else

diff --git a/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp b/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp
@@ -388,8 +388,7 @@ sk_sp<SkSpecialImage> SkMatrixConvolutionImageFilter::onFilterImage(const Contex
         // called pad_image to account for our dilation of bounds, so the result will already be
         // moved to the destination color space. If a filter DAG avoids that, then we use this
         // fall-back, which saves us from having to do the xform during the filter itself.
-        input = ImageToColorSpace(input.get(), ctx.colorType(), ctx.colorSpace(),
-                                  ctx.surfaceProps());
+        input = ImageToColorSpace(ctx, input.get());
 
         GrSurfaceProxyView inputView = input->view(context);
         SkASSERT(inputView.asTextureProxy());

diff --git a/src/effects/imagefilters/SkMorphologyImageFilter.cpp b/src/effects/imagefilters/SkMorphologyImageFilter.cpp
@@ -701,8 +701,7 @@ sk_sp<SkSpecialImage> SkMorphologyImageFilter::onFilterImage(const Context& ctx,
         // called pad_image to account for our dilation of bounds, so the result will already be
         // moved to the destination color space. If a filter DAG avoids that, then we use this
         // fall-back, which saves us from having to do the xform during the filter itself.
-        input = ImageToColorSpace(input.get(), ctx.colorType(), ctx.colorSpace(),
-                                  ctx.surfaceProps());
+        input = ImageToColorSpace(ctx, input.get());
 
         sk_sp<SkSpecialImage> result(apply_morphology(context, input.get(), srcBounds, fType,
                                                       SkISize::Make(width, height), ctx));

diff --git a/src/gpu/graphite/SpecialImage_Graphite.cpp b/src/gpu/graphite/SpecialImage_Graphite.cpp
@@ -68,18 +68,6 @@ class SkSpecialImage_Graphite final : public SkSpecialImage {
         return false;
     }
 
-    sk_sp<SkSpecialSurface> onMakeSurface(SkColorType colorType,
-                                          const SkColorSpace* colorSpace,
-                                          const SkISize& size,
-                                          SkAlphaType at,
-                                          const SkSurfaceProps& props) const override {
-        SkASSERT(fRecorder);
-
-        SkImageInfo ii = SkImageInfo::Make(size, colorType, at, sk_ref_sp(colorSpace));
-
-        return SkSpecialSurface::MakeGraphite(fRecorder, ii, props);
-    }
-
     sk_sp<SkSpecialImage> onMakeSubset(const SkIRect& subset) const override {
         return SkSpecialImage::MakeGraphite(fRecorder,
                                             subset,

diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
@@ -278,7 +278,8 @@ sk_sp<SkImage> SkImage::makeWithFilter(GrRecordingContext* rContext, const SkIma
         context = skif::Context::MakeRaster(ctxInfo);
     }
 
-    sk_sp<SkSpecialImage> result = as_IFB(filter)->filterImage(context).imageAndOffset(offset);
+    sk_sp<SkSpecialImage> result = as_IFB(filter)->filterImage(context)
+                                                  .imageAndOffset(context, offset);
     if (!result) {
         return nullptr;
     }

diff --git a/tests/FilterResultTest.cpp b/tests/FilterResultTest.cpp
@@ -157,7 +157,8 @@ class ApplyAction {
         SkUNREACHABLE;
     }
 
-    sk_sp<SkSpecialImage> renderExpectedImage(sk_sp<SkSpecialImage> source,
+    sk_sp<SkSpecialImage> renderExpectedImage(const Context& ctx,
+                                              sk_sp<SkSpecialImage> source,
                                               const LayerSpace<SkIPoint>& origin,
                                               const LayerSpace<SkIRect>& desiredOutput) const {
         SkASSERT(source);
@@ -167,10 +168,7 @@ class ApplyAction {
             size = {1, 1};
         }
 
-        auto surface = source->makeSurface(source->colorType(),
-                                           source->getColorSpace(),
-                                           size,
-                                           kPremul_SkAlphaType, {});
+        auto surface = ctx.makeSurface(size);
         SkCanvas* canvas = surface->getCanvas();
         canvas->clear(SK_ColorTRANSPARENT);
         canvas->translate(-desiredOutput.left(), -desiredOutput.top());
@@ -289,13 +287,14 @@ class TestRunner {
         }
     }
 
-    bool compareImages(SkSpecialImage* expectedImage,
+    bool compareImages(const skif::Context& ctx,
+                       SkSpecialImage* expectedImage,
                        SkIPoint expectedOrigin,
                        const FilterResult& actual) {
         SkASSERT(expectedImage);
 
         SkIPoint actualOrigin;
-        sk_sp<SkSpecialImage> actualImage = actual.imageAndOffset(&actualOrigin);
+        sk_sp<SkSpecialImage> actualImage = actual.imageAndOffset(ctx, &actualOrigin);
 
         SkBitmap expectedBM = this->readPixels(expectedImage);
         SkBitmap actualBM = this->readPixels(actualImage.get()); // empty if actualImage is null
@@ -657,11 +656,15 @@ class TestCase {
                 REPORTER_ASSERT(fRunner, output.sampling() == fActions[i].expectedSampling());
             }
 
-            expectedImage = fActions[i].renderExpectedImage(std::move(expectedImage),
+            expectedImage = fActions[i].renderExpectedImage(ctx,
+                                                            std::move(expectedImage),
                                                             expectedOrigin,
                                                             desiredOutputs[i]);
             expectedOrigin = desiredOutputs[i].topLeft();
-            if (!fRunner.compareImages(expectedImage.get(), SkIPoint(expectedOrigin), output)) {
+            if (!fRunner.compareImages(ctx,
+                                       expectedImage.get(),
+                                       SkIPoint(expectedOrigin),
+                                       output)) {
                 // If one iteration is incorrect, its failures will likely cascade to further
                 // actions so end now as the test has failed.
                 break;