[M140] [dawn][native][d3d12] Force higher minimums for maxDynamic* limits Adding maxImmediateSize to D3D12 took space away from maxDynamicUniformBuffersPerPipelineLayout and maxDynamicStorageBuffersPerPipelineLayout, which is a regression. Set those first, so that allocating maxImmediateSize can't take away from them. This should be safe for all machines because kMaxRootSignatureSize and the default values for the limits are all constants, so all this allocation logic has the same logic on all machines. Fixed: 441149385 Bug: 440381283 Change-Id: I679481bf6a31cda73c117d5b7c9518b59f9008b6 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/259055 Commit-Queue: Kai Ninomiya <kainino@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Loko Kung <lokokung@google.com> (cherry picked from commit c737bb0d8524777131ea0475670fc26ae41f36df) Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/259095 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
diff --git a/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp b/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp index cfab633..2fa853b 100644 --- a/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp +++ b/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp
@@ -317,6 +317,10 @@ // This is maxColorAttachments times 16, the color format with the largest cost. limits->v1.maxColorAttachmentBytesPerSample = D3D12_SIMULTANEOUS_RENDER_TARGET_COUNT * 16; + // We want to support a minimum of 10 and 8 for these limits (see https://crbug.com/440381283). + limits->v1.maxDynamicUniformBuffersPerPipelineLayout = 10; + limits->v1.maxDynamicStorageBuffersPerPipelineLayout = 8; + // https://docs.microsoft.com/en-us/windows/win32/direct3d12/root-signature-limits // In DWORDS. Descriptor tables cost 1, Root constants cost 1, Root descriptors cost 2. static constexpr uint32_t kMaxRootSignatureSize = 64u; @@ -353,6 +357,10 @@ availableRootSignatureSlots -= maxImmediateDataSlots; limits->v1.maxImmediateSize = maxImmediateDataSlots * kImmediateConstantElementByteSize; + // TODO(crbug.com/440381283): We used to allocate more root signature space toward other limits + // here, but the logic above effectively hardcodes limits very close to the max so this doesn't + // actually do much. If we free up space by moving things (e.g. immediates) out of the root + // signature, then revisit this. while (availableRootSignatureSlots >= 2) { // Start by incrementing maxDynamicStorageBuffersPerPipelineLayout since the // default is just 4 and developers likely want more. This scheme currently @@ -371,10 +379,11 @@ } } + DAWN_ASSERT(limits->v1.maxImmediateSize >= 32); DAWN_ASSERT( 2 * limits->v1.maxBindGroups + 2 * limits->v1.maxDynamicUniformBuffersPerPipelineLayout + 3 * limits->v1.maxDynamicStorageBuffersPerPipelineLayout + - limits->v1.maxImmediateSize / kImmediateConstantElementByteSize + kReservedSlots == + limits->v1.maxImmediateSize / kImmediateConstantElementByteSize + kReservedSlots <= kMaxRootSignatureSize); // https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sm5-attributes-numthreads