diff --git a/src/dawn/native/vulkan/RenderPipelineVk.cpp b/src/dawn/native/vulkan/RenderPipelineVk.cpp index ed4727de3fd..264125be0b0 100644 --- a/src/dawn/native/vulkan/RenderPipelineVk.cpp +++ b/src/dawn/native/vulkan/RenderPipelineVk.cpp @@ -485,6 +485,19 @@ ResultOrError RenderPipeline::InitializeSp return {}; }; + // The pixel-center polyfill adds a synthetic inter-stage varying. Its + // Location must match between the vertex and fragment SPIR-V, so compute + // it once here from the vertex stage's output locations (which by WebGPU + // validation are a superset of the fragment stage's input locations). + uint32_t pixelCenterLocation = 0; + if (NeedsPixelCenterPolyfill()) { + const auto& used = + GetStage(SingleShaderStage::Vertex).metadata->usedInterStageVariables; + while (pixelCenterLocation < used.size() && used[pixelCenterLocation]) { + pixelCenterLocation++; + } + } + // Add the vertex stage that's always present. DAWN_TRY(AddShaderStage({ .stage = &GetStage(SingleShaderStage::Vertex), @@ -493,6 +506,7 @@ ResultOrError RenderPipeline::InitializeSp .ycbcrExternalTextures = &specialization.ycbcrExternalTextures, .emitPointSize = GetPrimitiveTopology() == wgpu::PrimitiveTopology::PointList, .polyfillPixelCenter = NeedsPixelCenterPolyfill(), + .pixelCenterLocation = pixelCenterLocation, })); // Add the fragment stage if present. @@ -503,6 +517,7 @@ ResultOrError RenderPipeline::InitializeSp .immediateMask = GetImmediateMask(), .ycbcrExternalTextures = &specialization.ycbcrExternalTextures, .polyfillPixelCenter = NeedsPixelCenterPolyfill(), + .pixelCenterLocation = pixelCenterLocation, .needsMultisampledFramebufferFetch = UseSampleRateShading() && UsesFramebufferFetch(), })); } diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp index f2bf037b65d..59fd6f9eee4 100644 --- a/src/dawn/native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp @@ -269,6 +269,7 @@ ResultOrError ShaderModule::GetHandleAndSpirv( req.tintOptions.emit_vertex_point_size = in.emitPointSize; req.tintOptions.polyfill_pixel_center = in.polyfillPixelCenter; + req.tintOptions.pixel_center_location = in.pixelCenterLocation; req.tintOptions.multisampled_framebuffer_fetch = in.needsMultisampledFramebufferFetch; req.tintOptions.spirv_version = GetDevice()->IsToggleEnabled(Toggle::UseSpirv14) diff --git a/src/dawn/native/vulkan/ShaderModuleVk.h b/src/dawn/native/vulkan/ShaderModuleVk.h index 38de4727c57..a949ead59bd 100644 --- a/src/dawn/native/vulkan/ShaderModuleVk.h +++ b/src/dawn/native/vulkan/ShaderModuleVk.h @@ -74,6 +74,11 @@ class ShaderModule final : public ShaderModuleBase { bool emitPointSize = false; bool polyfillPixelCenter = false; + // The inter-stage Location to use for the pixel-center polyfill varying. + // Must be the same value for both the vertex and fragment stage of a + // pipeline. Computed once at the pipeline level (which can see both + // stages' user-declared locations) rather than per-stage. + uint32_t pixelCenterLocation = 0; bool needsMultisampledFramebufferFetch = false; }; diff --git a/src/tint/lang/spirv/writer/common/options.h b/src/tint/lang/spirv/writer/common/options.h index afde49f0634..f64bee0ef3e 100644 --- a/src/tint/lang/spirv/writer/common/options.h +++ b/src/tint/lang/spirv/writer/common/options.h @@ -233,6 +233,11 @@ struct Options { /// Set to `true` to apply builtin 'position' pixel center emulation. bool polyfill_pixel_center = false; + /// The inter-stage Location to use for the pixel-center polyfill varying. + /// Must be identical for both stages of a pipeline; the caller must compute + /// it once from the union of both stages' user-declared locations. + uint32_t pixel_center_location = 0; + /// Set to `true` if framebuffer fetch should be multisampled bool multisampled_framebuffer_fetch = false; @@ -268,6 +273,7 @@ struct Options { disable_integer_range_analysis, emit_vertex_point_size, polyfill_pixel_center, + pixel_center_location, multisampled_framebuffer_fetch, workarounds, extensions, diff --git a/src/tint/lang/spirv/writer/raise/raise.cc b/src/tint/lang/spirv/writer/raise/raise.cc index b30de709bd4..8b00943bf5b 100644 --- a/src/tint/lang/spirv/writer/raise/raise.cc +++ b/src/tint/lang/spirv/writer/raise/raise.cc @@ -251,6 +251,7 @@ Result Raise(core::ir::Module& module, const Options& options) { .emit_vertex_point_size = options.emit_vertex_point_size, .polyfill_f16_io = !options.extensions.use_storage_input_output_16, .polyfill_pixel_center = options.polyfill_pixel_center, + .pixel_center_location = options.pixel_center_location, .multisampled_framebuffer_fetch = options.multisampled_framebuffer_fetch, .depth_range_offsets = options.depth_range_offsets, })); diff --git a/src/tint/lang/spirv/writer/raise/shader_io.cc b/src/tint/lang/spirv/writer/raise/shader_io.cc index 279b2239982..645debc9041 100644 --- a/src/tint/lang/spirv/writer/raise/shader_io.cc +++ b/src/tint/lang/spirv/writer/raise/shader_io.cc @@ -103,22 +103,17 @@ struct StateImpl : core::ir::transform::ShaderIOBackendState { /// @param addrspace the address to use for the global variables uint32_t AddCenterPosInterpolant(Vector& entries, core::AddressSpace addrspace) { - // Verbose way of finding the smallest free location (id). This of course needs to be the - // same id value for both vertex and fragment. - std::set existing_locations; + // The location *must* be the same for the vertex output and the fragment + // input, but each stage is compiled in isolation and may legally have a + // different set of user IO locations (vertex outputs can be a strict + // superset of fragment inputs). The caller therefore computes a single + // location for the whole pipeline and passes it via the config. + const uint32_t free_location = config.pixel_center_location; + // Defence-in-depth: ensure the chosen location does not collide with a + // user-declared one in this stage. for (auto io : entries) { - if (io.attributes.location.has_value()) { - existing_locations.insert(io.attributes.location.value()); - } - } - uint32_t free_location = 0u; - // We only need to search through existing_locations.size + 1 because either we will simply - // add an index to the end or there will be a hole in the range of locations - for (uint32_t i = 0u; i < (existing_locations.size() + 1); i++) { - if (existing_locations.find(i) == existing_locations.end()) { - free_location = i; - break; - } + TINT_ASSERT(!io.attributes.location.has_value() || + io.attributes.location.value() != free_location); } // For our 'position' -> 'FragCoord' polyfill we must use 'perspective' because diff --git a/src/tint/lang/spirv/writer/raise/shader_io.h b/src/tint/lang/spirv/writer/raise/shader_io.h index 80f87587441..388146ba349 100644 --- a/src/tint/lang/spirv/writer/raise/shader_io.h +++ b/src/tint/lang/spirv/writer/raise/shader_io.h @@ -65,6 +65,9 @@ struct ShaderIOConfig { bool polyfill_f16_io = false; /// true if we should force pixel centers via polyfill when multi-sampling. bool polyfill_pixel_center = false; + /// the inter-stage Location to use for the polyfill varying (must agree + /// between vertex and fragment). + uint32_t pixel_center_location = 0; /// true if the framebuffer fetch should be multisampled bool multisampled_framebuffer_fetch = false; /// offsets for clamping frag depth