Add default uniform binding value for Metal/SPIR-V.
This allows interface blocks in Metal to compile even if
`layout(binding=...)` is not specified. It will also be used in SPIR-V
in the followup CL, when an interface block is automatically synthesized
for top-level uniforms.
This CL also reorganizes the unit tests around uniforms a bit.
Change-Id: Ia898c536b454dda6f51677e232a8f6e6c3606022
Bug: skia:11225
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360778
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
index 08894c1..bc3530f 100644
--- a/gn/sksl_tests.gni
+++ b/gn/sksl_tests.gni
@@ -190,7 +190,6 @@
"/sksl/metal/CastMat2x3ToMat4x4.sksl",
"/sksl/metal/CastMat4x4ToMat3x4.sksl",
"/sksl/metal/CastMat4x4ToMat4x3.sksl",
- "/sksl/metal/InterfaceBlocksRequireBinding.sksl",
"/sksl/metal/NumericGlobals.sksl",
"/sksl/metal/OpaqueTypeInInterfaceBlock.sksl",
"/sksl/metal/OpaqueTypeInStruct.sksl",
@@ -327,9 +326,8 @@
"/sksl/shared/HelloWorld.sksl",
"/sksl/shared/Hex.sksl",
"/sksl/shared/InstanceID.vert",
- "/sksl/shared/InterfaceBlockAnonymous.sksl",
- "/sksl/shared/InterfaceBlockArray.sksl",
"/sksl/shared/InterfaceBlockNamed.sksl",
+ "/sksl/shared/InterfaceBlockNamedArray.sksl",
"/sksl/shared/MatricesFloat.sksl",
"/sksl/shared/MatricesHalf.sksl",
"/sksl/shared/MixedTypeCommaOperator.sksl",
@@ -389,6 +387,8 @@
"/sksl/shared/TextureSharpen.sksl",
"/sksl/shared/UnaryPositiveNegative.sksl",
"/sksl/shared/UniformArray.sksl",
+ "/sksl/shared/UniformBuffers.sksl",
+ "/sksl/shared/Uniforms.sksl",
"/sksl/shared/UnusedVariables.sksl",
"/sksl/shared/VectorConstructors.sksl",
"/sksl/shared/VertexEarlyReturn.vert",
diff --git a/resources/sksl/metal/InterfaceBlocksRequireBinding.sksl b/resources/sksl/metal/InterfaceBlocksRequireBinding.sksl
deleted file mode 100644
index daaa330..0000000
--- a/resources/sksl/metal/InterfaceBlocksRequireBinding.sksl
+++ /dev/null
@@ -1,3 +0,0 @@
-testBlock { int x; };
-
-void main() {}
diff --git a/resources/sksl/shared/InterfaceBlockAnonymous.sksl b/resources/sksl/shared/InterfaceBlockAnonymous.sksl
deleted file mode 100644
index 9c21d58..0000000
--- a/resources/sksl/shared/InterfaceBlockAnonymous.sksl
+++ /dev/null
@@ -1,10 +0,0 @@
-layout(binding=789) uniform testBlock {
- half x;
- half y[2];
- layout(binding=12) half3x2 z;
- bool w;
-};
-
-void main() {
- sk_FragColor = half4(x, y[0], y[1], 0);
-}
diff --git a/resources/sksl/shared/InterfaceBlockArray.sksl b/resources/sksl/shared/InterfaceBlockNamedArray.sksl
similarity index 100%
rename from resources/sksl/shared/InterfaceBlockArray.sksl
rename to resources/sksl/shared/InterfaceBlockNamedArray.sksl
diff --git a/resources/sksl/shared/UniformBuffers.sksl b/resources/sksl/shared/UniformBuffers.sksl
new file mode 100644
index 0000000..f1cf997
--- /dev/null
+++ b/resources/sksl/shared/UniformBuffers.sksl
@@ -0,0 +1,10 @@
+layout(binding=0) uniform testBlock {
+ layout(offset=0) half x;
+ layout(offset=4) int w;
+ layout(offset=16) half y[2];
+ layout(offset=48) half3x3 z;
+};
+
+void main() {
+ sk_FragColor = half4(x, y[0], y[1], 0);
+}
diff --git a/resources/sksl/shared/Uniforms.sksl b/resources/sksl/shared/Uniforms.sksl
new file mode 100644
index 0000000..bb94aa1
--- /dev/null
+++ b/resources/sksl/shared/Uniforms.sksl
@@ -0,0 +1,6 @@
+uniform half myHalf;
+uniform half4 myHalf4;
+
+half4 main() {
+ return myHalf4 * myHalf;
+}
diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp
index 3e61170..4a9fd40 100644
--- a/src/sksl/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/SkSLMetalCodeGenerator.cpp
@@ -1449,6 +1449,16 @@
}
}
+int MetalCodeGenerator::getUniformBinding(const Modifiers& m) {
+ return (m.fLayout.fBinding >= 0) ? m.fLayout.fBinding
+ : fProgram.fSettings.fDefaultUniformBinding;
+}
+
+int MetalCodeGenerator::getUniformSet(const Modifiers& m) {
+ return (m.fLayout.fSet >= 0) ? m.fLayout.fSet
+ : fProgram.fSettings.fDefaultUniformSet;
+}
+
bool MetalCodeGenerator::writeFunctionDeclaration(const FunctionDeclaration& f) {
fRTHeightName = fProgram.fInputs.fRTHeight ? "_globals._anonInterface0->u_skRTHeight" : "";
const char* separator = "";
@@ -1501,17 +1511,12 @@
if (intf.typeName() == "sk_PerVertex") {
continue;
}
- if (intf.variable().modifiers().fLayout.fBinding < 0) {
- fErrors.error(intf.fOffset,
- "Metal interface blocks must have 'layout(binding=...)'");
- return false;
- }
this->write(", constant ");
this->writeBaseType(intf.variable().type());
this->write("& " );
this->write(fInterfaceBlockNameMap[&intf]);
this->write(" [[buffer(");
- this->write(to_string(intf.variable().modifiers().fLayout.fBinding));
+ this->write(to_string(this->getUniformBinding(intf.variable().modifiers())));
this->write(")]]");
}
}
@@ -1940,12 +1945,7 @@
const Variable& var = decls.declaration()->as<VarDeclaration>().var();
if (var.modifiers().fFlags & Modifiers::kUniform_Flag &&
var.type().typeKind() != Type::TypeKind::kSampler) {
- // If the uniform has the `layout(set=N)` attribute, honor that. If not, use the
- // default uniform-set value from settings.
- int uniformSet = var.modifiers().fLayout.fSet;
- if (uniformSet == -1) {
- uniformSet = fProgram.fSettings.fDefaultUniformSet;
- }
+ int uniformSet = this->getUniformSet(var.modifiers());
// Make sure that the program's uniform-set value is consistent throughout.
if (-1 == fUniformBuffer) {
this->write("struct Uniforms {\n");
diff --git a/src/sksl/SkSLMetalCodeGenerator.h b/src/sksl/SkSLMetalCodeGenerator.h
index 0da786e..eee2d4c 100644
--- a/src/sksl/SkSLMetalCodeGenerator.h
+++ b/src/sksl/SkSLMetalCodeGenerator.h
@@ -293,6 +293,10 @@
Requirements requirements(const Statement* s);
+ int getUniformBinding(const Modifiers& m);
+
+ int getUniformSet(const Modifiers& m);
+
std::unordered_map<String, IntrinsicKind> fIntrinsicMap;
std::unordered_set<String> fReservedWords;
std::unordered_map<const Type::Field*, const InterfaceBlock*> fInterfaceBlockMap;
diff --git a/src/sksl/ir/SkSLProgram.h b/src/sksl/ir/SkSLProgram.h
index 8a27156..8147a75 100644
--- a/src/sksl/ir/SkSLProgram.h
+++ b/src/sksl/ir/SkSLProgram.h
@@ -130,9 +130,10 @@
// binding and set number of the uniform buffer.
int fRTHeightBinding = -1;
int fRTHeightSet = -1;
- // If layout(binding=N) is not specified for a uniform, this value will be used. At present,
- // zero is always used by our backends.
+ // If layout(set=S, binding=B) is not specified for a uniform, these values will be used.
+ // At present, zero is always used by our backends.
int fDefaultUniformSet = 0;
+ int fDefaultUniformBinding = 0;
// If true, remove any uncalled functions other than main(). Note that a function which
// starts out being used may end up being uncalled after optimization.
bool fRemoveDeadFunctions = true;
diff --git a/tests/sksl/metal/InterfaceBlocksRequireBinding.metal b/tests/sksl/metal/InterfaceBlocksRequireBinding.metal
deleted file mode 100644
index 84663ba..0000000
--- a/tests/sksl/metal/InterfaceBlocksRequireBinding.metal
+++ /dev/null
@@ -1,4 +0,0 @@
-### Compilation failed:
-
-error: 1: Metal interface blocks must have 'layout(binding=...)'
-1 error
diff --git a/tests/sksl/shared/InterfaceBlockNamedArray.asm.frag b/tests/sksl/shared/InterfaceBlockNamedArray.asm.frag
new file mode 100644
index 0000000..c5dc16d
--- /dev/null
+++ b/tests/sksl/shared/InterfaceBlockNamedArray.asm.frag
@@ -0,0 +1,55 @@
+### Compilation failed:
+
+error: SPIR-V validation error: Uniform id '5' is missing Block or BufferBlock decoration.
+From Vulkan spec, section 14.5.2:
+Such variables must be identified with a Block or BufferBlock decoration
+ %testBlock = OpTypeStruct %float
+
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %sk_FragColor %sk_Clockwise
+OpExecutionMode %main OriginUpperLeft
+OpName %testBlock "testBlock"
+OpMemberName %testBlock 0 "x"
+OpName %sk_FragColor "sk_FragColor"
+OpName %sk_Clockwise "sk_Clockwise"
+OpName %main "main"
+OpMemberDecorate %testBlock 0 Offset 0
+OpDecorate %_arr_testBlock_int_2 ArrayStride 16
+OpDecorate %_arr_testBlock_int_2 Block
+OpDecorate %3 Binding 123
+OpDecorate %3 DescriptorSet 0
+OpDecorate %sk_FragColor RelaxedPrecision
+OpDecorate %sk_FragColor Location 0
+OpDecorate %sk_FragColor Index 0
+OpDecorate %sk_Clockwise RelaxedPrecision
+OpDecorate %sk_Clockwise BuiltIn FrontFacing
+%float = OpTypeFloat 32
+%testBlock = OpTypeStruct %float
+%int = OpTypeInt 32 1
+%int_2 = OpConstant %int 2
+%_arr_testBlock_int_2 = OpTypeArray %testBlock %int_2
+%_ptr_Uniform__arr_testBlock_int_2 = OpTypePointer Uniform %_arr_testBlock_int_2
+%3 = OpVariable %_ptr_Uniform__arr_testBlock_int_2 Uniform
+%v4float = OpTypeVector %float 4
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%sk_FragColor = OpVariable %_ptr_Output_v4float Output
+%bool = OpTypeBool
+%_ptr_Input_bool = OpTypePointer Input %bool
+%sk_Clockwise = OpVariable %_ptr_Input_bool Input
+%void = OpTypeVoid
+%17 = OpTypeFunction %void
+%int_1 = OpConstant %int 1
+%int_0 = OpConstant %int 0
+%_ptr_Uniform_float = OpTypePointer Uniform %float
+%main = OpFunction %void None %17
+%18 = OpLabel
+%21 = OpAccessChain %_ptr_Uniform_float %3 %int_1 %int_0
+%23 = OpLoad %float %21
+%24 = OpCompositeConstruct %v4float %23 %23 %23 %23
+OpStore %sk_FragColor %24
+OpReturn
+OpFunctionEnd
+
+1 error
diff --git a/tests/sksl/shared/InterfaceBlockNamedArray.glsl b/tests/sksl/shared/InterfaceBlockNamedArray.glsl
new file mode 100644
index 0000000..9968988
--- /dev/null
+++ b/tests/sksl/shared/InterfaceBlockNamedArray.glsl
@@ -0,0 +1,8 @@
+
+out vec4 sk_FragColor;
+layout (binding = 123) uniform testBlock {
+ float x;
+} test[2];
+void main() {
+ sk_FragColor = vec4(test[1].x);
+}
diff --git a/tests/sksl/shared/InterfaceBlockNamedArray.metal b/tests/sksl/shared/InterfaceBlockNamedArray.metal
new file mode 100644
index 0000000..cd0beaa
--- /dev/null
+++ b/tests/sksl/shared/InterfaceBlockNamedArray.metal
@@ -0,0 +1,22 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+using namespace metal;
+struct Inputs {
+};
+struct Outputs {
+ float4 sk_FragColor [[color(0)]];
+};
+struct testBlock {
+ float x;
+} test[2];
+struct Globals {
+ constant testBlock* test;
+};
+fragment Outputs fragmentMain(Inputs _in [[stage_in]], constant testBlock& test [[buffer(123)]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
+ Globals _globals{&test};
+ (void)_globals;
+ Outputs _out;
+ (void)_out;
+ _out.sk_FragColor = float4(_uniforms.test[1].x);
+ return _out;
+}
diff --git a/tests/sksl/shared/UniformBuffers.asm.frag b/tests/sksl/shared/UniformBuffers.asm.frag
new file mode 100644
index 0000000..532f019
--- /dev/null
+++ b/tests/sksl/shared/UniformBuffers.asm.frag
@@ -0,0 +1,67 @@
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %sk_FragColor %sk_Clockwise
+OpExecutionMode %main OriginUpperLeft
+OpName %testBlock "testBlock"
+OpMemberName %testBlock 0 "x"
+OpMemberName %testBlock 1 "w"
+OpMemberName %testBlock 2 "y"
+OpMemberName %testBlock 3 "z"
+OpName %sk_FragColor "sk_FragColor"
+OpName %sk_Clockwise "sk_Clockwise"
+OpName %main "main"
+OpDecorate %_arr_float_int_2 ArrayStride 16
+OpMemberDecorate %testBlock 0 Offset 0
+OpMemberDecorate %testBlock 0 RelaxedPrecision
+OpMemberDecorate %testBlock 1 Offset 4
+OpMemberDecorate %testBlock 2 Offset 16
+OpMemberDecorate %testBlock 2 RelaxedPrecision
+OpMemberDecorate %testBlock 3 Offset 48
+OpMemberDecorate %testBlock 3 ColMajor
+OpMemberDecorate %testBlock 3 MatrixStride 16
+OpMemberDecorate %testBlock 3 RelaxedPrecision
+OpDecorate %testBlock Block
+OpDecorate %3 Binding 0
+OpDecorate %3 DescriptorSet 0
+OpDecorate %sk_FragColor RelaxedPrecision
+OpDecorate %sk_FragColor Location 0
+OpDecorate %sk_FragColor Index 0
+OpDecorate %sk_Clockwise RelaxedPrecision
+OpDecorate %sk_Clockwise BuiltIn FrontFacing
+OpDecorate %24 RelaxedPrecision
+OpDecorate %26 RelaxedPrecision
+OpDecorate %29 RelaxedPrecision
+%float = OpTypeFloat 32
+%int = OpTypeInt 32 1
+%int_2 = OpConstant %int 2
+%_arr_float_int_2 = OpTypeArray %float %int_2
+%v3float = OpTypeVector %float 3
+%mat3v3float = OpTypeMatrix %v3float 3
+%testBlock = OpTypeStruct %float %int %_arr_float_int_2 %mat3v3float
+%_ptr_Uniform_testBlock = OpTypePointer Uniform %testBlock
+%3 = OpVariable %_ptr_Uniform_testBlock Uniform
+%v4float = OpTypeVector %float 4
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%sk_FragColor = OpVariable %_ptr_Output_v4float Output
+%bool = OpTypeBool
+%_ptr_Input_bool = OpTypePointer Input %bool
+%sk_Clockwise = OpVariable %_ptr_Input_bool Input
+%void = OpTypeVoid
+%19 = OpTypeFunction %void
+%int_0 = OpConstant %int 0
+%_ptr_Uniform_float = OpTypePointer Uniform %float
+%int_1 = OpConstant %int 1
+%float_0 = OpConstant %float 0
+%main = OpFunction %void None %19
+%20 = OpLabel
+%22 = OpAccessChain %_ptr_Uniform_float %3 %int_0
+%24 = OpLoad %float %22
+%25 = OpAccessChain %_ptr_Uniform_float %3 %int_2 %int_0
+%26 = OpLoad %float %25
+%28 = OpAccessChain %_ptr_Uniform_float %3 %int_2 %int_1
+%29 = OpLoad %float %28
+%31 = OpCompositeConstruct %v4float %24 %26 %29 %float_0
+OpStore %sk_FragColor %31
+OpReturn
+OpFunctionEnd
diff --git a/tests/sksl/shared/UniformBuffers.glsl b/tests/sksl/shared/UniformBuffers.glsl
new file mode 100644
index 0000000..9c966c0
--- /dev/null
+++ b/tests/sksl/shared/UniformBuffers.glsl
@@ -0,0 +1,11 @@
+
+out vec4 sk_FragColor;
+layout (binding = 0) uniform testBlock {
+ layout (offset = 0) float x;
+ layout (offset = 4) int w;
+ layout (offset = 16) float[2] y;
+ layout (offset = 48) mat3 z;
+};
+void main() {
+ sk_FragColor = vec4(x, y[0], y[1], 0.0);
+}
diff --git a/tests/sksl/shared/UniformBuffers.metal b/tests/sksl/shared/UniformBuffers.metal
new file mode 100644
index 0000000..6e71149
--- /dev/null
+++ b/tests/sksl/shared/UniformBuffers.metal
@@ -0,0 +1,27 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+using namespace metal;
+struct Inputs {
+};
+struct Outputs {
+ float4 sk_FragColor [[color(0)]];
+};
+struct testBlock {
+ float x;
+ int w;
+ char pad0[8];
+ float y[2];
+ char pad1[24];
+ float3x3 z;
+};
+struct Globals {
+ constant testBlock* _anonInterface0;
+};
+fragment Outputs fragmentMain(Inputs _in [[stage_in]], constant testBlock& _anonInterface0 [[buffer(0)]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
+ Globals _globals{&_anonInterface0};
+ (void)_globals;
+ Outputs _out;
+ (void)_out;
+ _out.sk_FragColor = float4(_globals._anonInterface0->x, _globals._anonInterface0->y[0], _globals._anonInterface0->y[1], 0.0);
+ return _out;
+}
diff --git a/tests/sksl/shared/Uniforms.asm.frag b/tests/sksl/shared/Uniforms.asm.frag
new file mode 100644
index 0000000..12803a1
--- /dev/null
+++ b/tests/sksl/shared/Uniforms.asm.frag
@@ -0,0 +1,58 @@
+### Compilation failed:
+
+error: SPIR-V validation error: Uniform OpVariable <id> '10[%myHalf]' has illegal type.
+From Vulkan spec, section 14.5.2:
+Variables identified with the Uniform storage class are used to access transparent buffer backed resources. Such variables must be typed as OpTypeStruct, or an array of this type
+ %myHalf = OpVariable %_ptr_Uniform_float Uniform
+
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %_entrypoint "_entrypoint" %sk_FragColor %sk_Clockwise
+OpExecutionMode %_entrypoint OriginUpperLeft
+OpName %sk_FragColor "sk_FragColor"
+OpName %sk_Clockwise "sk_Clockwise"
+OpName %myHalf "myHalf"
+OpName %myHalf4 "myHalf4"
+OpName %_entrypoint "_entrypoint"
+OpName %main "main"
+OpDecorate %sk_FragColor RelaxedPrecision
+OpDecorate %sk_FragColor Location 0
+OpDecorate %sk_FragColor Index 0
+OpDecorate %sk_Clockwise RelaxedPrecision
+OpDecorate %sk_Clockwise BuiltIn FrontFacing
+OpDecorate %myHalf RelaxedPrecision
+OpDecorate %myHalf DescriptorSet 0
+OpDecorate %myHalf4 RelaxedPrecision
+OpDecorate %myHalf4 DescriptorSet 0
+OpDecorate %21 RelaxedPrecision
+OpDecorate %22 RelaxedPrecision
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%sk_FragColor = OpVariable %_ptr_Output_v4float Output
+%bool = OpTypeBool
+%_ptr_Input_bool = OpTypePointer Input %bool
+%sk_Clockwise = OpVariable %_ptr_Input_bool Input
+%_ptr_Uniform_float = OpTypePointer Uniform %float
+%myHalf = OpVariable %_ptr_Uniform_float Uniform
+%_ptr_Uniform_v4float = OpTypePointer Uniform %v4float
+%myHalf4 = OpVariable %_ptr_Uniform_v4float Uniform
+%void = OpTypeVoid
+%16 = OpTypeFunction %void
+%19 = OpTypeFunction %v4float
+%_entrypoint = OpFunction %void None %16
+%17 = OpLabel
+%18 = OpFunctionCall %v4float %main
+OpStore %sk_FragColor %18
+OpReturn
+OpFunctionEnd
+%main = OpFunction %v4float None %19
+%20 = OpLabel
+%21 = OpLoad %v4float %myHalf4
+%22 = OpLoad %float %myHalf
+%23 = OpVectorTimesScalar %v4float %21 %22
+OpReturnValue %23
+OpFunctionEnd
+
+1 error
diff --git a/tests/sksl/shared/Uniforms.glsl b/tests/sksl/shared/Uniforms.glsl
new file mode 100644
index 0000000..d93a450
--- /dev/null
+++ b/tests/sksl/shared/Uniforms.glsl
@@ -0,0 +1,7 @@
+
+out vec4 sk_FragColor;
+uniform float myHalf;
+uniform vec4 myHalf4;
+vec4 main() {
+ return myHalf4 * myHalf;
+}
diff --git a/tests/sksl/shared/Uniforms.metal b/tests/sksl/shared/Uniforms.metal
new file mode 100644
index 0000000..b40ed47
--- /dev/null
+++ b/tests/sksl/shared/Uniforms.metal
@@ -0,0 +1,20 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+using namespace metal;
+struct Uniforms {
+ float myHalf;
+ float4 myHalf4;
+};
+struct Inputs {
+};
+struct Outputs {
+ float4 sk_FragColor [[color(0)]];
+};
+
+
+fragment Outputs fragmentMain(Inputs _in [[stage_in]], constant Uniforms& _uniforms [[buffer(0)]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
+ Outputs _out;
+ (void)_out;
+ _out.sk_FragColor = _uniforms.myHalf4 * _uniforms.myHalf;
+ return _out;
+}
