From 5f4055d03de8fbb70c6eb1232d5edafc2d437046 Mon Sep 17 00:00:00 2001 From: Leandro Lovisolo Date: Tue, 23 Nov 2021 20:47:14 +0000 Subject: [PATCH] Revert "Add public API support for SkImageFilters::RuntimeShader" This reverts commit ad9d774c1fd570c27c85559bc350a606042fd92c. Reason for revert: Task Test-Debian10-Clang-NUC7i5BNK-GPU-IntelIris640-x86_64-Release-All-DDL3_TSAN has been failing since this CL landed. Original change's description: > Add public API support for SkImageFilters::RuntimeShader > > This new image filter constructor enables SkRuntimeEffects to be > used as shaders within the ImageFilter DAG. The shader is created > lazily using the SkRuntimeShaderBuilder enabling the resulting > shader to consume the previous stage of the ImageFilter graph. > > Change-Id: I5d6917e34a8e5fdd053399f15a1e2cc7409e686f > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470459 > Reviewed-by: Brian Osman > Reviewed-by: Michael Ludwig > Commit-Queue: Derek Sollenberger Change-Id: I0367a5c480df109c6116eb168792fe3c2fc58807 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475644 Auto-Submit: Leandro Lovisolo Reviewed-by: Leandro Lovisolo Reviewed-by: Derek Sollenberger Bot-Commit: Rubber Stamper Commit-Queue: Leandro Lovisolo --- RELEASE_NOTES.txt | 4 - gm/runtimeimagefilter.cpp | 5 +- include/effects/SkImageFilters.h | 22 ---- include/effects/SkRuntimeEffect.h | 10 -- .../imagefilters/SkRuntimeImageFilter.cpp | 100 +++--------------- tests/ShaderImageFilterTest.cpp | 78 -------------- 6 files changed, 19 insertions(+), 200 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 9bd12d0855..907fbb9f03 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -16,10 +16,6 @@ Milestone 98 could be created, but the CPU backend would fail to index them correctly. * SkCanvas::drawVertices and SkCanvas::drawPatch variants that did not take SkBlendMode are removed. - * SkImageFilters::RuntimeShader is a new public API that enables adding RuntimeShaderEffects into - image filter graph. - -* * * Milestone 97 ------------ diff --git a/gm/runtimeimagefilter.cpp b/gm/runtimeimagefilter.cpp index 75522b394f..e5189766cf 100644 --- a/gm/runtimeimagefilter.cpp +++ b/gm/runtimeimagefilter.cpp @@ -30,8 +30,9 @@ static sk_sp make_filter() { return child.eval(coord); } )")).effect; - SkRuntimeShaderBuilder builder(std::move(effect)); - return SkImageFilters::RuntimeShader(builder, /*childShaderName=*/nullptr, /*input=*/nullptr); + return SkMakeRuntimeImageFilter(std::move(effect), + /*uniforms=*/nullptr, + /*input=*/nullptr); } DEF_SIMPLE_GM_BG(rtif_distort, canvas, 500, 750, SK_ColorBLACK) { diff --git a/include/effects/SkImageFilters.h b/include/effects/SkImageFilters.h index e91837ec57..bc79cf9341 100644 --- a/include/effects/SkImageFilters.h +++ b/include/effects/SkImageFilters.h @@ -15,8 +15,6 @@ #include "include/core/SkPicture.h" #include "include/core/SkRect.h" #include "include/core/SkTileMode.h" -#include "include/core/SkTypes.h" -#include "include/effects/SkRuntimeEffect.h" #include @@ -334,26 +332,6 @@ public: return Picture(std::move(pic), target); } -#ifdef SK_ENABLE_SKSL - /** - * Create a filter that fills the output with the per-pixel evaluation of the SkShader produced - * by the SkRuntimeShaderBuilder. The shader is defined in the image filter's local coordinate - * system, so it will automatically be affected by SkCanvas' transform. - * - * @param builder The builder used to produce the runtime shader, that will in turn - * fill the result image - * @param childShaderName The name of the child shader defined in the builder that will be - * bound to the input param (or the source image if the input param - * is null). If null the builder can have exactly one child shader, - * which automatically binds the input param. - * @param input The image filter that will be provided as input to the runtime - * shader. If null the implicit source image is used instead - */ - static sk_sp RuntimeShader(const SkRuntimeShaderBuilder& builder, - const char* childShaderName, - sk_sp input); -#endif // SK_ENABLE_SKSL - enum class Dither : bool { kNo = false, kYes = true diff --git a/include/effects/SkRuntimeEffect.h b/include/effects/SkRuntimeEffect.h index 29b0f01ba4..7226d0f0ff 100644 --- a/include/effects/SkRuntimeEffect.h +++ b/include/effects/SkRuntimeEffect.h @@ -28,7 +28,6 @@ class GrRecordingContext; class SkFilterColorProgram; class SkImage; -class SkRuntimeImageFilter; namespace SkSL { class FunctionDefinition; @@ -383,10 +382,6 @@ protected: : fEffect(std::move(effect)) , fUniforms(SkData::MakeZeroInitialized(fEffect->uniformSize())) , fChildren(fEffect->children().size()) {} - explicit SkRuntimeEffectBuilder(sk_sp effect, sk_sp uniforms) - : fEffect(std::move(effect)) - , fUniforms(std::move(uniforms)) - , fChildren(fEffect->children().size()) {} SkRuntimeEffectBuilder(SkRuntimeEffectBuilder&&) = default; SkRuntimeEffectBuilder(const SkRuntimeEffectBuilder&) = default; @@ -447,11 +442,6 @@ public: private: using INHERITED = SkRuntimeEffectBuilder; - - explicit SkRuntimeShaderBuilder(sk_sp effect, sk_sp uniforms) - : INHERITED(std::move(effect), std::move(uniforms)) {} - - friend class SkRuntimeImageFilter; }; /** diff --git a/src/effects/imagefilters/SkRuntimeImageFilter.cpp b/src/effects/imagefilters/SkRuntimeImageFilter.cpp index 3b555d3617..0ff9fa6609 100644 --- a/src/effects/imagefilters/SkRuntimeImageFilter.cpp +++ b/src/effects/imagefilters/SkRuntimeImageFilter.cpp @@ -19,20 +19,16 @@ #ifdef SK_ENABLE_SKSL +namespace { + class SkRuntimeImageFilter final : public SkImageFilter_Base { public: SkRuntimeImageFilter(sk_sp effect, sk_sp uniforms, sk_sp input) : INHERITED(&input, 1, /*cropRect=*/nullptr) - , fShaderBuilder(std::move(effect), std::move(uniforms)) - , fChildShaderName(fShaderBuilder.effect()->children().front().name) {} - SkRuntimeImageFilter(const SkRuntimeShaderBuilder& builder, - const char* childShaderName, - sk_sp input) - : INHERITED(&input, 1, /*cropRect=*/nullptr) - , fShaderBuilder(builder) - , fChildShaderName(childShaderName) {} + , fEffect(std::move(effect)) + , fUniforms(std::move(uniforms)) {} bool onAffectsTransparentBlack() const override { return true; } MatrixCapability onGetCTMCapability() const override { return MatrixCapability::kTranslate; } @@ -45,12 +41,14 @@ private: friend void ::SkRegisterRuntimeImageFilterFlattenable(); SK_FLATTENABLE_HOOKS(SkRuntimeImageFilter) - mutable SkRuntimeShaderBuilder fShaderBuilder; - SkString fChildShaderName; + sk_sp fEffect; + sk_sp fUniforms; using INHERITED = SkImageFilter_Base; }; +} // end namespace + sk_sp SkMakeRuntimeImageFilter(sk_sp effect, sk_sp uniforms, sk_sp input) { @@ -73,67 +71,25 @@ void SkRegisterRuntimeImageFilterFlattenable() { sk_sp SkRuntimeImageFilter::CreateProc(SkReadBuffer& buffer) { SK_IMAGEFILTER_UNFLATTEN_COMMON(common, 1); - if (common.cropRect()) { - return nullptr; - } - - // Read the SkSL string and convert it into a runtime effect SkString sksl; buffer.readString(&sksl); + sk_sp uniforms = buffer.readByteArrayAsData(); + auto effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForShader, std::move(sksl)); if (!buffer.validate(effect != nullptr)) { return nullptr; } - - // Read the uniform data and make sure it matches the size from the runtime effect - sk_sp uniforms = buffer.readByteArrayAsData(); - if (!buffer.validate(uniforms->size() == effect->uniformSize())) { + if (common.cropRect()) { return nullptr; } - // Read the child shader name and make sure it matches one declared in the effect - SkString childShaderName; - buffer.readString(&childShaderName); - if (!buffer.validate(effect->findChild(childShaderName.c_str()) != nullptr)) { - return nullptr; - } - - SkRuntimeShaderBuilder builder(std::move(effect), std::move(uniforms)); - - // Populate the builder with the corresponding children - for (auto& child : builder.effect()->children()) { - const char* name = child.name.c_str(); - switch (child.type) { - case SkRuntimeEffect::ChildType::kBlender: { - builder.child(name) = buffer.readBlender(); - break; - } - case SkRuntimeEffect::ChildType::kColorFilter: { - builder.child(name) = buffer.readColorFilter(); - break; - } - case SkRuntimeEffect::ChildType::kShader: { - builder.child(name) = buffer.readShader(); - break; - } - } - } - - if (!buffer.isValid()) { - return nullptr; - } - - return SkImageFilters::RuntimeShader(builder, childShaderName.c_str(), common.getInput(0)); + return SkMakeRuntimeImageFilter(std::move(effect), std::move(uniforms), common.getInput(0)); } void SkRuntimeImageFilter::flatten(SkWriteBuffer& buffer) const { this->INHERITED::flatten(buffer); - buffer.writeString(fShaderBuilder.effect()->source().c_str()); - buffer.writeDataAsByteArray(fShaderBuilder.uniforms().get()); - buffer.writeString(fChildShaderName.c_str()); - for (size_t x = 0; x < fShaderBuilder.numChildren(); x++) { - buffer.writeFlattenable(fShaderBuilder.children()[x].flattenable()); - } + buffer.writeString(fEffect->source().c_str()); + buffer.writeDataAsByteArray(fUniforms.get()); } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -163,9 +119,8 @@ sk_sp SkRuntimeImageFilter::onFilterImage(const Context& ctx, input->asImage()->makeShader(SkSamplingOptions(SkFilterMode::kLinear), &localM); SkASSERT(inputShader); - fShaderBuilder.child(fChildShaderName.c_str()) = inputShader; - sk_sp shader = fShaderBuilder.makeShader(nullptr, false); - SkASSERT(shader.get()); + auto shader = fEffect->makeShader(fUniforms, &inputShader, 1, nullptr, false); + SkASSERT(shader); SkPaint paint; paint.setShader(std::move(shader)); @@ -181,31 +136,8 @@ sk_sp SkRuntimeImageFilter::onFilterImage(const Context& ctx, canvas->drawPaint(paint); - // Remove the shader from the builder to avoid unnecessarily prolonging the shader's lifetime - fShaderBuilder.child(fChildShaderName.c_str()) = nullptr; - *offset = outputBounds.topLeft(); return surf->makeImageSnapshot(); } -sk_sp SkImageFilters::RuntimeShader(const SkRuntimeShaderBuilder& builder, - const char* childShaderName, - sk_sp input) { - // if no childShaderName is provided check to see if we can implicitly assign it to the only - // child in the effect - if (childShaderName == nullptr) { - auto children = builder.effect()->children(); - if (children.size() != 1) { - return nullptr; - } - childShaderName = children.front().name.c_str(); - } else if (builder.effect()->findChild(childShaderName) == nullptr) { - // there was no child declared in the runtime effect that matches the provided name - return nullptr; - } - - return sk_sp( - new SkRuntimeImageFilter(builder, childShaderName, std::move(input))); -} - #endif // SK_ENABLE_SKSL diff --git a/tests/ShaderImageFilterTest.cpp b/tests/ShaderImageFilterTest.cpp index ee08d063d1..4124b2b877 100644 --- a/tests/ShaderImageFilterTest.cpp +++ b/tests/ShaderImageFilterTest.cpp @@ -8,11 +8,8 @@ #include "include/core/SkBitmap.h" #include "include/core/SkCanvas.h" #include "include/core/SkShader.h" -#include "include/core/SkSurface.h" #include "include/effects/SkGradientShader.h" #include "include/effects/SkImageFilters.h" -#include "include/effects/SkRuntimeEffect.h" -#include "src/effects/imagefilters/SkRuntimeImageFilter.h" #include "tests/Test.h" static void test_unscaled(skiatest::Reporter* reporter) { @@ -117,78 +114,3 @@ DEF_TEST(PaintImageFilter, reporter) { test_unscaled(reporter); test_scaled(reporter); } - -static void test_runtime_shader(skiatest::Reporter* r, SkSurface* surface) { - sk_sp effect = SkRuntimeEffect::MakeForShader(SkString(R"( - uniform shader child; - vec4 main(vec2 coord) { - return child.eval(coord) * 0.5; - } - )")) - .effect; - SkRuntimeShaderBuilder builder(effect); - - // create a red image filter to feed as input into the SkImageFilters::RuntimeShader - SkPaint redPaint; - redPaint.setColor(SK_ColorRED); - sk_sp input = SkImageFilters::Paint(redPaint); - - // Create the different variations of SkImageFilters::RuntimeShader - // All 3 variations should produce the same pixel output - std::vector> filters = { - SkMakeRuntimeImageFilter(effect, /*uniforms=*/nullptr, input), - SkImageFilters::RuntimeShader(builder, /*childShaderName=*/nullptr, input), - SkImageFilters::RuntimeShader(builder, /*childShaderName=*/"child", input)}; - - for (auto&& filter : filters) { - auto canvas = surface->getCanvas(); - - // clear to transparent - SkPaint paint; - paint.setColor(SK_ColorTRANSPARENT); - paint.setBlendMode(SkBlendMode::kSrc); - canvas->drawPaint(paint); - - SkPaint filterPaint; - // the green color will be ignored by the filter within the runtime shader - filterPaint.setColor(SK_ColorGREEN); - filterPaint.setImageFilter(filter); - canvas->saveLayer(nullptr, &filterPaint); - // the blue color will be ignored by the filter because the input to the image filter is not - // null - canvas->drawColor(SK_ColorBLUE); - canvas->restore(); - - // This is expected to read back the half transparent red pixel produced by the image filter - SkBitmap bitmap; - REPORTER_ASSERT(r, bitmap.tryAllocPixels(surface->imageInfo())); - REPORTER_ASSERT(r, - surface->readPixels(bitmap.info(), - bitmap.getPixels(), - bitmap.rowBytes(), - /*srcX=*/0, - /*srcY=*/0)); - SkColor color = bitmap.getColor(/*x=*/0, /*y=*/0); - - // check alpha with a small tolerance - SkAlpha alpha = SkColorGetA(color); - REPORTER_ASSERT(r, alpha >= 127 && alpha <= 129, "Expected: %d Actual: %d", 128, alpha); - - // check each color channel - color = SkColorSetA(color, 255); - REPORTER_ASSERT(r, SK_ColorRED == color, "Expected: %08x Actual: %08x", SK_ColorRED, color); - } -} - -DEF_TEST(SkRuntimeShaderImageFilter_CPU, r) { - const SkImageInfo info = SkImageInfo::MakeN32Premul(/*width=*/1, /*height=*/1); - sk_sp surface(SkSurface::MakeRaster(info)); - test_runtime_shader(r, surface.get()); -} - -DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRuntimeShaderImageFilter_GPU, r, ctxInfo) { - const SkImageInfo info = SkImageInfo::MakeN32Premul(/*width=*/1, /*height=*/1); - sk_sp surface( - SkSurface::MakeRenderTarget(ctxInfo.directContext(), SkBudgeted::kNo, info)); - test_runtime_shader(r, surface.get()); -}