encounter/dawn-cmake
1
0
Fork 0
mirror of https://github.com/encounter/dawn-cmake.git synced 2025-07-12 16:15:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve draw-time bind group validation messages

Browse Source 
Makes it clear that the index being reported by these messages is the
group index and not the binding number. This was a point of confusion
in on the bug.

Additionally, adds more information to the error message regarding
buffer sizes being too small for the current pipeline. Now includes the
pipeline name and buffer size as well as the minimum required size. Also
includes a note explaining that uniform buffer bindings must be a
multiple of 16. (This recently changed and cause several existing
samples to break for non-obvious reasons.)

The error message still does not contain the buffer or binding number,
which would be helpful. This is because we currently lack a way to look
up the binding index from the packed index that this error is generated
with.

Bug: dawn:1604
Change-Id: Ibb2b44bc9e1583ddef34d703e83bcf64ed7a3aa2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/113602
Commit-Queue: Brandon Jones <bajones@chromium.org>
Auto-Submit: Brandon Jones <bajones@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Brandon Jones 2022-12-14 23:44:11 +00:00 committed by Dawn LUCI CQ
parent ea1cc79cb8
commit 87a1724368
1 changed files with 37 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
src/dawn/native/CommandBufferStateTracker.cpp
View File

@ -14,6 +14,8 @@
#include "dawn/native/CommandBufferStateTracker.h" #include "dawn/native/CommandBufferStateTracker.h"
#include <optional>
#include "dawn/common/Assert.h" #include "dawn/common/Assert.h"
#include "dawn/common/BitSetIterator.h" #include "dawn/common/BitSetIterator.h"
#include "dawn/native/BindGroup.h" #include "dawn/native/BindGroup.h"
@ -31,17 +33,21 @@
namespace dawn::native { namespace dawn::native {
namespace { namespace {
bool BufferSizesAtLeastAsBig(const ityp::span<uint32_t, uint64_t> unverifiedBufferSizes, // Returns nullopt if all buffers in unverifiedBufferSizes are at least large enough to satisfy the
const std::vector<uint64_t>& pipelineMinBufferSizes) { // minimum listed in pipelineMinBufferSizes, or the index of the first detected failing buffer
// otherwise.
std::optional<uint32_t> FindFirstUndersizedBuffer(
const ityp::span<uint32_t, uint64_t> unverifiedBufferSizes,
const std::vector<uint64_t>& pipelineMinBufferSizes) {
ASSERT(unverifiedBufferSizes.size() == pipelineMinBufferSizes.size()); ASSERT(unverifiedBufferSizes.size() == pipelineMinBufferSizes.size());
for (uint32_t i = 0; i < unverifiedBufferSizes.size(); ++i) { for (uint32_t i = 0; i < unverifiedBufferSizes.size(); ++i) {
if (unverifiedBufferSizes[i] < pipelineMinBufferSizes[i]) { if (unverifiedBufferSizes[i] < pipelineMinBufferSizes[i]) {
return false; return i;
} }
} }
return true; return std::nullopt;
} }
} // namespace } // namespace
@ -234,8 +240,9 @@ void CommandBufferStateTracker::RecomputeLazyAspects(ValidationAspects aspects)
for (BindGroupIndex i : IterateBitSet(mLastPipelineLayout->GetBindGroupLayoutsMask())) { for (BindGroupIndex i : IterateBitSet(mLastPipelineLayout->GetBindGroupLayoutsMask())) {
if (mBindgroups[i] == nullptr || if (mBindgroups[i] == nullptr ||
mLastPipelineLayout->GetBindGroupLayout(i) != mBindgroups[i]->GetLayout() || mLastPipelineLayout->GetBindGroupLayout(i) != mBindgroups[i]->GetLayout() ||
!BufferSizesAtLeastAsBig(mBindgroups[i]->GetUnverifiedBufferSizes(), FindFirstUndersizedBuffer(mBindgroups[i]->GetUnverifiedBufferSizes(),
(*mMinBufferSizes)[i])) { (*mMinBufferSizes)[i])
.has_value()) {
matches = false; matches = false;
break; break;
} }
@ -315,7 +322,7 @@ MaybeError CommandBufferStateTracker::CheckMissingAspects(ValidationAspects aspe
for (BindGroupIndex i : IterateBitSet(mLastPipelineLayout->GetBindGroupLayoutsMask())) { for (BindGroupIndex i : IterateBitSet(mLastPipelineLayout->GetBindGroupLayoutsMask())) {
ASSERT(HasPipeline()); ASSERT(HasPipeline());
DAWN_INVALID_IF(mBindgroups[i] == nullptr, "No bind group set at index %u.", DAWN_INVALID_IF(mBindgroups[i] == nullptr, "No bind group set at group index %u.",
static_cast<uint32_t>(i)); static_cast<uint32_t>(i));
BindGroupLayoutBase* requiredBGL = mLastPipelineLayout->GetBindGroupLayout(i); BindGroupLayoutBase* requiredBGL = mLastPipelineLayout->GetBindGroupLayout(i);
@ -326,8 +333,8 @@ MaybeError CommandBufferStateTracker::CheckMissingAspects(ValidationAspects aspe
currentBGL->GetPipelineCompatibilityToken() != currentBGL->GetPipelineCompatibilityToken() !=
requiredBGL->GetPipelineCompatibilityToken(), requiredBGL->GetPipelineCompatibilityToken(),
"The current pipeline (%s) was created with a default layout, and is not " "The current pipeline (%s) was created with a default layout, and is not "
"compatible with the %s at index %u which uses a %s that was not created by " "compatible with the %s set at group index %u which uses a %s that was not "
"the pipeline. Either use the bind group layout returned by calling " "created by the pipeline. Either use the bind group layout returned by calling "
"getBindGroupLayout(%u) on the pipeline when creating the bind group, or " "getBindGroupLayout(%u) on the pipeline when creating the bind group, or "
"provide an explicit pipeline layout when creating the pipeline.", "provide an explicit pipeline layout when creating the pipeline.",
mLastPipeline, mBindgroups[i], static_cast<uint32_t>(i), currentBGL, mLastPipeline, mBindgroups[i], static_cast<uint32_t>(i), currentBGL,
@ -336,8 +343,9 @@ MaybeError CommandBufferStateTracker::CheckMissingAspects(ValidationAspects aspe
DAWN_INVALID_IF( DAWN_INVALID_IF(
requiredBGL->GetPipelineCompatibilityToken() == PipelineCompatibilityToken(0) && requiredBGL->GetPipelineCompatibilityToken() == PipelineCompatibilityToken(0) &&
currentBGL->GetPipelineCompatibilityToken() != PipelineCompatibilityToken(0), currentBGL->GetPipelineCompatibilityToken() != PipelineCompatibilityToken(0),
"%s at index %u uses a %s which was created as part of the default layout for " "%s set at group index %u uses a %s which was created as part of the default "
"a different pipeline than the current one (%s), and as a result is not " "layout "
"for a different pipeline than the current one (%s), and as a result is not "
"compatible. Use an explicit bind group layout when creating bind groups and " "compatible. Use an explicit bind group layout when creating bind groups and "
"an explicit pipeline layout when creating pipelines to share bind groups " "an explicit pipeline layout when creating pipelines to share bind groups "
"between pipelines.", "between pipelines.",
@ -346,15 +354,27 @@ MaybeError CommandBufferStateTracker::CheckMissingAspects(ValidationAspects aspe
DAWN_INVALID_IF( DAWN_INVALID_IF(
mLastPipelineLayout->GetBindGroupLayout(i) != mBindgroups[i]->GetLayout(), mLastPipelineLayout->GetBindGroupLayout(i) != mBindgroups[i]->GetLayout(),
"Bind group layout %s of pipeline layout %s does not match layout %s of bind " "Bind group layout %s of pipeline layout %s does not match layout %s of bind "
"group %s at index %u.", "group %s set at group index %u.",
requiredBGL, mLastPipelineLayout, currentBGL, mBindgroups[i], requiredBGL, mLastPipelineLayout, currentBGL, mBindgroups[i],
static_cast<uint32_t>(i)); static_cast<uint32_t>(i));
// TODO(dawn:563): Report the binding sizes and which ones are failing. // TODO(dawn:563): Report which buffer bindings are failing. This requires the ability
DAWN_INVALID_IF(!BufferSizesAtLeastAsBig(mBindgroups[i]->GetUnverifiedBufferSizes(), // to look up the binding index from the packed index.
(*mMinBufferSizes)[i]), std::optional<uint32_t> packedIndex = FindFirstUndersizedBuffer(
"Binding sizes are too small for bind group %s at index %u", mBindgroups[i]->GetUnverifiedBufferSizes(), (*mMinBufferSizes)[i]);
mBindgroups[i], static_cast<uint32_t>(i)); if (packedIndex.has_value()) {
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);
}
} }
// The chunk of code above should be similar to the one in |RecomputeLazyAspects|. // The chunk of code above should be similar to the one in |RecomputeLazyAspects|.