Revert "remove 'in uniform' support from GrSkSLFP, make rules more clear"
This reverts commit ac18a5ca60d47c26ada431b49373f7b9371686a5.
Reason for revert: breaking Chrome roll
Original change's description:
> remove 'in uniform' support from GrSkSLFP, make rules more clear
>
> Bug: skia:
> Change-Id: Iaa4d33c1bfb295d87343411ba6aacc8fae68ef9c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244300
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com
Change-Id: I6e53f5197c751d961abfa21861b940d4168de213
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244508
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
diff --git a/gm/runtimecolorfilter.cpp b/gm/runtimecolorfilter.cpp
index 443cd50..3d8c9e5 100644
--- a/gm/runtimecolorfilter.cpp
+++ b/gm/runtimecolorfilter.cpp
@@ -26,7 +26,7 @@
class GrRenderTargetContext;
const char* SKSL_TEST_SRC = R"(
- layout(ctype=float) uniform half b;
+ layout(ctype=float) in uniform half b;
void main(inout half4 color) {
color.a = color.r*0.3 + color.g*0.6 + color.b*0.1;
diff --git a/gm/runtimefunctions.cpp b/gm/runtimefunctions.cpp
index 045fb63..15c9177 100644
--- a/gm/runtimefunctions.cpp
+++ b/gm/runtimefunctions.cpp
@@ -23,7 +23,7 @@
#include <stddef.h>
static const char* RUNTIME_FUNCTIONS_SRC = R"(
- layout(ctype=SkRect) uniform half4 gColor;
+ layout(ctype=SkRect) in uniform half4 gColor;
half scale(float x) {
return half(x) / 255;
diff --git a/gm/runtimeshader.cpp b/gm/runtimeshader.cpp
index d0e8dc2..6234130 100644
--- a/gm/runtimeshader.cpp
+++ b/gm/runtimeshader.cpp
@@ -23,7 +23,7 @@
#include <stddef.h>
const char* gProg = R"(
- layout(ctype=SkRect) uniform half4 gColor;
+ layout(ctype=SkRect) in uniform half4 gColor;
void main(float x, float y, inout half4 color) {
color = half4(half(x)*(1.0/255), half(y)*(1.0/255), gColor.b, 1);
diff --git a/modules/particles/src/SkParticleEffect.cpp b/modules/particles/src/SkParticleEffect.cpp
index 9ca6750..fa66b9d 100644
--- a/modules/particles/src/SkParticleEffect.cpp
+++ b/modules/particles/src/SkParticleEffect.cpp
@@ -59,7 +59,7 @@
uint flags;
};
-uniform float dt;
+in uniform float dt;
)";
static const char* kParticleHeader =
@@ -77,7 +77,7 @@
uint flags;
};
-uniform Effect effect;
+in uniform Effect effect;
)";
static const char* kDefaultEffectCode =
diff --git a/src/effects/SkOverdrawColorFilter.cpp b/src/effects/SkOverdrawColorFilter.cpp
index 1552ab8..6928aac 100644
--- a/src/effects/SkOverdrawColorFilter.cpp
+++ b/src/effects/SkOverdrawColorFilter.cpp
@@ -15,12 +15,12 @@
#include "src/gpu/effects/GrSkSLFP.h"
GR_FP_SRC_STRING SKSL_OVERDRAW_SRC = R"(
-layout(ctype=SkPMColor) uniform half4 color0;
-layout(ctype=SkPMColor) uniform half4 color1;
-layout(ctype=SkPMColor) uniform half4 color2;
-layout(ctype=SkPMColor) uniform half4 color3;
-layout(ctype=SkPMColor) uniform half4 color4;
-layout(ctype=SkPMColor) uniform half4 color5;
+layout(ctype=SkPMColor) in uniform half4 color0;
+layout(ctype=SkPMColor) in uniform half4 color1;
+layout(ctype=SkPMColor) in uniform half4 color2;
+layout(ctype=SkPMColor) in uniform half4 color3;
+layout(ctype=SkPMColor) in uniform half4 color4;
+layout(ctype=SkPMColor) in uniform half4 color5;
void main(inout half4 color) {
half alpha = 255.0 * color.a;
diff --git a/src/effects/imagefilters/SkArithmeticImageFilter.cpp b/src/effects/imagefilters/SkArithmeticImageFilter.cpp
index 5d42e24..d08d5b7 100644
--- a/src/effects/imagefilters/SkArithmeticImageFilter.cpp
+++ b/src/effects/imagefilters/SkArithmeticImageFilter.cpp
@@ -32,14 +32,14 @@
#include "src/gpu/glsl/GrGLSLUniformHandler.h"
GR_FP_SRC_STRING SKSL_ARITHMETIC_SRC = R"(
-uniform float4 k;
-in bool enforcePMColor;
+in uniform float4 k;
+layout(key) const in bool enforcePMColor;
in fragmentProcessor child;
void main(inout half4 color) {
half4 dst = sample(child);
color = saturate(half(k.x) * color * dst + half(k.y) * color + half(k.z) * dst + half(k.w));
- @if (enforcePMColor) {
+ if (enforcePMColor) {
color.rgb = min(color.rgb, color.a);
}
}
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index c337509..ca1ee23 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -46,7 +46,7 @@
GR_FP_SRC_STRING SKSL_DITHER_SRC = R"(
// This controls the range of values added to color channels
-in int rangeType;
+layout(key) in int rangeType;
void main(float x, float y, inout half4 color) {
half value;
diff --git a/src/gpu/effects/GrSkSLFP.cpp b/src/gpu/effects/GrSkSLFP.cpp
index 9269ebf..b56b8cb 100644
--- a/src/gpu/effects/GrSkSLFP.cpp
+++ b/src/gpu/effects/GrSkSLFP.cpp
@@ -34,16 +34,9 @@
SkSL::VarDeclarations& v = (SkSL::VarDeclarations&) e;
for (const auto& varStatement : v.fVars) {
const SkSL::Variable& var = *((SkSL::VarDeclaration&) *varStatement).fVar;
- if ((var.fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) ||
- (var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag)) {
- fInAndUniformVars.push_back(&var);
+ if (var.fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) {
+ fInputVars.push_back(&var);
}
- // "in uniform" doesn't make sense outside of .fp files
- SkASSERT((var.fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) == 0 ||
- (var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag) == 0);
- // "layout(key)" doesn't make sense outside of .fp files; all 'in' variables are
- // part of the key
- SkASSERT(!var.fModifiers.fLayout.fKey);
}
}
}
@@ -58,7 +51,7 @@
std::unordered_map<SkSL::String, SkSL::Program::Settings::Value> inputMap;
size_t offset = 0;
- for (const auto& v : fInAndUniformVars) {
+ for (const auto& v : fInputVars) {
SkSL::String name(v->fName);
if (&v->fType == fCompiler.context().fInt_Type.get() ||
&v->fType == fCompiler.context().fShort_Type.get()) {
@@ -120,12 +113,11 @@
class GrGLSLSkSLFP : public GrGLSLFragmentProcessor {
public:
- GrGLSLSkSLFP(const SkSL::Context* context,
- const std::vector<const SkSL::Variable*>* inAndUniformVars,
+ GrGLSLSkSLFP(const SkSL::Context* context, const std::vector<const SkSL::Variable*>* inputVars,
SkSL::String glsl, std::vector<SkSL::Compiler::FormatArg> formatArgs,
std::vector<SkSL::Compiler::GLSLFunction> functions)
: fContext(*context)
- , fInAndUniformVars(*inAndUniformVars)
+ , fInputVars(*inputVars)
, fGLSL(glsl)
, fFormatArgs(std::move(formatArgs))
, fFunctions(std::move(functions)) {}
@@ -157,7 +149,7 @@
}
void emitCode(EmitArgs& args) override {
- for (const auto& v : fInAndUniformVars) {
+ for (const auto& v : fInputVars) {
if (v->fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag && v->fType !=
*fContext.fFragmentProcessor_Type) {
fUniformHandles.push_back(args.fUniformHandler->addUniform(
@@ -238,7 +230,7 @@
size_t offset = 0;
const GrSkSLFP& outer = _proc.cast<GrSkSLFP>();
char* inputs = (char*) outer.fInputs.get();
- for (const auto& v : outer.fFactory->fInAndUniformVars) {
+ for (const auto& v : outer.fFactory->fInputVars) {
switch (get_ctype(fContext, *v)) {
case SkSL::Layout::CType::kSkPMColor: {
float f1 = ((uint8_t*) inputs)[offset++] / 255.0;
@@ -293,7 +285,7 @@
}
const SkSL::Context& fContext;
- const std::vector<const SkSL::Variable*>& fInAndUniformVars;
+ const std::vector<const SkSL::Variable*>& fInputVars;
// nearly-finished GLSL; still contains printf-style "%s" format tokens
const SkSL::String fGLSL;
std::vector<SkSL::Compiler::FormatArg> fFormatArgs;
@@ -394,8 +386,8 @@
printf("%s\n", fFactory->fCompiler.errorText().c_str());
SkASSERT(false);
}
- return new GrGLSLSkSLFP(specialized->fContext.get(), &fFactory->fInAndUniformVars, glsl,
- formatArgs, functions);
+ return new GrGLSLSkSLFP(specialized->fContext.get(), &fFactory->fInputVars, glsl, formatArgs,
+ functions);
}
void GrSkSLFP::onGetGLSLProcessorKey(const GrShaderCaps& caps,
@@ -405,13 +397,13 @@
size_t offset = 0;
char* inputs = (char*) fInputs.get();
const SkSL::Context& context = fFactory->fCompiler.context();
- for (const auto& v : fFactory->fInAndUniformVars) {
+ for (const auto& v : fFactory->fInputVars) {
if (&v->fType == context.fFragmentProcessor_Type.get()) {
continue;
}
switch (get_ctype(context, *v)) {
case SkSL::Layout::CType::kBool:
- if (v->fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) {
+ if (v->fModifiers.fLayout.fKey) {
fKey += inputs[offset];
b->add32(inputs[offset]);
}
@@ -419,7 +411,7 @@
break;
case SkSL::Layout::CType::kInt32: {
offset = SkAlign4(offset);
- if (v->fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) {
+ if (v->fModifiers.fLayout.fKey) {
fKey += inputs[offset + 0];
fKey += inputs[offset + 1];
fKey += inputs[offset + 2];
@@ -431,7 +423,7 @@
}
case SkSL::Layout::CType::kFloat: {
offset = SkAlign4(offset);
- if (v->fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) {
+ if (v->fModifiers.fLayout.fKey) {
fKey += inputs[offset + 0];
fKey += inputs[offset + 1];
fKey += inputs[offset + 2];
@@ -444,7 +436,7 @@
case SkSL::Layout::CType::kSkPMColor:
case SkSL::Layout::CType::kSkPMColor4f:
case SkSL::Layout::CType::kSkRect:
- if (v->fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) {
+ if (v->fModifiers.fLayout.fKey) {
for (size_t i = 0; i < sizeof(float) * 4; ++i) {
fKey += inputs[offset + i];
}
diff --git a/src/gpu/effects/GrSkSLFP.h b/src/gpu/effects/GrSkSLFP.h
index eba33c6..dbeffaf 100644
--- a/src/gpu/effects/GrSkSLFP.h
+++ b/src/gpu/effects/GrSkSLFP.h
@@ -40,12 +40,12 @@
/**
* Creates a new fragment processor from an SkSL source string and a struct of inputs to the
- * program. The input struct's type is derived from the 'in' and 'uniform' variables in the SkSL
- * source, so e.g. the shader:
+ * program. The input struct's type is derived from the 'in' variables in the SkSL source, so
+ * e.g. the shader:
*
* in bool dither;
- * uniform float x;
- * uniform float y;
+ * in float x;
+ * in float y;
* ....
*
* would expect a pointer to a struct set up like:
@@ -56,24 +56,6 @@
* float y;
* };
*
- * While both 'in' and 'uniform' variables go into this struct, the difference between them is
- * that 'in' variables are statically "baked in" to the generated code, becoming literals,
- * whereas uniform variables may be changed from invocation to invocation without having to
- * recompile the shader.
- *
- * As the decision of whether to create a new shader or just upload new uniforms all happens
- * behind the scenes, the difference between the two from an end-user perspective is primarily
- * in performance: on the one hand, changing the value of an 'in' variable is very expensive
- * (requiring the compiler to regenerate the code, upload a new shader to the GPU, and so
- * forth), but on the other hand the compiler can optimize around its value because it is known
- * at compile time. 'in' variables are therefore suitable for things like flags, where there are
- * only a few possible values and a known-in-advance value can cause entire chunks of code to
- * become dead (think static @ifs), while 'uniform's are used for continuous values like colors
- * and coordinates, where it would be silly to create a separate shader for each possible set of
- * values. Other than the (significant) performance implications, the only difference between
- * the two is that 'in' variables can be used in static @if / @switch tests. When in doubt, use
- * 'uniform'.
- *
* As turning SkSL into GLSL / SPIR-V / etc. is fairly expensive, and the output may differ
* based on the inputs, internally the process is divided into two steps: we first parse and
* semantically analyze the SkSL into an internal representation, and then "specialize" this
@@ -192,7 +174,7 @@
std::shared_ptr<SkSL::Program> fBaseProgram;
- std::vector<const SkSL::Variable*> fInAndUniformVars;
+ std::vector<const SkSL::Variable*> fInputVars;
std::unordered_map<SkSL::String, std::unique_ptr<const SkSL::Program>> fSpecializations;
diff --git a/src/sksl/SkSLByteCodeGenerator.cpp b/src/sksl/SkSLByteCodeGenerator.cpp
index d48c9b5..b44b6c9 100644
--- a/src/sksl/SkSLByteCodeGenerator.cpp
+++ b/src/sksl/SkSLByteCodeGenerator.cpp
@@ -66,12 +66,7 @@
if (declVar->fModifiers.fLayout.fBuiltin >= 0) {
continue;
}
- // if you trip this assert, it means the program has raw 'in' variables. You
- // should either specialize the program (Compiler::specialize) to bake in the
- // final values of the 'in' variables, or not use 'in' variables (maybe you
- // meant to use 'uniform' instead?).
- SkASSERT(!(declVar->fModifiers.fFlags & Modifiers::kIn_Flag));
- if (declVar->fModifiers.fFlags & Modifiers::kUniform_Flag) {
+ if (declVar->fModifiers.fFlags & Modifiers::kIn_Flag) {
for (int i = SlotCount(declVar->fType); i > 0; --i) {
fOutput->fInputSlots.push_back(fOutput->fGlobalCount++);
}
diff --git a/tests/SkSLInterpreterTest.cpp b/tests/SkSLInterpreterTest.cpp
index 37afcc0..e54249c 100644
--- a/tests/SkSLInterpreterTest.cpp
+++ b/tests/SkSLInterpreterTest.cpp
@@ -589,7 +589,7 @@
"}\n"
// Uniforms, array-of-structs, dynamic indices
- "uniform Rect gRects[4];\n"
+ "in uniform Rect gRects[4];\n"
"Rect get_rect(int i) { return gRects[i]; }\n"
// Kitchen sink (swizzles, inout, SoAoS)