Added constraints for too large uniform buffer bindings

Added a constraint in ValidateBufferBinding function checking for too
large uniform buffer bindings. Added MaxUniformBufferBindingSize
test in BindGroupValidationTest.

Bug: dawn:436
Change-Id: I31c6e2236ce928d5e81c43455eb18cf4eacdc0f1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24081
Commit-Queue: Tomek Ponitka <tommek@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
This commit is contained in:
Tomek Ponitka 2020-06-30 18:49:20 +00:00 committed by Commit Bot service account
parent bd2ab92853
commit 268fe254ec
3 changed files with 53 additions and 3 deletions

View File

@ -42,6 +42,8 @@ static constexpr uint32_t kMaxDynamicStorageBufferCount = 4u;
// Max numbers of dynamic buffers // Max numbers of dynamic buffers
static constexpr uint32_t kMaxDynamicBufferCount = static constexpr uint32_t kMaxDynamicBufferCount =
kMaxDynamicUniformBufferCount + kMaxDynamicStorageBufferCount; kMaxDynamicUniformBufferCount + kMaxDynamicStorageBufferCount;
// Max size of uniform buffer binding
static constexpr uint64_t kMaxUniformBufferBindingSize = 16384u;
// Indirect command sizes // Indirect command sizes
static constexpr uint64_t kDispatchIndirectSize = 3 * sizeof(uint32_t); static constexpr uint64_t kDispatchIndirectSize = 3 * sizeof(uint32_t);
static constexpr uint64_t kDrawIndirectSize = 4 * sizeof(uint32_t); static constexpr uint64_t kDrawIndirectSize = 4 * sizeof(uint32_t);

View File

@ -32,7 +32,8 @@ namespace dawn_native {
MaybeError ValidateBufferBinding(const DeviceBase* device, MaybeError ValidateBufferBinding(const DeviceBase* device,
const BindGroupEntry& entry, const BindGroupEntry& entry,
wgpu::BufferUsage requiredUsage, wgpu::BufferUsage requiredUsage,
const BindingInfo& bindingInfo) { const BindingInfo& bindingInfo,
const uint64_t maxBindingSize) {
if (entry.buffer == nullptr || entry.sampler != nullptr || if (entry.buffer == nullptr || entry.sampler != nullptr ||
entry.textureView != nullptr) { entry.textureView != nullptr) {
return DAWN_VALIDATION_ERROR("expected buffer binding"); return DAWN_VALIDATION_ERROR("expected buffer binding");
@ -79,6 +80,14 @@ namespace dawn_native {
" bytes"); " bytes");
} }
if (bindingSize > maxBindingSize) {
return DAWN_VALIDATION_ERROR(
"Binding size bigger than maximum uniform buffer binding size: binding " +
std::to_string(entry.binding) + " given " + std::to_string(bindingSize) +
" bytes, maximum is " + std::to_string(kMaxUniformBufferBindingSize) +
" bytes");
}
return {}; return {};
} }
@ -192,12 +201,13 @@ namespace dawn_native {
switch (bindingInfo.type) { switch (bindingInfo.type) {
case wgpu::BindingType::UniformBuffer: case wgpu::BindingType::UniformBuffer:
DAWN_TRY(ValidateBufferBinding(device, entry, wgpu::BufferUsage::Uniform, DAWN_TRY(ValidateBufferBinding(device, entry, wgpu::BufferUsage::Uniform,
bindingInfo)); bindingInfo, kMaxUniformBufferBindingSize));
break; break;
case wgpu::BindingType::StorageBuffer: case wgpu::BindingType::StorageBuffer:
case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::ReadonlyStorageBuffer:
DAWN_TRY(ValidateBufferBinding(device, entry, wgpu::BufferUsage::Storage, DAWN_TRY(ValidateBufferBinding(device, entry, wgpu::BufferUsage::Storage,
bindingInfo)); bindingInfo,
std::numeric_limits<uint64_t>::max()));
break; break;
case wgpu::BindingType::SampledTexture: case wgpu::BindingType::SampledTexture:
DAWN_TRY(ValidateTextureBinding(device, entry, wgpu::TextureUsage::Sampled, DAWN_TRY(ValidateTextureBinding(device, entry, wgpu::TextureUsage::Sampled,

View File

@ -441,6 +441,44 @@ TEST_F(BindGroupValidationTest, BufferBindingOOB) {
ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, buffer, 256, uint32_t(0) - uint32_t(256)}})); ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, buffer, 256, uint32_t(0) - uint32_t(256)}}));
} }
// Tests constraints to be sure the uniform buffer binding isn't too large
TEST_F(BindGroupValidationTest, MaxUniformBufferBindingSize) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 2 * kMaxUniformBufferBindingSize;
descriptor.usage = wgpu::BufferUsage::Uniform | wgpu::BufferUsage::Storage;
wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
wgpu::BindGroupLayout uniformLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::UniformBuffer}});
// Success case, this is exactly the limit
utils::MakeBindGroup(device, uniformLayout, {{0, buffer, 0, kMaxUniformBufferBindingSize}});
wgpu::BindGroupLayout doubleUniformLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::UniformBuffer},
{1, wgpu::ShaderStage::Vertex, wgpu::BindingType::UniformBuffer}});
// Success case, individual bindings don't exceed the limit
utils::MakeBindGroup(device, doubleUniformLayout,
{{0, buffer, 0, kMaxUniformBufferBindingSize},
{1, buffer, kMaxUniformBufferBindingSize, kMaxUniformBufferBindingSize}});
// Error case, this is above the limit
ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, uniformLayout,
{{0, buffer, 0, kMaxUniformBufferBindingSize + 1}}));
// Making sure the constraint doesn't apply to storage buffers
wgpu::BindGroupLayout readonlyStorageLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ReadonlyStorageBuffer}});
wgpu::BindGroupLayout storageLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::StorageBuffer}});
// Success case, storage buffer can still be created.
utils::MakeBindGroup(device, readonlyStorageLayout,
{{0, buffer, 0, 2 * kMaxUniformBufferBindingSize}});
utils::MakeBindGroup(device, storageLayout, {{0, buffer, 0, 2 * kMaxUniformBufferBindingSize}});
}
// Test what happens when the layout is an error. // Test what happens when the layout is an error.
TEST_F(BindGroupValidationTest, ErrorLayout) { TEST_F(BindGroupValidationTest, ErrorLayout) {
wgpu::BindGroupLayout goodLayout = utils::MakeBindGroupLayout( wgpu::BindGroupLayout goodLayout = utils::MakeBindGroupLayout(