diff --git a/src/common/Constants.h b/src/common/Constants.h index 998991f574..d6f42e71c2 100644 --- a/src/common/Constants.h +++ b/src/common/Constants.h @@ -25,6 +25,7 @@ static constexpr uint32_t kNumStages = 3; static constexpr uint8_t kMaxColorAttachments = 8u; static constexpr uint32_t kTextureBytesPerRowAlignment = 256u; static constexpr uint32_t kMaxInterStageShaderComponents = 60u; +static constexpr uint32_t kMaxInterStageShaderVariables = kMaxInterStageShaderComponents / 4; // Compute constants static constexpr uint32_t kMaxComputeWorkgroupStorageSize = 16352u; diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index cb9eac79fa..3831e95beb 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/src/dawn_native/RenderPipeline.cpp @@ -23,6 +23,7 @@ #include "dawn_native/VertexFormat.h" #include +#include namespace dawn_native { // Helper functions @@ -311,6 +312,42 @@ namespace dawn_native { return {}; } + + MaybeError ValidateInterStageMatching(DeviceBase* device, + const VertexState& vertexState, + const FragmentState& fragmentState) { + const EntryPointMetadata& vertexMetadata = + vertexState.module->GetEntryPoint(vertexState.entryPoint); + const EntryPointMetadata& fragmentMetadata = + fragmentState.module->GetEntryPoint(fragmentState.entryPoint); + + if (vertexMetadata.usedInterStageVariables != + fragmentMetadata.usedInterStageVariables) { + return DAWN_VALIDATION_ERROR( + "One or more fragment inputs and vertex outputs are not one-to-one matching"); + } + + auto generateErrorString = [](const char* interStageAttribute, size_t location) { + std::ostringstream stream; + stream << "The " << interStageAttribute << " of the vertex output at location " + << location + << " is different from the one of the fragment input at the same location"; + return stream.str(); + }; + // TODO(dawn:802): Validate interpolation types and interpolition sampling types + for (size_t i : IterateBitSet(vertexMetadata.usedInterStageVariables)) { + const auto& vertexOutputInfo = vertexMetadata.interStageVariables[i]; + const auto& fragmentInputInfo = fragmentMetadata.interStageVariables[i]; + if (vertexOutputInfo.baseType != fragmentInputInfo.baseType) { + return DAWN_VALIDATION_ERROR(generateErrorString("base type", i)); + } + if (vertexOutputInfo.componentCount != fragmentInputInfo.componentCount) { + return DAWN_VALIDATION_ERROR(generateErrorString("componentCount", i)); + } + } + + return {}; + } } // anonymous namespace // Helper functions @@ -362,6 +399,8 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("Should have at least one color target or a depthStencil"); } + DAWN_TRY(ValidateInterStageMatching(device, descriptor->vertex, *(descriptor->fragment))); + return {}; } diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index 619a398cca..69ff2c14e8 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -337,6 +337,38 @@ namespace dawn_native { } } + ResultOrError TintComponentTypeToInterStageComponentType( + tint::inspector::ComponentType type) { + switch (type) { + case tint::inspector::ComponentType::kFloat: + return InterStageComponentType::Float; + case tint::inspector::ComponentType::kSInt: + return InterStageComponentType::Sint; + case tint::inspector::ComponentType::kUInt: + return InterStageComponentType::Uint; + case tint::inspector::ComponentType::kUnknown: + return DAWN_VALIDATION_ERROR( + "Attempted to convert 'Unknown' component type from Tint"); + } + } + + ResultOrError TintCompositionTypeToInterStageComponentCount( + tint::inspector::CompositionType type) { + switch (type) { + case tint::inspector::CompositionType::kScalar: + return 1u; + case tint::inspector::CompositionType::kVec2: + return 2u; + case tint::inspector::CompositionType::kVec3: + return 3u; + case tint::inspector::CompositionType::kVec4: + return 4u; + case tint::inspector::CompositionType::kUnknown: + return DAWN_VALIDATION_ERROR( + "Attempt to convert 'Unknown' composition type from Tint"); + } + } + MaybeError ValidateSpirv(const uint32_t* code, uint32_t codeSize) { spvtools::SpirvTools spirvTools(SPV_ENV_VULKAN_1_1); @@ -910,8 +942,7 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - constexpr uint32_t kMaxInterStageShaderLocation = - kMaxInterStageShaderComponents / 4 - 1; + constexpr uint32_t kMaxInterStageShaderLocation = kMaxInterStageShaderVariables - 1; for (auto& entryPoint : entryPoints) { ASSERT(result.count(entryPoint.name) == 0); @@ -1006,6 +1037,13 @@ namespace dawn_native { ss << "Vertex output location (" << location << ") over limits"; return DAWN_VALIDATION_ERROR(ss.str()); } + metadata->usedInterStageVariables.set(location); + DAWN_TRY_ASSIGN( + metadata->interStageVariables[location].baseType, + TintComponentTypeToInterStageComponentType(output_var.component_type)); + DAWN_TRY_ASSIGN(metadata->interStageVariables[location].componentCount, + TintCompositionTypeToInterStageComponentCount( + output_var.composition_type)); } } @@ -1021,6 +1059,13 @@ namespace dawn_native { ss << "Fragment input location (" << location << ") over limits"; return DAWN_VALIDATION_ERROR(ss.str()); } + metadata->usedInterStageVariables.set(location); + DAWN_TRY_ASSIGN( + metadata->interStageVariables[location].baseType, + TintComponentTypeToInterStageComponentType(input_var.component_type)); + DAWN_TRY_ASSIGN(metadata->interStageVariables[location].componentCount, + TintCompositionTypeToInterStageComponentCount( + input_var.composition_type)); } for (const auto& output_var : entryPoint.output_variables) { diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h index da948e3900..2e501537f0 100644 --- a/src/dawn_native/ShaderModule.h +++ b/src/dawn_native/ShaderModule.h @@ -53,6 +53,13 @@ namespace dawn_native { struct EntryPointMetadata; + // Base component type of an inter-stage variable + enum class InterStageComponentType { + Sint, + Uint, + Float, + }; + using PipelineLayoutEntryPointPair = std::pair; struct PipelineLayoutEntryPointPairHashFunc { size_t operator()(const PipelineLayoutEntryPointPair& pair) const; @@ -157,6 +164,17 @@ namespace dawn_native { fragmentOutputFormatBaseTypes; ityp::bitset fragmentOutputsWritten; + // TODO(dawn:802): store InterpolationType and IntepolationSampling when we add the + // validations on them. + struct InterStageVariableInfo { + InterStageComponentType baseType; + uint32_t componentCount; + }; + // Now that we only support vertex and fragment stages, there can't be both inter-stage + // inputs and outputs in one shader stage. + std::bitset usedInterStageVariables; + std::array interStageVariables; + // The local workgroup size declared for a compute entry point (or 0s otehrwise). Origin3D localWorkgroupSize; diff --git a/src/tests/end2end/TextureViewTests.cpp b/src/tests/end2end/TextureViewTests.cpp index 61116d2fb3..afacd9483b 100644 --- a/src/tests/end2end/TextureViewTests.cpp +++ b/src/tests/end2end/TextureViewTests.cpp @@ -498,7 +498,8 @@ class TextureViewRenderingTest : public DawnTest { renderPassInfo.cColorAttachments[0].clearColor = {1.0f, 0.0f, 0.0f, 1.0f}; const char* oneColorFragmentShader = R"( - [[stage(fragment)]] fn main() -> [[location(0)]] vec4 { + [[stage(fragment)]] fn main([[location(0)]] texCoord : vec2) -> + [[location(0)]] vec4 { return vec4(0.0, 1.0, 0.0, 1.0); } )"; diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp index bd78d8847e..62371e718c 100644 --- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -832,3 +832,124 @@ TEST_F(DepthClampingValidationTest, Success) { device.CreateRenderPipeline(&descriptor); } } + +class InterStageVariableMatchingValidationTest : public RenderPipelineValidationTest { + protected: + void CheckCreatingRenderPipeline(wgpu::ShaderModule vertexModule, + wgpu::ShaderModule fragmentModule, + bool shouldSucceed) { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vertexModule; + descriptor.cFragment.module = fragmentModule; + if (shouldSucceed) { + device.CreateRenderPipeline(&descriptor); + } else { + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); + } + } +}; + +// Tests that creating render pipeline should fail when there is a vertex output that doesn't have +// its corresponding fragment input at the same location, and there is a fragment input that +// doesn't have its corresponding vertex output at the same location. +TEST_F(InterStageVariableMatchingValidationTest, MissingDeclarationAtSameLocation) { + wgpu::ShaderModule vertexModuleOutputAtLocation0 = utils::CreateShaderModule(device, R"( + struct A { + [[location(0)]] vout: f32; + [[builtin(position)]] pos: vec4; + }; + [[stage(vertex)]] fn main() -> A { + var vertexOut: A; + vertexOut.pos = vec4(0.0, 0.0, 0.0, 1.0); + return vertexOut; + })"); + wgpu::ShaderModule fragmentModuleAtLocation0 = utils::CreateShaderModule(device, R"( + struct B { + [[location(0)]] fin: f32; + }; + [[stage(fragment)]] fn main(fragmentIn: B) -> [[location(0)]] vec4 { + return vec4(fragmentIn.fin, 0.0, 0.0, 1.0); + })"); + wgpu::ShaderModule fragmentModuleInputAtLocation1 = utils::CreateShaderModule(device, R"( + struct A { + [[location(1)]] vout: f32; + }; + [[stage(fragment)]] fn main(vertexOut: A) -> [[location(0)]] vec4 { + return vec4(vertexOut.vout, 0.0, 0.0, 1.0); + })"); + wgpu::ShaderModule vertexModuleOutputAtLocation1 = utils::CreateShaderModule(device, R"( + struct B { + [[location(1)]] fin: f32; + [[builtin(position)]] pos: vec4; + }; + [[stage(vertex)]] fn main() -> B { + var fragmentIn: B; + fragmentIn.pos = vec4(0.0, 0.0, 0.0, 1.0); + return fragmentIn; + })"); + + { + CheckCreatingRenderPipeline(vertexModuleOutputAtLocation0, fsModule, false); + CheckCreatingRenderPipeline(vsModule, fragmentModuleAtLocation0, false); + CheckCreatingRenderPipeline(vertexModuleOutputAtLocation0, fragmentModuleInputAtLocation1, + false); + CheckCreatingRenderPipeline(vertexModuleOutputAtLocation1, fragmentModuleAtLocation0, + false); + } + + { + CheckCreatingRenderPipeline(vertexModuleOutputAtLocation0, fragmentModuleAtLocation0, true); + CheckCreatingRenderPipeline(vertexModuleOutputAtLocation1, fragmentModuleInputAtLocation1, + true); + } +} + +// Tests that creating render pipeline should fail when the type of a vertex stage output variable +// doesn't match the type of the fragment stage input variable at the same location. +TEST_F(InterStageVariableMatchingValidationTest, DifferentTypeAtSameLocation) { + constexpr std::array kTypes = {{"f32", "vec2", "vec3", "vec4", + "i32", "vec2", "vec3", "vec4", + "u32", "vec2", "vec3", "vec4"}}; + + std::array vertexModules; + std::array fragmentModules; + for (uint32_t i = 0; i < kTypes.size(); ++i) { + std::string interfaceDeclaration; + { + std::ostringstream sstream; + sstream << "struct A { [[location(0)]] a: " << kTypes[i] << ";" << std::endl; + interfaceDeclaration = sstream.str(); + } + { + std::ostringstream vertexStream; + vertexStream << interfaceDeclaration << R"( + [[builtin(position)]] pos: vec4; + }; + [[stage(vertex)]] fn main() -> A { + var vertexOut: A; + vertexOut.pos = vec4(0.0, 0.0, 0.0, 1.0); + return vertexOut; + })"; + vertexModules[i] = utils::CreateShaderModule(device, vertexStream.str().c_str()); + } + { + std::ostringstream fragmentStream; + fragmentStream << interfaceDeclaration << R"( + }; + [[stage(fragment)]] fn main(fragmentIn: A) -> [[location(0)]] vec4 { + return vec4(0.0, 0.0, 0.0, 1.0); + })"; + fragmentModules[i] = utils::CreateShaderModule(device, fragmentStream.str().c_str()); + } + } + + for (uint32_t vertexModuleIndex = 0; vertexModuleIndex < kTypes.size(); ++vertexModuleIndex) { + wgpu::ShaderModule vertexModule = vertexModules[vertexModuleIndex]; + for (uint32_t fragmentModuleIndex = 0; fragmentModuleIndex < kTypes.size(); + ++fragmentModuleIndex) { + wgpu::ShaderModule fragmentModule = fragmentModules[fragmentModuleIndex]; + bool shouldSuccess = vertexModuleIndex == fragmentModuleIndex; + CheckCreatingRenderPipeline(vertexModule, fragmentModule, shouldSuccess); + } + } +}