Add validation for GPURender/ComputePassTimestampLocation
Disallow duplicate location in timestampWrites in a render/compute pass
to match WebGPU SPEC.
Bug: dawn:1250
Change-Id: Id9e3b54530d37ffc0c00aa15c23312bb1ea30d00
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/90460
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Hao Li <hao.x.li@intel.com>
diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index 9f2a5d1..51fee4b 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -14,6 +14,7 @@
#include "dawn/native/CommandEncoder.h"
+#include <unordered_set>
#include <utility>
#include <vector>
@@ -414,6 +415,30 @@
return {};
}
+MaybeError ValidateTimestampLocationOnRenderPass(
+ wgpu::RenderPassTimestampLocation location,
+ const std::unordered_set<wgpu::RenderPassTimestampLocation>& writtenLocations) {
+ DAWN_TRY(ValidateRenderPassTimestampLocation(location));
+
+ DAWN_INVALID_IF(writtenLocations.find(location) != writtenLocations.end(),
+ "There are two same RenderPassTimestampLocation %u in a render pass.",
+ location);
+
+ return {};
+}
+
+MaybeError ValidateTimestampLocationOnComputePass(
+ wgpu::ComputePassTimestampLocation location,
+ const std::unordered_set<wgpu::ComputePassTimestampLocation>& writtenLocations) {
+ DAWN_TRY(ValidateComputePassTimestampLocation(location));
+
+ DAWN_INVALID_IF(writtenLocations.find(location) != writtenLocations.end(),
+ "There are two same ComputePassTimestampLocation %u in a compute pass.",
+ location);
+
+ return {};
+}
+
MaybeError ValidateRenderPassDescriptor(DeviceBase* device,
const RenderPassDescriptor* descriptor,
uint32_t* width,
@@ -465,15 +490,22 @@
// not validated and encoded one by one, but encoded together after passing the
// validation.
QueryAvailabilityMap usedQueries;
+ // TODO(https://crbug.com/dawn/1452):
+ // 1. Add an enum that's TimestampLocationMask and has bit values.
+ // 2. Add a function with a switch that converts from one to the other.
+ // 3. type alias the ityp::bitset for that to call it TimestampLocationSet.
+ // 4. Use it here.
+ std::unordered_set<wgpu::RenderPassTimestampLocation> writtenLocations;
for (uint32_t i = 0; i < descriptor->timestampWriteCount; ++i) {
QuerySetBase* querySet = descriptor->timestampWrites[i].querySet;
DAWN_ASSERT(querySet != nullptr);
uint32_t queryIndex = descriptor->timestampWrites[i].queryIndex;
DAWN_TRY_CONTEXT(ValidateTimestampQuery(device, querySet, queryIndex),
"validating querySet and queryIndex of timestampWrites[%u].", i);
- DAWN_TRY_CONTEXT(
- ValidateRenderPassTimestampLocation(descriptor->timestampWrites[i].location),
- "validating location of timestampWrites[%u].", i);
+ DAWN_TRY_CONTEXT(ValidateTimestampLocationOnRenderPass(
+ descriptor->timestampWrites[i].location, writtenLocations),
+ "validating location of timestampWrites[%u].", i);
+ writtenLocations.insert(descriptor->timestampWrites[i].location);
auto checkIt = usedQueries.find(querySet);
DAWN_INVALID_IF(checkIt != usedQueries.end() && checkIt->second[queryIndex],
@@ -503,14 +535,21 @@
if (descriptor->timestampWriteCount > 0) {
DAWN_ASSERT(descriptor->timestampWrites != nullptr);
+ // TODO(https://crbug.com/dawn/1452):
+ // 1. Add an enum that's TimestampLocationMask and has bit values.
+ // 2. Add a function with a switch that converts from one to the other.
+ // 3. type alias the ityp::bitset for that to call it TimestampLocationSet.
+ // 4. Use it here.
+ std::unordered_set<wgpu::ComputePassTimestampLocation> writtenLocations;
for (uint32_t i = 0; i < descriptor->timestampWriteCount; ++i) {
DAWN_ASSERT(descriptor->timestampWrites[i].querySet != nullptr);
DAWN_TRY_CONTEXT(ValidateTimestampQuery(device, descriptor->timestampWrites[i].querySet,
descriptor->timestampWrites[i].queryIndex),
"validating querySet and queryIndex of timestampWrites[%u].", i);
- DAWN_TRY_CONTEXT(
- ValidateComputePassTimestampLocation(descriptor->timestampWrites[i].location),
- "validating location of timestampWrites[%u].", i);
+ DAWN_TRY_CONTEXT(ValidateTimestampLocationOnComputePass(
+ descriptor->timestampWrites[i].location, writtenLocations),
+ "validating location of timestampWrites[%u].", i);
+ writtenLocations.insert(descriptor->timestampWrites[i].location);
}
}
diff --git a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp
index 04899ad..fdc863e 100644
--- a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp
@@ -352,13 +352,23 @@
encoder.Finish();
}
- // Success to write timestamps at same location of compute pass
+ // Success to write timestamps at same location of different compute pass
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ EncodeComputePassWithTimestampWrites(
+ encoder, {{querySet, 0, wgpu::ComputePassTimestampLocation::Beginning}});
+ EncodeComputePassWithTimestampWrites(
+ encoder, {{querySet, 0, wgpu::ComputePassTimestampLocation::Beginning}});
+ encoder.Finish();
+ }
+
+ // Fail to write timestamps at same location of a compute pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
EncodeComputePassWithTimestampWrites(
encoder, {{querySet, 0, wgpu::ComputePassTimestampLocation::Beginning},
{querySet, 1, wgpu::ComputePassTimestampLocation::Beginning}});
- encoder.Finish();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps at invalid location of compute pass
@@ -427,7 +437,8 @@
ASSERT_DEVICE_ERROR(encoder.Finish());
}
- // Success to write timestamps to the same query index twice on different render pass
+ // Success to write timestamps to the same query index and location twice on different render
+ // pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
EncodeRenderPassWithTimestampWrites(
@@ -449,13 +460,13 @@
ASSERT_DEVICE_ERROR(encoder.Finish());
}
- // Success to write timestamps at same location of render pass
+ // Fail to write timestamps at same location of a render pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
EncodeRenderPassWithTimestampWrites(
encoder, {{querySet, 0, wgpu::RenderPassTimestampLocation::Beginning},
{querySet, 1, wgpu::RenderPassTimestampLocation::Beginning}});
- encoder.Finish();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps at invalid location of render pass
