Remove SPIRVDawnCompatFlag from SkSL.
In practice this was only used for synthesizing separate texture
sampler pairs. We can get the same information with layout flags
instead, allowing us to remove the setting.
This code can be removed entirely once we've fully transitioned
to the WGSL backend for WebGPU and stop relying on the SPIR-V
reader.
Change-Id: I18f5ccb707a9e4d42fa85a68bd13a3139d729f33
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/749141
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
index c6102b7..0dc1036 100644
--- a/gn/sksl_tests.gni
+++ b/gn/sksl_tests.gni
@@ -370,6 +370,7 @@
"spirv/InterfaceBlockPushConstant.sksl",
"spirv/LayoutMultipleOf4.sksl",
"spirv/LayoutOutOfOrder.sksl",
+ "spirv/MixedSamplerTypes.sksl",
"spirv/OpaqueTypeInArray.sksl",
"spirv/Ossfuzz35916.sksl",
"spirv/Ossfuzz37627.sksl",
@@ -377,8 +378,7 @@
"spirv/Ossfuzz53202.sksl",
"spirv/StructArrayMemberInDifferentLayouts.sksl",
"spirv/UnusedInterfaceBlock.sksl",
- "spirv/WGSLLayoutInVulkanSPIRV.sksl",
- "spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.sksl",
+ "spirv/WrongCombinedSamplerLayoutForWebGPUSampler.sksl",
"workarounds/RewriteMatrixVectorMultiply.sksl",
]
diff --git a/resources/sksl/BUILD.bazel b/resources/sksl/BUILD.bazel
index 7679d79..4bff927 100644
--- a/resources/sksl/BUILD.bazel
+++ b/resources/sksl/BUILD.bazel
@@ -1047,6 +1047,7 @@
"spirv/InterfaceBlockPushConstant.sksl",
"spirv/LayoutMultipleOf4.sksl",
"spirv/LayoutOutOfOrder.sksl",
+ "spirv/MixedSamplerTypes.sksl",
"spirv/OpaqueTypeInArray.sksl",
"spirv/Ossfuzz35916.sksl",
"spirv/Ossfuzz37627.sksl",
@@ -1054,8 +1055,7 @@
"spirv/Ossfuzz53202.sksl",
"spirv/StructArrayMemberInDifferentLayouts.sksl",
"spirv/UnusedInterfaceBlock.sksl",
- "spirv/WGSLLayoutInVulkanSPIRV.sksl",
- "spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.sksl",
+ "spirv/WrongCombinedSamplerLayoutForWebGPUSampler.sksl",
"workarounds/RewriteMatrixVectorMultiply.sksl",
],
)
diff --git a/resources/sksl/spirv/CombinedSamplerTypeDawnCompatMode.sksl b/resources/sksl/spirv/CombinedSamplerTypeDawnCompatMode.sksl
index e2c8db2..d68f478 100644
--- a/resources/sksl/spirv/CombinedSamplerTypeDawnCompatMode.sksl
+++ b/resources/sksl/spirv/CombinedSamplerTypeDawnCompatMode.sksl
@@ -1,4 +1,3 @@
-/*#pragma settings SPIRVDawnCompatMode*/
layout(webgpu, set=1, texture=2, sampler=3) sampler2D aSampler;
layout(webgpu, set=1, texture=4, sampler=5) sampler2D anotherSampler;
diff --git a/resources/sksl/spirv/FunctionParametersOfTextureAndSamplerTypeDawnCompatMode.sksl b/resources/sksl/spirv/FunctionParametersOfTextureAndSamplerTypeDawnCompatMode.sksl
index e00625d..e55f5e4 100644
--- a/resources/sksl/spirv/FunctionParametersOfTextureAndSamplerTypeDawnCompatMode.sksl
+++ b/resources/sksl/spirv/FunctionParametersOfTextureAndSamplerTypeDawnCompatMode.sksl
@@ -1,4 +1,3 @@
-/*#pragma settings SPIRVDawnCompatMode*/
layout(set = 0, binding = 1) texture2D aTexture;
layout(webgpu, set = 0, texture = 2, sampler = 3) sampler2D aSampledTexture;
diff --git a/resources/sksl/spirv/MixedSamplerTypes.sksl b/resources/sksl/spirv/MixedSamplerTypes.sksl
new file mode 100644
index 0000000..295508d
--- /dev/null
+++ b/resources/sksl/spirv/MixedSamplerTypes.sksl
@@ -0,0 +1,7 @@
+layout(webgpu, set=1, texture=2, sampler=3) sampler2D wgpuSampler;
+layout(vulkan, set=1, binding=4) sampler2D vkSampler;
+
+void main() {
+ sk_FragColor = sample(wgpuSampler, float2(0));
+ sk_FragColor = sample(vkSampler, float2(0));
+}
diff --git a/resources/sksl/spirv/WGSLLayoutInVulkanSPIRV.sksl b/resources/sksl/spirv/WGSLLayoutInVulkanSPIRV.sksl
deleted file mode 100644
index 0373779..0000000
--- a/resources/sksl/spirv/WGSLLayoutInVulkanSPIRV.sksl
+++ /dev/null
@@ -1,9 +0,0 @@
-layout(webgpu, set=1, texture=2, sampler=3) sampler2D aSampler;
-
-void main() {
- sk_FragColor = sample(aSampler, float2(0));
-}
-
-/*%%*
-incompatible backend flag in SPIR-V codegen
-*%%*/
diff --git a/resources/sksl/spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.sksl b/resources/sksl/spirv/WrongCombinedSamplerLayoutForWebGPUSampler.sksl
similarity index 64%
rename from resources/sksl/spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.sksl
rename to resources/sksl/spirv/WrongCombinedSamplerLayoutForWebGPUSampler.sksl
index b238d54..bf9a456 100644
--- a/resources/sksl/spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.sksl
+++ b/resources/sksl/spirv/WrongCombinedSamplerLayoutForWebGPUSampler.sksl
@@ -1,5 +1,4 @@
-/*#pragma settings SPIRVDawnCompatMode*/
-layout(set=0, binding=0) sampler2D aSampler;
+layout(webgpu, set=0, binding=0) sampler2D aSampler;
void main() {
sk_FragColor = sample(aSampler, float2(0));
diff --git a/src/gpu/graphite/dawn/DawnGraphicsPipeline.cpp b/src/gpu/graphite/dawn/DawnGraphicsPipeline.cpp
index bef7545..6348b08 100644
--- a/src/gpu/graphite/dawn/DawnGraphicsPipeline.cpp
+++ b/src/gpu/graphite/dawn/DawnGraphicsPipeline.cpp
@@ -262,7 +262,6 @@
SkSL::ProgramSettings settings;
settings.fForceNoRTFlip = true;
- settings.fSPIRVDawnCompatMode = true;
ShaderErrorHandler* errorHandler = caps.shaderErrorHandler();
diff --git a/src/sksl/SkSLProgramSettings.h b/src/sksl/SkSLProgramSettings.h
index 1d53c8a..dd27de0 100644
--- a/src/sksl/SkSLProgramSettings.h
+++ b/src/sksl/SkSLProgramSettings.h
@@ -76,9 +76,6 @@
// If true, VarDeclaration can be cloned for testing purposes. See VarDeclaration::clone for
// more information.
bool fAllowVarDeclarationCloneForTesting = false;
- // If true, SPIR-V codegen restricted to a subset supported by Dawn.
- // TODO(skia:13840, skia:14023): Remove this setting when Skia can use WGSL on Dawn.
- bool fSPIRVDawnCompatMode = false;
};
/**
diff --git a/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp b/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp
index 61954cc..aee9c6e 100644
--- a/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp
@@ -1180,8 +1180,7 @@
words.push_back(Word::Result());
words.push_back(this->getType(function.returnType()));
for (const Variable* parameter : function.parameters()) {
- if (parameter->type().typeKind() == Type::TypeKind::kSampler &&
- fProgram.fConfig->fSettings.fSPIRVDawnCompatMode) {
+ if (fUseTextureSamplerPairs && parameter->type().isSampler()) {
words.push_back(this->getFunctionParameterType(parameter->type().textureType()));
words.push_back(this->getFunctionParameterType(*fContext.fTypes.fSampler));
} else {
@@ -1879,18 +1878,19 @@
const Variable* var = arg.as<VariableReference>().variable();
// In Dawn-mode the texture and sampler arguments are forwarded to the helper function.
- if (const auto* p = fSynthesizedSamplerMap.find(var)) {
- SkASSERT(fProgram.fConfig->fSettings.fSPIRVDawnCompatMode);
- SkASSERT(arg.type().typeKind() == Type::TypeKind::kSampler);
- SkASSERT(outSynthesizedSamplerId);
+ if (fUseTextureSamplerPairs && var->type().isSampler()) {
+ if (const auto* p = fSynthesizedSamplerMap.find(var)) {
+ SkASSERT(outSynthesizedSamplerId);
- SpvId* img = fVariableMap.find((*p)->fTexture.get());
- SpvId* sampler = fVariableMap.find((*p)->fSampler.get());
- SkASSERT(img);
- SkASSERT(sampler);
+ SpvId* img = fVariableMap.find((*p)->fTexture.get());
+ SpvId* sampler = fVariableMap.find((*p)->fSampler.get());
+ SkASSERT(img);
+ SkASSERT(sampler);
- *outSynthesizedSamplerId = *sampler;
- return *img;
+ *outSynthesizedSamplerId = *sampler;
+ return *img;
+ }
+ SkDEBUGFAIL("sampler missing from fSynthesizedSamplerMap");
}
SpvId* entry = fVariableMap.find(var);
@@ -2861,30 +2861,30 @@
//
// Variable references to opaque handles (texture/sampler) that appear as the argument
// of a user-defined function call are explicitly handled in writeFunctionCallArgument.
- if (const auto* p = fSynthesizedSamplerMap.find(variable)) {
- SkASSERT(fProgram.fConfig->fSettings.fSPIRVDawnCompatMode);
+ if (fUseTextureSamplerPairs && variable->type().isSampler()) {
+ if (const auto* p = fSynthesizedSamplerMap.find(variable)) {
+ SpvId* imgPtr = fVariableMap.find((*p)->fTexture.get());
+ SpvId* samplerPtr = fVariableMap.find((*p)->fSampler.get());
+ SkASSERT(imgPtr);
+ SkASSERT(samplerPtr);
- SpvId* imgPtr = fVariableMap.find((*p)->fTexture.get());
- SpvId* samplerPtr = fVariableMap.find((*p)->fSampler.get());
- SkASSERT(imgPtr);
- SkASSERT(samplerPtr);
-
- SpvId img = this->writeOpLoad(this->getType((*p)->fTexture->type()),
- Precision::kDefault, *imgPtr, out);
- SpvId sampler = this->writeOpLoad(this->getType((*p)->fSampler->type()),
- Precision::kDefault,
- *samplerPtr,
- out);
- SpvId result = this->nextId(nullptr);
- this->writeInstruction(SpvOpSampledImage,
- this->getType(variable->type()),
- result,
- img,
- sampler,
- out);
- return result;
+ SpvId img = this->writeOpLoad(this->getType((*p)->fTexture->type()),
+ Precision::kDefault, *imgPtr, out);
+ SpvId sampler = this->writeOpLoad(this->getType((*p)->fSampler->type()),
+ Precision::kDefault,
+ *samplerPtr,
+ out);
+ SpvId result = this->nextId(nullptr);
+ this->writeInstruction(SpvOpSampledImage,
+ this->getType(variable->type()),
+ result,
+ img,
+ sampler,
+ out);
+ return result;
+ }
+ SkDEBUGFAIL("sampler missing from fSynthesizedSamplerMap");
}
-
return this->getLValue(ref, out)->load(out);
}
}
@@ -3629,8 +3629,7 @@
std::string_view(mangledName.c_str(), mangledName.size()),
fNameBuffer);
for (const Variable* parameter : f.parameters()) {
- if (parameter->type().typeKind() == Type::TypeKind::kSampler &&
- fProgram.fConfig->fSettings.fSPIRVDawnCompatMode) {
+ if (fUseTextureSamplerPairs && parameter->type().isSampler()) {
auto [texture, sampler] = this->synthesizeTextureAndSampler(*parameter);
SpvId textureId = this->nextId(nullptr);
@@ -3870,11 +3869,9 @@
bool SPIRVCodeGenerator::writeGlobalVarDeclaration(ProgramKind kind,
const VarDeclaration& varDecl) {
const Variable* var = varDecl.var();
- const bool inDawnMode = fProgram.fConfig->fSettings.fSPIRVDawnCompatMode;
const LayoutFlags backendFlags = var->layout().fFlags & LayoutFlag::kAllBackends;
- const LayoutFlags permittedBackendFlags =
- LayoutFlag::kVulkan | (inDawnMode ? LayoutFlag::kWebGPU : LayoutFlag::kNone);
- if (backendFlags & ~permittedBackendFlags) {
+ const LayoutFlags kPermittedBackendFlags = LayoutFlag::kVulkan | LayoutFlag::kWebGPU;
+ if (backendFlags & ~kPermittedBackendFlags) {
fContext.fErrors->error(var->fPosition, "incompatible backend flag in SPIR-V codegen");
return false;
}
@@ -3897,12 +3894,10 @@
return true;
}
- if (var->type().typeKind() == Type::TypeKind::kSampler && inDawnMode) {
- if (var->layout().fTexture == -1 ||
- var->layout().fSampler == -1 ||
- !(var->layout().fFlags & LayoutFlag::kWebGPU)) {
- fContext.fErrors->error(var->fPosition, "SPIR-V dawn compatibility mode requires an "
- "explicit texture and sampler index");
+ if (fUseTextureSamplerPairs && var->type().isSampler()) {
+ if (var->layout().fTexture == -1 || var->layout().fSampler == -1) {
+ fContext.fErrors->error(var->fPosition, "WebGPU samplers require explicit texture and "
+ "sampler indices");
return false;
}
SkASSERT(storageClass == SpvStorageClassUniformConstant);
@@ -4442,7 +4437,7 @@
std::tuple<const Variable*, const Variable*> SPIRVCodeGenerator::synthesizeTextureAndSampler(
const Variable& combinedSampler) {
- SkASSERT(fProgram.fConfig->fSettings.fSPIRVDawnCompatMode);
+ SkASSERT(fUseTextureSamplerPairs);
SkASSERT(combinedSampler.type().typeKind() == Type::TypeKind::kSampler);
const Layout& layout = combinedSampler.layout();
@@ -4489,31 +4484,59 @@
void SPIRVCodeGenerator::writeInstructions(const Program& program, OutputStream& out) {
fGLSLExtendedInstructions = this->nextId(nullptr);
StringStream body;
- // Assign SpvIds to functions.
+
+ // Do an initial pass over the program elements to establish some baseline info.
const FunctionDeclaration* main = nullptr;
- // During the same iteration, collect the local size values to assign if this is a compute
- // program. Dimensions that are not present get assigned a value of 1.
int localSizeX = 1, localSizeY = 1, localSizeZ = 1;
+ Position vulkanSamplerPos;
+ Position webGPUSamplerPos;
for (const ProgramElement* e : program.elements()) {
- if (e->is<FunctionDefinition>()) {
- const FunctionDefinition& funcDef = e->as<FunctionDefinition>();
- const FunctionDeclaration& funcDecl = funcDef.declaration();
- fFunctionMap.set(&funcDecl, this->nextId(nullptr));
- if (funcDecl.isMain()) {
- main = &funcDecl;
+ switch (e->kind()) {
+ case ProgramElement::Kind::kFunction: {
+ // Assign SpvIds to functions.
+ const FunctionDefinition& funcDef = e->as<FunctionDefinition>();
+ const FunctionDeclaration& funcDecl = funcDef.declaration();
+ fFunctionMap.set(&funcDecl, this->nextId(nullptr));
+ if (funcDecl.isMain()) {
+ main = &funcDecl;
+ }
+ break;
}
- } else if (ProgramConfig::IsCompute(program.fConfig->fKind) &&
- e->is<ModifiersDeclaration>()) {
- const ModifiersDeclaration& modifiers = e->as<ModifiersDeclaration>();
- if (modifiers.layout().fLocalSizeX >= 0) {
- localSizeX = modifiers.layout().fLocalSizeX;
+ case ProgramElement::Kind::kGlobalVar: {
+ // Look for sampler variables and determine whether or not this program uses
+ // combined samplers or separate samplers. The layout backend will be marked as
+ // WebGPU for separate samplers, or Vulkan for combined samplers.
+ const GlobalVarDeclaration& decl = e->as<GlobalVarDeclaration>();
+ const Variable& var = *decl.varDeclaration().var();
+ if (var.type().isSampler()) {
+ if (var.layout().fFlags & LayoutFlag::kVulkan) {
+ vulkanSamplerPos = decl.position();
+ }
+ if (var.layout().fFlags & LayoutFlag::kWebGPU) {
+ webGPUSamplerPos = decl.position();
+ }
+ }
+ break;
}
- if (modifiers.layout().fLocalSizeY >= 0) {
- localSizeY = modifiers.layout().fLocalSizeY;
+ case ProgramElement::Kind::kModifiers: {
+ // If this is a compute program, collect the local-size values. Dimensions that are
+ // not present will be assigned a value of 1.
+ if (ProgramConfig::IsCompute(program.fConfig->fKind)) {
+ const ModifiersDeclaration& modifiers = e->as<ModifiersDeclaration>();
+ if (modifiers.layout().fLocalSizeX >= 0) {
+ localSizeX = modifiers.layout().fLocalSizeX;
+ }
+ if (modifiers.layout().fLocalSizeY >= 0) {
+ localSizeY = modifiers.layout().fLocalSizeY;
+ }
+ if (modifiers.layout().fLocalSizeZ >= 0) {
+ localSizeZ = modifiers.layout().fLocalSizeZ;
+ }
+ }
+ break;
}
- if (modifiers.layout().fLocalSizeZ >= 0) {
- localSizeZ = modifiers.layout().fLocalSizeZ;
- }
+ default:
+ break;
}
}
@@ -4522,6 +4545,15 @@
fContext.fErrors->error(Position(), "program does not contain a main() function");
return;
}
+ // Make sure our program's sampler usage is consistent.
+ if (vulkanSamplerPos.valid() && webGPUSamplerPos.valid()) {
+ fContext.fErrors->error(Position(), "programs cannot contain a mixture of sampler types");
+ fContext.fErrors->error(vulkanSamplerPos, "Vulkan sampler found here:");
+ fContext.fErrors->error(webGPUSamplerPos, "WebGPU sampler found here:");
+ return;
+ }
+ fUseTextureSamplerPairs = webGPUSamplerPos.valid();
+
// Emit interface blocks.
std::set<SpvId> interfaceVars;
for (const ProgramElement* e : program.elements()) {
diff --git a/src/sksl/codegen/SkSLSPIRVCodeGenerator.h b/src/sksl/codegen/SkSLSPIRVCodeGenerator.h
index 4d8509b..a68bfb8 100644
--- a/src/sksl/codegen/SkSLSPIRVCodeGenerator.h
+++ b/src/sksl/codegen/SkSLSPIRVCodeGenerator.h
@@ -561,8 +561,9 @@
StringStream fDecorationBuffer;
// Mapping from combined sampler declarations to synthesized texture/sampler variables.
- // This is only used if the SPIRVDawnCompatMode setting is enabled.
- // TODO(skia:14023): Remove when WGSL codegen is complete
+ // This is used when the sampler is declared as `layout(webgpu)`.
+ // TODO(skia:14023): Remove when WebGPU backend is fully transitioned to WGSL codegen.
+ bool fUseTextureSamplerPairs = false;
struct SynthesizedTextureSamplerPair {
// The names of the synthesized variables. The Variable objects themselves store string
// views referencing these strings. It is important for the std::string instances to have a
diff --git a/tests/sksl/spirv/MixedSamplerTypes.asm.frag b/tests/sksl/spirv/MixedSamplerTypes.asm.frag
new file mode 100644
index 0000000..96cfcbc
--- /dev/null
+++ b/tests/sksl/spirv/MixedSamplerTypes.asm.frag
@@ -0,0 +1,10 @@
+### Compilation failed:
+
+error: programs cannot contain a mixture of sampler types
+error: 2: Vulkan sampler found here:
+layout(vulkan, set=1, binding=4) sampler2D vkSampler;
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+error: 1: WebGPU sampler found here:
+layout(webgpu, set=1, texture=2, sampler=3) sampler2D wgpuSampler;
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+3 errors
diff --git a/tests/sksl/spirv/WGSLLayoutInVulkanSPIRV.asm.frag b/tests/sksl/spirv/WGSLLayoutInVulkanSPIRV.asm.frag
deleted file mode 100644
index 7075a6c..0000000
--- a/tests/sksl/spirv/WGSLLayoutInVulkanSPIRV.asm.frag
+++ /dev/null
@@ -1,6 +0,0 @@
-### Compilation failed:
-
-error: 1: incompatible backend flag in SPIR-V codegen
-layout(webgpu, set=1, texture=2, sampler=3) sampler2D aSampler;
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-1 error
diff --git a/tests/sksl/spirv/WrongCombinedSamplerLayoutForWebGPUSampler.asm.frag b/tests/sksl/spirv/WrongCombinedSamplerLayoutForWebGPUSampler.asm.frag
new file mode 100644
index 0000000..5d48f08
--- /dev/null
+++ b/tests/sksl/spirv/WrongCombinedSamplerLayoutForWebGPUSampler.asm.frag
@@ -0,0 +1,6 @@
+### Compilation failed:
+
+error: 1: WebGPU samplers require explicit texture and sampler indices
+layout(webgpu, set=0, binding=0) sampler2D aSampler;
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+1 error
diff --git a/tests/sksl/spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.asm.frag b/tests/sksl/spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.asm.frag
deleted file mode 100644
index 09588ab..0000000
--- a/tests/sksl/spirv/WrongCombinedSamplerLayoutSPIRVDawnCompatMode.asm.frag
+++ /dev/null
@@ -1,6 +0,0 @@
-### Compilation failed:
-
-error: 2: SPIR-V dawn compatibility mode requires an explicit texture and sampler index
-layout(set=0, binding=0) sampler2D aSampler;
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-1 error
diff --git a/tools/skslc/Main.cpp b/tools/skslc/Main.cpp
index 2e1b333..5311e77 100644
--- a/tools/skslc/Main.cpp
+++ b/tools/skslc/Main.cpp
@@ -442,9 +442,6 @@
settings->fOptimize = false;
*debugTrace = std::make_unique<SkSL::DebugTracePriv>();
}
- if (consume_suffix(&settingsText, " SPIRVDawnCompatMode")) {
- settings->fSPIRVDawnCompatMode = true;
- }
if (settingsText.empty()) {
break;
