Improve runtime minBindingSize validation message
The message now reports the group/number of the binding that failed
validation, as well as the buffer.
Bug: dawn:1604
Change-Id: Ib08a3eace5ec5ebaf9eecce01eb397b5c0681bd1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/131843
Reviewed-by: Brandon Jones <bajones@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/BindGroup.cpp b/src/dawn/native/BindGroup.cpp
index c1345ca..47c6929 100644
--- a/src/dawn/native/BindGroup.cpp
+++ b/src/dawn/native/BindGroup.cpp
@@ -253,6 +253,16 @@
return {};
}
+template <typename F>
+void ForEachUnverifiedBufferBindingIndexImpl(const BindGroupLayoutBase* bgl, F&& f) {
+ uint32_t packedIndex = 0;
+ for (BindingIndex bindingIndex{0}; bindingIndex < bgl->GetBufferCount(); ++bindingIndex) {
+ if (bgl->GetBindingInfo(bindingIndex).buffer.minBindingSize == 0) {
+ f(bindingIndex, packedIndex++);
+ }
+ }
+}
+
} // anonymous namespace
MaybeError ValidateBindGroupDescriptor(DeviceBase* device,
@@ -351,7 +361,7 @@
ASSERT(bindingsSet.count() == descriptor->layout->GetUnexpandedBindingCount());
return {};
-} // anonymous namespace
+}
// BindGroup
@@ -443,15 +453,11 @@
}
}
- uint32_t packedIdx = 0;
- for (BindingIndex bindingIndex{0}; bindingIndex < descriptor->layout->GetBufferCount();
- ++bindingIndex) {
- if (descriptor->layout->GetBindingInfo(bindingIndex).buffer.minBindingSize == 0) {
- mBindingData.unverifiedBufferSizes[packedIdx] =
- mBindingData.bufferData[bindingIndex].size;
- ++packedIdx;
- }
- }
+ ForEachUnverifiedBufferBindingIndexImpl(mLayout.Get(),
+ [&](BindingIndex bindingIndex, uint32_t packedIndex) {
+ mBindingData.unverifiedBufferSizes[packedIndex] =
+ mBindingData.bufferData[bindingIndex].size;
+ });
GetObjectTrackingList()->Track(this);
}
@@ -530,4 +536,9 @@
return mBoundExternalTextures;
}
+void BindGroupBase::ForEachUnverifiedBufferBindingIndex(
+ std::function<void(BindingIndex, uint32_t)> fn) const {
+ ForEachUnverifiedBufferBindingIndexImpl(mLayout.Get(), fn);
+}
+
} // namespace dawn::native
diff --git a/src/dawn/native/BindGroup.h b/src/dawn/native/BindGroup.h
index c8a2c4f..922d813 100644
--- a/src/dawn/native/BindGroup.h
+++ b/src/dawn/native/BindGroup.h
@@ -56,6 +56,8 @@
const ityp::span<uint32_t, uint64_t>& GetUnverifiedBufferSizes() const;
const std::vector<Ref<ExternalTextureBase>>& GetBoundExternalTextures() const;
+ void ForEachUnverifiedBufferBindingIndex(std::function<void(BindingIndex, uint32_t)> fn) const;
+
protected:
// To save memory, the size of a bind group is dynamically determined and the bind group is
// placement-allocated into memory big enough to hold the bind group with its
diff --git a/src/dawn/native/CommandBufferStateTracker.cpp b/src/dawn/native/CommandBufferStateTracker.cpp
index 8f018da..fae24bd 100644
--- a/src/dawn/native/CommandBufferStateTracker.cpp
+++ b/src/dawn/native/CommandBufferStateTracker.cpp
@@ -14,6 +14,7 @@
#include "dawn/native/CommandBufferStateTracker.h"
+#include <limits>
#include <optional>
#include <type_traits>
#include <utility>
@@ -573,22 +574,40 @@
requiredBGL, mLastPipelineLayout, currentBGL, mBindgroups[i],
static_cast<uint32_t>(i));
- // TODO(dawn:563): Report which buffer bindings are failing. This requires the ability
- // to look up the binding index from the packed index.
std::optional<uint32_t> packedIndex = FindFirstUndersizedBuffer(
mBindgroups[i]->GetUnverifiedBufferSizes(), (*mMinBufferSizes)[i]);
if (packedIndex.has_value()) {
+ // Find the binding index for this packed index.
+ BindingIndex bindingIndex{std::numeric_limits<uint32_t>::max()};
+ mBindgroups[i]->ForEachUnverifiedBufferBindingIndex(
+ [&](BindingIndex candidateBindingIndex, uint32_t candidatePackedIndex) {
+ if (candidatePackedIndex == *packedIndex) {
+ bindingIndex = candidateBindingIndex;
+ }
+ });
+ ASSERT(static_cast<uint32_t>(bindingIndex) != std::numeric_limits<uint32_t>::max());
+
+ const auto& bindingInfo = mBindgroups[i]->GetLayout()->GetBindingInfo(bindingIndex);
+ const BufferBinding& bufferBinding =
+ mBindgroups[i]->GetBindingAsBufferBinding(bindingIndex);
+
+ BindingNumber bindingNumber = bindingInfo.binding;
+ const BufferBase* buffer = bufferBinding.buffer;
+
uint64_t bufferSize =
mBindgroups[i]->GetUnverifiedBufferSizes()[packedIndex.value()];
uint64_t minBufferSize = (*mMinBufferSizes)[i][packedIndex.value()];
+
return DAWN_VALIDATION_ERROR(
- "Binding sizes are too small for %s set at group index %u. A bound buffer "
- "contained %u bytes, but the current pipeline (%s) requires a buffer which is "
- "at least %u bytes. (Note that uniform buffer bindings must be a multiple of "
- "16 bytes, and as a result may be larger than the associated data in the "
- "shader source.)",
- mBindgroups[i], static_cast<uint32_t>(i), bufferSize, mLastPipeline,
- minBufferSize);
+ "%s bound with size %u at group %u, binding %u is too small. The pipeline (%s) "
+ "requires a buffer binding which is at least %u bytes.%s",
+ buffer, bufferSize, static_cast<uint32_t>(i),
+ static_cast<uint32_t>(bindingNumber), mLastPipeline, minBufferSize,
+ (bindingInfo.buffer.type == wgpu::BufferBindingType::Uniform
+ ? " This binding is a uniform buffer binding. It is padded to a multiple "
+ "of 16 bytes, and as a result may be larger than the associated data in "
+ "the shader source."
+ : ""));
}
}
