Skip to content

Commit a6ca291

Browse files
KangzDawn LUCI CQ
authored andcommitted
[bindless] Reassign TODOs to more precise issues.
Bug: 473444513, 473354062, 473444514, 473354063, 473442434, 473444515, 473354064, 473442435, 473354065, 473459218, 435317394, 465122000 Change-Id: If258bcc5ad9e7ad841242910ae3f9316d100faf7 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/283075 Reviewed-by: dan sinclair <dsinclair@chromium.org> Commit-Queue: Corentin Wallez <cwallez@chromium.org>
1 parent ce2d442 commit a6ca291

File tree

15 files changed

+67
-64
lines changed

15 files changed

+67
-64
lines changed

src/dawn/common/Constants.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,9 @@ static constexpr uint32_t kMaxDynamicUniformBuffersPerPipelineLayout = 16u;
112112
static constexpr uint32_t kMaxDynamicStorageBuffersPerPipelineLayout = 16u;
113113

114114
// Default limit for the ResourceTable size.
115-
// TODO(https://issues.chromium.org/issues/435317394): Update once the spec decides on a value.
115+
// TODO(https://issues.chromium.org/465122000): Update once the spec decides on a value.
116116
static constexpr uint32_t kMaxResourceTableSize = 50'000;
117-
// TODO(https://issues.chromium.org/issues/435317394): Find if this is a reasonable amount to
117+
// TODO(https://issues.chromium.org/465122000): Find if this is a reasonable amount to
118118
// reserve for placeholders.
119119
static constexpr uint32_t kReservedResourceTableSlots = 1000;
120120

src/dawn/native/ResourceTable.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ tint::ResourceType ComputeTypeId(const TextureViewBase* view) {
4747
}
4848
const TextureBase* texture = view->GetTexture();
4949

50-
// TODO(https://crbug.com/435317394): In the future we should allow the same compatibility rules
51-
// that exist between TextureView and BGLEntry. This means that a depth texture can be either a
52-
// texture_depth_2d, or a texture_2d<f32> (unfilterable). We should also find a way to
53-
// differentiate unfilterable and filterable texture_2d<f32>.
50+
// TODO(https://issues.chromium.org/473354065): In the future we should allow the same
51+
// compatibility rules that exist between TextureView and BGLEntry. This means that a depth
52+
// texture can be either a texture_depth_2d, or a texture_2d<f32> (unfilterable). We should
53+
// also find a way to differentiate unfilterable and filterable texture_2d<f32>.
5454

5555
if (texture->IsMultisampledTexture()) {
5656
DAWN_ASSERT(view->GetDimension() == wgpu::TextureViewDimension::e2D);
@@ -158,17 +158,17 @@ MaybeError ValidateBindingResource(const DeviceBase* device, const BindingResour
158158
DAWN_INVALID_IF(resourceCount != 1,
159159
"%i resources are specified (when there must be exactly 1).", resourceCount);
160160

161-
// TODO(https://issues.chromium.org/435317394): Support buffers in FullResourceTable.
161+
// TODO(https://issues.chromium.org/473444515): Support buffers in FullResourceTable.
162162
if (resource->buffer != nullptr) {
163163
return DAWN_VALIDATION_ERROR("Buffers are not supported.");
164164
}
165165

166-
// TODO(https://issues.chromium.org/435317394): Support samplers in SamplingResourceTable.
166+
// TODO(https://issues.chromium.org/473354063): Support samplers in SamplingResourceTable.
167167
if (resource->sampler != nullptr) {
168168
return DAWN_VALIDATION_ERROR("Samplers are not supported.");
169169
}
170170

171-
// TODO(https://issues.chromium.org/435317394): Support texel buffers in FullResourceTable.
171+
// TODO(https://issues.chromium.org/473444515): Support texel buffers in FullResourceTable.
172172

173173
if (resource->textureView != nullptr) {
174174
TextureViewBase* view = resource->textureView;
@@ -178,7 +178,7 @@ MaybeError ValidateBindingResource(const DeviceBase* device, const BindingResour
178178
DAWN_INVALID_IF(!HasOneBit(aspect),
179179
"Multiple aspects (%s) selected in %s. Expected only 1.", aspect, view);
180180

181-
// TODO(https://issues.chromium.org/435317394): Support storage textures in
181+
// TODO(https://issues.chromium.org/473444515): Support storage textures in
182182
// FullResourceTable
183183
DAWN_INVALID_IF(
184184
(view->GetUsage() & kTextureViewOnlyUsages) != wgpu::TextureUsage::TextureBinding,
@@ -493,7 +493,7 @@ void ResourceTableBase::Remove(ResourceTableSlot slot) {
493493
}
494494

495495
void ResourceTableBase::SetEntry(ResourceTableSlot slot, const BindingResource* contents) {
496-
// TODO(435317394): Support resources that aren't TextureViews
496+
// TODO(https://issues.chromium.org/473354063): Support resources that aren't TextureViews
497497
DAWN_ASSERT(contents->buffer == nullptr && contents->sampler == nullptr);
498498
TextureViewBase* view = contents->textureView;
499499

src/dawn/native/ShaderModule.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1210,7 +1210,8 @@ ResultOrError<std::unique_ptr<EntryPointMetadata>> ReflectEntryPointUsingTint(
12101210
}
12111211

12121212
// Resource table reflection.
1213-
// TODO(crbug.com/435317394): Check that the list of uses of the resource table are
1213+
// TODO(https://issues.chromium.org/473354064): Check that the list of uses of the resource
1214+
// table are:
12141215
// 1) compatible with the bindless feature enabled (sampling vs. full)
12151216
// 2) potentially that each type used is enabled by whatever feature enables it (for example
12161217
// texel buffers might require an extension)

src/dawn/native/Texture.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,8 +1673,9 @@ MaybeError TextureBase::ValidatePin(wgpu::TextureUsage usage) const {
16731673
DAWN_INVALID_IF(!IsSubset(usage, kShaderTextureUsages),
16741674
"Pinned usages %s contain non-shader usages.", usage);
16751675

1676-
// TODO(https://crbug.com/435317394): Support pinning for readonly storage and storage as well.
1677-
// This might require adding readonly storage in the API so it can be specified.
1676+
// TODO(https://issues.chromium.org/473459218): Support pinning for readonly storage and
1677+
// storage as well. This might require adding readonly storage in the API so it can be
1678+
// specified.
16781679
DAWN_INVALID_IF(
16791680
usage != wgpu::TextureUsage::TextureBinding,
16801681
"Pinned usages %s is not %s (which is required in the current bindless prototype).", usage,

src/dawn/native/d3d12/CommandBufferD3D12.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1480,7 +1480,7 @@ MaybeError CommandBuffer::RecordComputePass(CommandRecordingContext* commandCont
14801480
}
14811481

14821482
case Command::SetResourceTable: {
1483-
// TODO(https://issues.chromium.org/435317394): Add support for resource tables.
1483+
// TODO(https://issues.chromium.org/473354062): Add support for resource tables.
14841484
return DAWN_UNIMPLEMENTED_ERROR("SetResourceTable unimplemented.");
14851485
}
14861486

src/dawn/native/d3d12/DeviceD3D12.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ Ref<RenderPipelineBase> Device::CreateUninitializedRenderPipelineImpl(
425425
}
426426
ResultOrError<Ref<ResourceTableBase>> Device::CreateResourceTableImpl(
427427
const ResourceTableDescriptor* descriptor) {
428-
// TODO(https://issues.chromium.org/435317394): Implement resource tables in D3D12.
428+
// TODO(https://issues.chromium.org/473354062): Implement resource tables in D3D12.
429429
return DAWN_UNIMPLEMENTED_ERROR("ResourceTable is not implemented on D3D12");
430430
}
431431
ResultOrError<Ref<SamplerBase>> Device::CreateSamplerImpl(const SamplerDescriptor* descriptor) {

src/dawn/native/metal/CommandBufferMTL.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1553,7 +1553,7 @@ void RecordCopyBufferToTexture(CommandRecordingContext* commandContext,
15531553
}
15541554

15551555
case Command::SetResourceTable: {
1556-
// TODO(https://issues.chromium.org/435317394): Add support for resource tables.
1556+
// TODO(https://issues.chromium.org/473444514): Add support for resource tables.
15571557
return DAWN_UNIMPLEMENTED_ERROR("SetResourceTable unimplemented.");
15581558
}
15591559

src/dawn/native/metal/DeviceMTL.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ void UpdateTimestampPeriod(id<MTLDevice> device,
222222
}
223223
ResultOrError<Ref<ResourceTableBase>> Device::CreateResourceTableImpl(
224224
const ResourceTableDescriptor* descriptor) {
225-
// TODO(https://issues.chromium.org/435317394): Implement resource tables in Metal.
225+
// TODO(https://issues.chromium.org/473444514): Implement resource tables in Metal.
226226
return DAWN_UNIMPLEMENTED_ERROR("ResourceTable is not implemented on Metal");
227227
}
228228
ResultOrError<Ref<SamplerBase>> Device::CreateSamplerImpl(const SamplerDescriptor* descriptor) {

src/dawn/native/vulkan/PhysicalDeviceVk.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ void PhysicalDevice::InitializeSupportedFeaturesImpl() {
650650
EnableFeature(Feature::ChromiumExperimentalSamplingResourceTable);
651651
}
652652

653-
// TODO(https://issues.chromium.org/435317394): add support for heterogeneous bindless by
653+
// TODO(https://issues.chromium.org/473444515): add support for heterogeneous bindless by
654654
// checking both for storage buffers/images limits and some support for heterogeneous
655655
// bindings via VK_EXT_mutable_descriptor_type, VK_EXT_descriptor_buffer or descriptor
656656
// heaps.

src/dawn/native/vulkan/ResourceTableVk.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,10 @@ MaybeError ResourceTable::ApplyPendingUpdates(CommandRecordingContext* recording
192192
MaybeError ResourceTable::UpdateMetadataBuffer(CommandRecordingContext* recordingContext,
193193
const std::vector<MetadataUpdate>& updates) {
194194
// Updates the metadata buffer by scheduling a copy for each u32 that needs to be updated.
195-
// TODO(https://crbug.com/435317394): If we had a way to use Dawn reentrantly now, we could use
196-
// a compute shader to dispatch the updates instead of individual copies for each update, and
197-
// move that logic in the frontend to share it between backends. (also a single dispatch could
198-
// update multiple metadata buffers potentially).
195+
// TODO(https://issues.chromium.org/473442435): If we had a way to use Dawn reentrantly now, we
196+
// could use a compute shader to dispatch the updates instead of individual copies for each
197+
// update, and move that logic in the frontend to share it between backends. (also a single
198+
// dispatch could update multiple metadata buffers potentially).
199199
Device* device = ToBackend(GetDevice());
200200

201201
// Allocate enough space for all the data to modify and schedule the copies.
@@ -241,8 +241,8 @@ void ResourceTable::UpdateResourceBindings(const std::vector<ResourceUpdate>& up
241241
std::vector<uint32_t> arrayElements;
242242

243243
for (const ResourceUpdate& update : updates) {
244-
// TODO(https://issues.chromium.org/435317394): Support samplers updates.
245-
// TODO(https://issues.chromium.org/435317394): Support buffer, texel buffers and storage
244+
// TODO(https://issues.chromium.org/473354063): Support samplers updates.
245+
// TODO(https://issues.chromium.org/473444515): Support buffer, texel buffers and storage
246246
// textures.
247247

248248
VkImageView handle = ToBackend(update.textureView)->GetHandle();

0 commit comments

Comments
 (0)