use MakeFor...() when caching
and remove SkRuntimeEffectInvalidColorFilters.
Change-Id: I80753e635fb0b2e93637a8b2a7f2add66020e4c1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402196
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/include/effects/SkRuntimeEffect.h b/include/effects/SkRuntimeEffect.h
index dc9c62e..43d2868 100644
--- a/include/effects/SkRuntimeEffect.h
+++ b/include/effects/SkRuntimeEffect.h
@@ -224,7 +224,6 @@
friend class SkRTShader; // fBaseProgram, fMain
friend class SkRuntimeColorFilter; //
- friend sk_sp<SkRuntimeEffect> SkMakeCachedRuntimeEffect(SkString);
uint32_t fHash;
SkString fSkSL;
diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index 537de5a..3633e09 100644
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -463,10 +463,14 @@
}
sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(
+ SkRuntimeEffect::MakeForColorFilter,
"uniform shader cf0;"
"uniform shader cf1;"
"uniform half weight;"
- "half4 main() { return mix(sample(cf0), sample(cf1), weight); }"
+ "half4 main(half4 color) {"
+ "float2 moot_xy;"
+ "return mix(sample(cf0, moot_xy), sample(cf1, moot_xy), weight);"
+ "}"
);
SkASSERT(effect);
diff --git a/src/core/SkColorFilter_Matrix.cpp b/src/core/SkColorFilter_Matrix.cpp
index ec00ee3..1f2d168 100644
--- a/src/core/SkColorFilter_Matrix.cpp
+++ b/src/core/SkColorFilter_Matrix.cpp
@@ -203,7 +203,8 @@
}
code += "}";
- sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(std::move(code));
+ sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForColorFilter,
+ std::move(code));
SkASSERT(effect);
SkAlphaType unpremul = kUnpremul_SkAlphaType;
diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp
index 32d0411..a1c3aab 100644
--- a/src/core/SkRuntimeEffect.cpp
+++ b/src/core/SkRuntimeEffect.cpp
@@ -292,7 +292,8 @@
return result;
}
-sk_sp<SkRuntimeEffect> SkMakeCachedRuntimeEffect(SkString sksl) {
+sk_sp<SkRuntimeEffect> SkMakeCachedRuntimeEffect(SkRuntimeEffect::Result (*make)(SkString sksl),
+ SkString sksl) {
SK_BEGIN_REQUIRE_DENSE
struct Key {
uint32_t skslHashA;
@@ -320,7 +321,7 @@
}
}
- auto [effect, err] = SkRuntimeEffect::Make(std::move(sksl));
+ auto [effect, err] = make(std::move(sksl));
if (!effect) {
return nullptr;
}
@@ -641,7 +642,7 @@
buffer.readString(&sksl);
sk_sp<SkData> uniforms = buffer.readByteArrayAsData();
- auto effect = SkMakeCachedRuntimeEffect(std::move(sksl));
+ auto effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForColorFilter, std::move(sksl));
if (!buffer.validate(effect != nullptr)) {
return nullptr;
}
@@ -808,7 +809,7 @@
localMPtr = &localM;
}
- auto effect = SkMakeCachedRuntimeEffect(std::move(sksl));
+ auto effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForShader, std::move(sksl));
if (!buffer.validate(effect != nullptr)) {
return nullptr;
}
diff --git a/src/core/SkRuntimeEffectPriv.h b/src/core/SkRuntimeEffectPriv.h
index 5e7d38c..08b6f83 100644
--- a/src/core/SkRuntimeEffectPriv.h
+++ b/src/core/SkRuntimeEffectPriv.h
@@ -15,13 +15,15 @@
// 1) they're used in contexts where it's not useful to receive an error message;
// 2) they're cached.
//
-// Users of the public SkRuntimeEffect::Make() can of course cache however they like themselves;
+// Users of the public SkRuntimeEffect::Make*() can of course cache however they like themselves;
// keeping these APIs private means users will not be forced into our cache or cache policy.
-sk_sp<SkRuntimeEffect> SkMakeCachedRuntimeEffect(SkString);
+sk_sp<SkRuntimeEffect> SkMakeCachedRuntimeEffect(SkRuntimeEffect::Result (*make)(SkString sksl),
+ SkString sksl);
-inline sk_sp<SkRuntimeEffect> SkMakeCachedRuntimeEffect(const char* sksl) {
- return SkMakeCachedRuntimeEffect(SkString{sksl});
+inline sk_sp<SkRuntimeEffect> SkMakeCachedRuntimeEffect(SkRuntimeEffect::Result (*make)(SkString),
+ const char* sksl) {
+ return SkMakeCachedRuntimeEffect(make, SkString{sksl});
}
// This is mostly from skvm's rgb->hsl code, with some GPU-related finesse pulled from
diff --git a/src/effects/SkHighContrastFilter.cpp b/src/effects/SkHighContrastFilter.cpp
index d629b59..1414184 100644
--- a/src/effects/SkHighContrastFilter.cpp
+++ b/src/effects/SkHighContrastFilter.cpp
@@ -41,7 +41,8 @@
}
)";
- sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(std::move(code));
+ sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForColorFilter,
+ std::move(code));
SkASSERT(effect);
// A contrast setting of exactly +1 would divide by zero (1+c)/(1-c), so pull in to +1-ε.
diff --git a/src/effects/SkLumaColorFilter.cpp b/src/effects/SkLumaColorFilter.cpp
index e290653..5ee0a96 100644
--- a/src/effects/SkLumaColorFilter.cpp
+++ b/src/effects/SkLumaColorFilter.cpp
@@ -15,7 +15,8 @@
"half4 main(half4 inColor) {"
"return saturate(dot(half3(0.2126, 0.7152, 0.0722), inColor.rgb)).000r;"
"}";
- sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(code);
+ sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(SkRuntimeEffect::MakeForColorFilter,
+ code);
SkASSERT(effect);
return effect->makeColorFilter(SkData::MakeEmpty());
diff --git a/src/effects/SkOverdrawColorFilter.cpp b/src/effects/SkOverdrawColorFilter.cpp
index c1f35a9..7d0f53a 100644
--- a/src/effects/SkOverdrawColorFilter.cpp
+++ b/src/effects/SkOverdrawColorFilter.cpp
@@ -12,24 +12,25 @@
#include "src/core/SkRuntimeEffectPriv.h"
sk_sp<SkColorFilter> SkOverdrawColorFilter::MakeWithSkColors(const SkColor colors[kNumColors]) {
- sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(R"(
- uniform half4 color0;
- uniform half4 color1;
- uniform half4 color2;
- uniform half4 color3;
- uniform half4 color4;
- uniform half4 color5;
+ sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(
+ SkRuntimeEffect::MakeForColorFilter,
+ R"(uniform half4 color0;
+ uniform half4 color1;
+ uniform half4 color2;
+ uniform half4 color3;
+ uniform half4 color4;
+ uniform half4 color5;
- half4 main(half4 color) {
- half alpha = 255.0 * color.a;
- color = alpha < 0.5 ? color0
- : alpha < 1.5 ? color1
- : alpha < 2.5 ? color2
- : alpha < 3.5 ? color3
- : alpha < 4.5 ? color4 : color5;
- return color;
- }
- )");
+ half4 main(half4 color) {
+ half alpha = 255.0 * color.a;
+ color = alpha < 0.5 ? color0
+ : alpha < 1.5 ? color1
+ : alpha < 2.5 ? color2
+ : alpha < 3.5 ? color3
+ : alpha < 4.5 ? color4 : color5;
+ return color;
+ }
+ )");
if (effect) {
auto data = SkData::MakeUninitialized(kNumColors * sizeof(SkPMColor4f));
SkPMColor4f* premul = (SkPMColor4f*)data->writable_data();
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index 47d5449..77f66b7 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -75,23 +75,6 @@
"unknown identifier 'sk_Caps'");
}
-DEF_TEST(SkRuntimeEffectInvalidColorFilters, r) {
- auto test = [r](const char* sksl) {
- auto effect = SkMakeCachedRuntimeEffect(sksl);
- REPORTER_ASSERT(r, effect);
-
- sk_sp<SkData> uniforms = SkData::MakeUninitialized(effect->uniformSize());
-
- REPORTER_ASSERT(r, effect->makeShader(uniforms, nullptr, 0, nullptr, false));
- REPORTER_ASSERT(r, !effect->makeColorFilter(uniforms));
- };
-
- // Runtime effects that use sample coords or sk_FragCoord are valid shaders,
- // but not valid color filters
- test("half4 main(float2 p) { return half2(p).xy01; }");
- test("half4 main(float2 p) { return half2(sk_FragCoord.xy).xy01; }");
-}
-
DEF_TEST(SkRuntimeEffectForColorFilter, r) {
// Tests that the color filter factory rejects or accepts certain SkSL constructs
auto test_valid = [r](const char* sksl) {