[Compat GL] Fix emulated uniform setup on pipeline change
Current implementation only update the emulated uniforms
for texture builtins and array length on bindgroup changes,
but not on pipeline change.
Mark the bind group of the emulated internal uniform data
dirty on pipeline change. So they will be updated properly.
Add related tests where 2 pipelines sharing the same bind group
layout is changed but the bind group is unchanged.
Bug: 425579202
Change-Id: I2dbf7d964d9d373e6489c914d771c271c26fae8f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/247454
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Shrek Shao <shrekshao@google.com>
diff --git a/src/dawn/native/opengl/CommandBufferGL.cpp b/src/dawn/native/opengl/CommandBufferGL.cpp
index fb6a3a6..433a73e 100644
--- a/src/dawn/native/opengl/CommandBufferGL.cpp
+++ b/src/dawn/native/opengl/CommandBufferGL.cpp
@@ -286,15 +286,13 @@
void OnSetPipeline(RenderPipeline* pipeline) {
BindGroupTrackerBase::OnSetPipeline(pipeline);
mPipeline = pipeline;
- ResetInternalUniformDataDirtyRange();
- ResetInternalUniformDataDirtyRangeArrayLength();
+ ResetInternalUniformDataBindgroupAndDirtyRange();
}
void OnSetPipeline(ComputePipeline* pipeline) {
BindGroupTrackerBase::OnSetPipeline(pipeline);
mPipeline = pipeline;
- ResetInternalUniformDataDirtyRange();
- ResetInternalUniformDataDirtyRangeArrayLength();
+ ResetInternalUniformDataBindgroupAndDirtyRange();
}
MaybeError Apply(const OpenGLFunctions& gl) {
@@ -599,6 +597,20 @@
mDirtyRangeArrayLength = {mInternalArrayLengthBufferData.size(), 0};
}
+ void ResetInternalUniformDataBindgroupAndDirtyRange() {
+ // Mark bind groups that need emulated builtin uniforms dirty so that they can be updated
+ // properly, even if the bind group is not updated.
+ // TODO(crbug.com/408065421): This forces bindgroups with metadata to be completely set
+ // again each pipeline change. In the future we will want to optimize that to only recompute
+ // the metadata as needed, not force a rebind of all resources.
+ const auto& bindingInfo = mPipeline->GetBindingPointBuiltinDataInfo();
+ for (const auto& entry : bindingInfo) {
+ mDirtyBindGroupsObjectChangedOrIsDynamic.set(BindGroupIndex(entry.first.group));
+ }
+ ResetInternalUniformDataDirtyRange();
+ ResetInternalUniformDataDirtyRangeArrayLength();
+ }
+
raw_ptr<PipelineGL> mPipeline = nullptr;
// The data used for mPipeline's internal uniform buffer from current bind group.
diff --git a/src/dawn/tests/end2end/TextureShaderBuiltinTests.cpp b/src/dawn/tests/end2end/TextureShaderBuiltinTests.cpp
index a21ac29..8cf2f87 100644
--- a/src/dawn/tests/end2end/TextureShaderBuiltinTests.cpp
+++ b/src/dawn/tests/end2end/TextureShaderBuiltinTests.cpp
@@ -675,6 +675,120 @@
EXPECT_BUFFER_U32_RANGE_EQ(expected_2, buffer_2, 0, sizeof(expected_2) / sizeof(uint32_t));
}
+// Test when the pipeline changed but the bind group is unchanged, the texture builtins uniforms are
+// updated correctly.
+TEST_P(TextureShaderBuiltinTests, MultiplePipelinesOneBindGroup) {
+ const char* shader_1 = R"(
+@group(1) @binding(0) var<storage, read_write> dstBuf : array<u32>;
+// Use sparse binding to test impact of binding remapping
+@group(0) @binding(1) var tex1 : texture_2d<f32>;
+@group(0) @binding(4) var tex2 : texture_2d<f32>;
+
+@compute @workgroup_size(1, 1, 1) fn main() {
+ dstBuf[0] = textureNumLevels(tex1);
+}
+ )";
+
+ const char* shader_2 = R"(
+@group(1) @binding(0) var<storage, read_write> dstBuf : array<u32>;
+// Use sparse binding to test impact of binding remapping
+@group(0) @binding(1) var tex1 : texture_2d<f32>;
+@group(0) @binding(4) var tex2 : texture_2d<f32>;
+
+@compute @workgroup_size(1, 1, 1) fn main() {
+ dstBuf[0] = textureNumLevels(tex2);
+}
+ )";
+
+ // Explicitly create bind group layout, shared by 2 different pipelines.
+ wgpu::BindGroupLayout bindGroupLayoutShared = utils::MakeBindGroupLayout(
+ device, {
+ {1, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::UnfilterableFloat,
+ wgpu::TextureViewDimension::e2D},
+ {4, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::UnfilterableFloat,
+ wgpu::TextureViewDimension::e2D},
+ });
+
+ wgpu::BindGroupLayout bindGroupLayoutBuffer = utils::MakeBindGroupLayout(
+ device, {
+ {0, wgpu::ShaderStage::Compute, wgpu::BufferBindingType::Storage},
+ });
+
+ constexpr uint32_t kMipLevels_1 = 2;
+ constexpr uint32_t kMipLevels_2 = 3;
+
+ wgpu::TextureDescriptor textureDesc = {};
+ textureDesc.size.width = 4;
+ textureDesc.size.height = 4;
+ textureDesc.format = kDefaultFormat;
+ textureDesc.mipLevelCount = kMipLevels_1;
+ textureDesc.usage = wgpu::TextureUsage::TextureBinding;
+ wgpu::TextureView texView_1 = device.CreateTexture(&textureDesc).CreateView();
+
+ textureDesc.mipLevelCount = kMipLevels_2;
+ wgpu::TextureView texView_2 = device.CreateTexture(&textureDesc).CreateView();
+
+ constexpr uint32_t expected_1[] = {
+ // Output from first dispatch
+ kMipLevels_1,
+ };
+ constexpr uint32_t expected_2[] = {
+ // Output from second dispatch with different pipeline
+ kMipLevels_2,
+ };
+ DAWN_ASSERT(sizeof(expected_1) == sizeof(expected_2));
+
+ wgpu::BufferDescriptor bufferDesc;
+ bufferDesc.size = sizeof(expected_1);
+ bufferDesc.usage = wgpu::BufferUsage::Storage | wgpu::BufferUsage::CopySrc;
+ wgpu::Buffer buffer_1 = device.CreateBuffer(&bufferDesc);
+ wgpu::Buffer buffer_2 = device.CreateBuffer(&bufferDesc);
+
+ wgpu::PipelineLayout pipelineLayout =
+ utils::MakePipelineLayout(device, {bindGroupLayoutShared, bindGroupLayoutBuffer});
+
+ wgpu::ComputePipelineDescriptor pipelineDesc;
+ pipelineDesc.layout = pipelineLayout;
+ pipelineDesc.compute.module = utils::CreateShaderModule(device, shader_1);
+ wgpu::ComputePipeline pipeline_1 = device.CreateComputePipeline(&pipelineDesc);
+
+ pipelineDesc.compute.module = utils::CreateShaderModule(device, shader_2);
+ wgpu::ComputePipeline pipeline_2 = device.CreateComputePipeline(&pipelineDesc);
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ {
+ wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
+ pass.SetPipeline(pipeline_1);
+
+ pass.SetBindGroup(0, utils::MakeBindGroup(device, bindGroupLayoutShared,
+ {
+ {1, texView_1},
+ {4, texView_2},
+ }));
+ pass.SetBindGroup(1, utils::MakeBindGroup(device, bindGroupLayoutBuffer,
+ {
+ {0, buffer_1},
+ }));
+ pass.DispatchWorkgroups(1);
+
+ // Note: Change the pipeline, but don't change the shared bind group
+ pass.SetPipeline(pipeline_2);
+ pass.SetBindGroup(1, utils::MakeBindGroup(device, bindGroupLayoutBuffer,
+ {
+ {0, buffer_2},
+ }));
+ pass.DispatchWorkgroups(1);
+
+ pass.End();
+ }
+
+ wgpu::CommandBuffer commands = encoder.Finish();
+ queue.Submit(1, &commands);
+
+ EXPECT_BUFFER_U32_RANGE_EQ(expected_1, buffer_1, 0, sizeof(expected_1) / sizeof(uint32_t));
+ EXPECT_BUFFER_U32_RANGE_EQ(expected_2, buffer_2, 0, sizeof(expected_2) / sizeof(uint32_t));
+}
+
DAWN_INSTANTIATE_TEST(TextureShaderBuiltinTests,
D3D11Backend(),
D3D12Backend(),
