Triage Dawn TODOs

Change-Id: Ia049d5a03d0e251531f71def525492403588fd74
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/53460
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/BUILD.gn

@@ -63,7 +63,7 @@ config("dawn_native_internal") {
   # aren't on 10.11 it means we are on 10.11 and above, and Metal is available.
   # Skipping this check on 10.11 and above is important as it allows getting
   # proper compilation warning when using 10.12 and above feature for example.
-  # TODO(cwallez@chromium.org): Consider using API_AVAILABLE annotations on all
+  # TODO(crbug.com/1004024): Consider using API_AVAILABLE annotations on all
   # metal code in dawn once crbug.com/1004024 is sorted out if Chromium still
   # supports 10.10 then.
   if (is_mac && mac_deployment_target == "10.10.0") {

src/dawn_native/CopyTextureForBrowserHelper.cpp

@@ -32,7 +32,7 @@
 
 namespace dawn_native {
     namespace {
-        // TODO(shaobo.yan@intel.com) : Support premultiplay-alpha
+        // TODO(crbug.com/dawn/856) : Support premultiply-alpha
         static const char sCopyTextureForBrowserVertex[] = R"(
             [[block]] struct Uniforms {
                 u_scale : vec2<f32>;
@@ -101,7 +101,7 @@ namespace dawn_native {
             }
         )";
 
-        // TODO(shaobo.yan@intel.com): Expand copyTextureForBrowser to support any
+        // TODO(crbug.com/dawn/856): Expand copyTextureForBrowser to support any
         // non-depth, non-stencil, non-compressed texture format pair copy. Now this API
         // supports CopyImageBitmapToTexture normal format pairs.
         MaybeError ValidateCopyTextureFormatConversion(const wgpu::TextureFormat srcFormat,
@@ -281,7 +281,7 @@ namespace dawn_native {
                                        const ImageCopyTexture* destination,
                                        const Extent3D* copySize,
                                        const CopyTextureForBrowserOptions* options) {
-        // TODO(shaobo.yan@intel.com): In D3D12 and Vulkan, compatible texture format can directly
+        // TODO(crbug.com/dawn/856): In D3D12 and Vulkan, compatible texture format can directly
         // copy to each other. This can be a potential fast path.
 
         // Noop copy

src/dawn_native/RingBufferAllocator.cpp

@@ -26,7 +26,6 @@
 // only two indices that keep increasing (unbounded) but can be still indexed using bit masks.
 // However, this 1) requires the size to always be a power-of-two and 2) remove tests that check
 // used bytes.
-// TODO(bryan.bernhart@intel.com): Follow-up with ringbuffer optimization.
 namespace dawn_native {
 
     RingBufferAllocator::RingBufferAllocator(uint64_t maxSize) : mMaxBlockSize(maxSize) {

src/dawn_native/Texture.h

@@ -106,7 +106,7 @@ namespace dawn_native {
         wgpu::TextureUsage mUsage = wgpu::TextureUsage::None;
         TextureState mState;
 
-        // TODO(natlee@microsoft.com): Use a more optimized data structure to save space
+        // TODO(crbug.com/dawn/845): Use a more optimized data structure to save space
         std::vector<bool> mIsSubresourceContentInitializedAtIndex;
     };
 

src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp

@@ -137,7 +137,6 @@ namespace dawn_native { namespace d3d12 {
                 continue;
             }
 
-            // TODO(shaobo.yan@intel.com): Implement dynamic buffer offset.
             // TODO(dawn:728) In the future, special handling will be needed here for external
             // textures because they encompass multiple views.
             DescriptorType descriptorType = WGPUBindingInfoToDescriptorType(bindingInfo);

src/dawn_native/d3d12/BufferD3D12.cpp

@@ -422,7 +422,7 @@ namespace dawn_native { namespace d3d12 {
         ASSERT(GetDevice()->IsToggleEnabled(Toggle::LazyClearResourceOnFirstUse));
         ASSERT(!IsDataInitialized());
 
-        // TODO(jiawei.shao@intel.com): skip initializing the buffer when it is created on a heap
+        // TODO(crbug.com/dawn/484): skip initializing the buffer when it is created on a heap
         // that has already been zero initialized.
         DAWN_TRY(ClearBuffer(commandContext, uint8_t(0u)));
         SetIsDataInitialized();
@@ -441,7 +441,7 @@ namespace dawn_native { namespace d3d12 {
             memset(mMappedData, clearValue, GetSize());
             UnmapImpl();
         } else {
-            // TODO(jiawei.shao@intel.com): use ClearUnorderedAccessView*() when the buffer usage
+            // TODO(crbug.com/dawn/852): use ClearUnorderedAccessView*() when the buffer usage
             // includes STORAGE.
             DynamicUploader* uploader = device->GetDynamicUploader();
             UploadHandle uploadHandle;

src/dawn_native/d3d12/CommandBufferD3D12.cpp

@@ -208,7 +208,7 @@ namespace dawn_native { namespace d3d12 {
             for (size_t i = 0; i < usages.buffers.size(); ++i) {
                 Buffer* buffer = ToBackend(usages.buffers[i]);
 
-                // TODO(jiawei.shao@intel.com): clear storage buffers with
+                // TODO(crbug.com/dawn/852): clear storage buffers with
                 // ClearUnorderedAccessView*().
                 buffer->GetDevice()->ConsumedError(buffer->EnsureDataInitialized(commandContext));
 
@@ -285,7 +285,6 @@ namespace dawn_native { namespace d3d12 {
             // the signal to change the bounded heaps.
             // Re-populating all bindgroups after the last one fails causes duplicated allocations
             // to occur on overflow.
-            // TODO(bryan.bernhart@intel.com): Consider further optimization.
             bool didCreateBindGroupViews = true;
             bool didCreateBindGroupSamplers = true;
             for (BindGroupIndex index : IterateBitSet(mDirtyBindGroups)) {
@@ -793,9 +792,6 @@ namespace dawn_native { namespace d3d12 {
                         cmd->firstQuery, cmd->queryCount, destination->GetD3D12Resource(),
                         cmd->destinationOffset);
 
-                    // TODO(hao.x.li@intel.com): Add compute shader to convert the query result
-                    // (ticks) to timestamp (ns)
-
                     break;
                 }
 
@@ -1109,8 +1105,6 @@ namespace dawn_native { namespace d3d12 {
                             ->StencilBeginningAccess.Clear.ClearValue.DepthStencil.Stencil;
                 }
 
-                // TODO(kainino@chromium.org): investigate: should the Dawn clear
-                // stencil type be uint8_t?
                 if (clearFlags) {
                     commandList->ClearDepthStencilView(
                         renderPassBuilder->GetRenderPassDepthStencilDescriptor()->cpuDescriptor,

src/dawn_native/d3d12/HeapAllocatorD3D12.cpp

@@ -42,7 +42,7 @@ namespace dawn_native { namespace d3d12 {
         // It is preferred to use a size that is a multiple of the alignment.
         // However, MSAA heaps are always aligned to 4MB instead of 64KB. This means
         // if the heap size is too small, the VMM would fragment.
-        // TODO(bryan.bernhart@intel.com): Consider having MSAA vs non-MSAA heaps.
+        // TODO(crbug.com/dawn/849): Consider having MSAA vs non-MSAA heaps.
         heapDesc.Alignment = D3D12_DEFAULT_MSAA_RESOURCE_PLACEMENT_ALIGNMENT;
         heapDesc.Flags = mHeapFlags;
 

src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp

@@ -194,7 +194,7 @@ namespace dawn_native { namespace d3d12 {
             optimizedClearValue = &zero;
         }
 
-        // TODO(bryan.bernhart@intel.com): Conditionally disable sub-allocation.
+        // TODO(crbug.com/dawn/849): Conditionally disable sub-allocation.
         // For very large resources, there is no benefit to suballocate.
         // For very small resources, it is inefficent to suballocate given the min. heap
         // size could be much larger then the resource allocation.

src/dawn_native/d3d12/SwapChainD3D12.cpp

@@ -289,7 +289,7 @@ namespace dawn_native { namespace d3d12 {
         Device* device = ToBackend(GetDevice());
 
         // Transition the texture to the present state as required by IDXGISwapChain1::Present()
-        // TODO(cwallez@chromium.org): Remove the need for this by eagerly transitioning the
+        // TODO(crbug.com/dawn/269): Remove the need for this by eagerly transitioning the
         // presentable texture to present at the end of submits that use them.
         CommandRecordingContext* commandContext;
         DAWN_TRY_ASSIGN(commandContext, device->GetPendingCommandContext());
@@ -320,7 +320,7 @@ namespace dawn_native { namespace d3d12 {
 
         // Synchronously wait until previous operations on the next swapchain buffer are finished.
         // This is the logic that performs frame pacing.
-        // TODO(cwallez@chromium.org): Consider whether this should  be lifted for Mailbox so that
+        // TODO(crbug.com/dawn/269): Consider whether this should  be lifted for Mailbox so that
         // there is not frame pacing.
         mCurrentBuffer = mDXGISwapChain->GetCurrentBackBufferIndex();
         DAWN_TRY(device->WaitForSerial(mBufferLastUsedSerials[mCurrentBuffer]));

src/dawn_native/d3d12/TextureD3D12.cpp

@@ -499,7 +499,6 @@ namespace dawn_native { namespace d3d12 {
         resourceDescriptor.MipLevels = static_cast<UINT16>(GetNumMipLevels());
         resourceDescriptor.Format = dxgiFormat;
         resourceDescriptor.SampleDesc.Count = GetSampleCount();
-        // TODO(bryan.bernhart@intel.com): investigate how to specify standard MSAA sample pattern.
         resourceDescriptor.SampleDesc.Quality = 0;
         resourceDescriptor.Layout = D3D12_TEXTURE_LAYOUT_UNKNOWN;
         resourceDescriptor.Flags =
@@ -652,7 +651,6 @@ namespace dawn_native { namespace d3d12 {
                                              ExecutionSerial pendingCommandSerial) const {
         // Reuse the subresource(s) directly and avoid transition when it isn't needed, and
         // return false.
-        // TODO(cwallez@chromium.org): Need some form of UAV barriers at some point.
         if (state->lastState == newState) {
             return;
         }
@@ -1102,7 +1100,7 @@ namespace dawn_native { namespace d3d12 {
         // D3D12_SRV_DIMENSION_TEXTURE2DMS.
         // https://docs.microsoft.com/en-us/windows/desktop/api/d3d12/ns-d3d12-d3d12_tex2d_srv
         // https://docs.microsoft.com/en-us/windows/desktop/api/d3d12/ns-d3d12-d3d12_tex2d_array_srv
-        // TODO(jiawei.shao@intel.com): support more texture view dimensions.
+        // TODO(crbug.com/dawn/814): support 1D textures.
         if (GetTexture()->IsMultisampledTexture()) {
             switch (descriptor->dimension) {
                 case wgpu::TextureViewDimension::e2DArray:

src/dawn_native/metal/BufferMTL.mm

@@ -45,8 +45,6 @@ namespace dawn_native { namespace metal {
             storageMode = MTLResourceStorageModePrivate;
         }
 
-        // TODO(cwallez@chromium.org): Have a global "zero" buffer that can do everything instead
-        // of creating a new 4-byte buffer?
         if (GetSize() > std::numeric_limits<NSUInteger>::max()) {
             return DAWN_OUT_OF_MEMORY_ERROR("Buffer allocation is too large");
         }

src/dawn_native/metal/CommandBufferMTL.mm

@@ -359,7 +359,7 @@ namespace dawn_native { namespace metal {
                                     PipelineLayout* pipelineLayout) {
                 uint32_t currentDynamicBufferIndex = 0;
 
-                // TODO(kainino@chromium.org): Maintain buffers and offsets arrays in BindGroup
+                // TODO(crbug.com/dawn/854): Maintain buffers and offsets arrays in BindGroup
                 // so that we only have to do one setVertexBuffers and one setFragmentBuffers
                 // call here.
                 for (BindingIndex bindingIndex{0};
@@ -398,7 +398,7 @@ namespace dawn_native { namespace metal {
                             const id<MTLBuffer> buffer = ToBackend(binding.buffer)->GetMTLBuffer();
                             NSUInteger offset = binding.offset;
 
-                            // TODO(shaobo.yan@intel.com): Record bound buffer status to use
+                            // TODO(crbug.com/dawn/854): Record bound buffer status to use
                             // setBufferOffset to achieve better performance.
                             if (bindingInfo.buffer.hasDynamicOffset) {
                                 offset += dynamicOffsets[currentDynamicBufferIndex];
@@ -823,7 +823,7 @@ namespace dawn_native { namespace metal {
                     EnsureDestinationTextureInitialized(commandContext, dstTexture,
                                                         copy->destination, copy->copySize);
 
-                    // TODO(jiawei.shao@intel.com): support copies with 1D textures.
+                    // TODO(crbug.com/dawn/814): support copies with 1D textures.
                     ASSERT(srcTexture->GetDimension() != wgpu::TextureDimension::e1D &&
                            dstTexture->GetDimension() != wgpu::TextureDimension::e1D);
 

src/dawn_native/metal/DeviceMTL.mm

@@ -189,10 +189,10 @@ namespace dawn_native { namespace metal {
             SetToggle(Toggle::DisableBaseInstance, !haveBaseVertexBaseInstance);
         }
 
-        // TODO(jiawei.shao@intel.com): tighten this workaround when the driver bug is fixed.
+        // TODO(crbug.com/dawn/846): tighten this workaround when the driver bug is fixed.
         SetToggle(Toggle::AlwaysResolveIntoZeroLevelAndLayer, true);
 
-        // TODO(hao.x.li@intel.com): Use MTLStorageModeShared instead of MTLStorageModePrivate when
+        // TODO(crbug.com/dawn/847): Use MTLStorageModeShared instead of MTLStorageModePrivate when
         // creating MTLCounterSampleBuffer in QuerySet on Intel platforms, otherwise it fails to
         // create the buffer. Change to use MTLStorageModePrivate when the bug is fixed.
         if (@available(macOS 10.15, iOS 14.0, *)) {

src/dawn_native/metal/SwapChainMTL.mm

@@ -80,7 +80,7 @@ namespace dawn_native { namespace metal {
         ASSERT(GetSurface()->GetType() == Surface::Type::MetalLayer);
 
         if (previousSwapChain != nullptr) {
-            // TODO(cwallez@chromium.org): figure out what should happen when surfaces are used by
+            // TODO(crbug.com/dawn/269): figure out what should happen when surfaces are used by
             // multiple backends one after the other. It probably needs to block until the backend
             // and GPU are completely finished with the previous swapchain.
             if (previousSwapChain->GetBackendType() != wgpu::BackendType::Metal) {

src/dawn_native/null/DeviceNull.cpp

@@ -369,7 +369,7 @@ namespace dawn_native { namespace null {
 
     MaybeError SwapChain::Initialize(NewSwapChainBase* previousSwapChain) {
         if (previousSwapChain != nullptr) {
-            // TODO(cwallez@chromium.org): figure out what should happen when surfaces are used by
+            // TODO(crbug.com/dawn/269): figure out what should happen when surfaces are used by
             // multiple backends one after the other. It probably needs to block until the backend
             // and GPU are completely finished with the previous swapchain.
             if (previousSwapChain->GetBackendType() != wgpu::BackendType::Null) {

src/dawn_native/opengl/BufferGL.cpp

@@ -35,8 +35,6 @@ namespace dawn_native { namespace opengl {
 
     Buffer::Buffer(Device* device, const BufferDescriptor* descriptor)
         : BufferBase(device, descriptor) {
-        // TODO(cwallez@chromium.org): Have a global "zero" buffer instead of creating a new 4-byte
-        // buffer?
         uint64_t size = GetAppliedSize();
 
         device->gl.GenBuffers(1, &mBuffer);
@@ -69,8 +67,6 @@ namespace dawn_native { namespace opengl {
     }
 
     uint64_t Buffer::GetAppliedSize() const {
-        // TODO(cwallez@chromium.org): Have a global "zero" buffer instead of creating a new 4-byte
-        // buffer?
         return std::max(GetSize(), uint64_t(4u));
     }
 
@@ -151,7 +147,7 @@ namespace dawn_native { namespace opengl {
 
         EnsureDataInitialized();
 
-        // TODO(cwallez@chromium.org): this does GPU->CPU synchronization, we could require a high
+        // This does GPU->CPU synchronization, we could require a high
         // version of OpenGL that would let us map the buffer unsynchronized.
         gl.BindBuffer(GL_ARRAY_BUFFER, mBuffer);
         void* mappedData = nullptr;

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -774,7 +774,7 @@ namespace dawn_native { namespace opengl {
                     auto& src = copy->source;
                     auto& dst = copy->destination;
 
-                    // TODO(jiawei.shao@intel.com): add workaround for the case that imageExtentSrc
+                    // TODO(crbug.com/dawn/817): add workaround for the case that imageExtentSrc
                     // is not equal to imageExtentDst. For example when copySize fits in the virtual
                     // size of the source image but does not fit in the one of the destination
                     // image.
@@ -805,7 +805,7 @@ namespace dawn_native { namespace opengl {
                 }
 
                 case Command::ResolveQuerySet: {
-                    // TODO(hao.x.li@intel.com): Resolve non-precise occlusion query.
+                    // TODO(crbug.com/dawn/434): Resolve non-precise occlusion query.
                     SkipCommand(&mCommands, type);
                     break;
                 }
@@ -849,7 +849,6 @@ namespace dawn_native { namespace opengl {
                     bindGroupTracker.Apply(gl);
 
                     gl.DispatchCompute(dispatch->x, dispatch->y, dispatch->z);
-                    // TODO(cwallez@chromium.org): add barriers to the API
                     gl.MemoryBarrier(GL_ALL_BARRIER_BITS);
                     break;
                 }
@@ -863,7 +862,6 @@ namespace dawn_native { namespace opengl {
 
                     gl.BindBuffer(GL_DISPATCH_INDIRECT_BUFFER, indirectBuffer->GetHandle());
                     gl.DispatchComputeIndirect(static_cast<GLintptr>(indirectBufferOffset));
-                    // TODO(cwallez@chromium.org): add barriers to the API
                     gl.MemoryBarrier(GL_ALL_BARRIER_BITS);
                     break;
                 }
@@ -964,8 +962,6 @@ namespace dawn_native { namespace opengl {
 
                 // Attach depth/stencil buffer.
                 GLenum glAttachment = 0;
-                // TODO(kainino@chromium.org): it may be valid to just always use
-                // GL_DEPTH_STENCIL_ATTACHMENT here.
                 if (format.aspects == (Aspect::Depth | Aspect::Stencil)) {
                     glAttachment = GL_DEPTH_STENCIL_ATTACHMENT;
                 } else if (format.aspects == Aspect::Depth) {

src/dawn_native/opengl/TextureGL.cpp

@@ -210,7 +210,7 @@ namespace dawn_native { namespace opengl {
 
     MaybeError Texture::ClearTexture(const SubresourceRange& range,
                                      TextureBase::ClearValue clearValue) {
-        // TODO(jiawei.shao@intel.com): initialize the textures with compressed formats.
+        // TODO(crbug.com/dawn/850): initialize the textures with compressed formats.
         if (GetFormat().isCompressed) {
             return {};
         }
@@ -539,7 +539,7 @@ namespace dawn_native { namespace opengl {
             mHandle = ToBackend(texture)->GetHandle();
         } else {
             // glTextureView() is supported on OpenGL version >= 4.3
-            // TODO(jiawei.shao@intel.com): support texture view on OpenGL version <= 4.2
+            // TODO(crbug.com/dawn/593): support texture view on OpenGL version <= 4.2 and ES
             const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
             mHandle = GenTexture(gl);
             const Texture* textureGL = ToBackend(texture);

src/dawn_native/vulkan/BindGroupVk.cpp

@@ -138,7 +138,7 @@ namespace dawn_native { namespace vulkan {
             numWrites++;
         }
 
-        // TODO(cwallez@chromium.org): Batch these updates
+        // TODO(crbug.com/dawn/855): Batch these updates
         device->fn.UpdateDescriptorSets(device->GetVkDevice(), numWrites, writes.data(), 0,
                                         nullptr);
     }

src/dawn_native/vulkan/BufferVk.cpp

@@ -153,8 +153,6 @@ namespace dawn_native { namespace vulkan {
         createInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO;
         createInfo.pNext = nullptr;
         createInfo.flags = 0;
-        // TODO(cwallez@chromium.org): Have a global "zero" buffer that can do everything instead
-        // of creating a new 4-byte buffer?
         createInfo.size = std::max(GetSize(), uint64_t(4u));
         // Add CopyDst for non-mappable buffer initialization with mappedAtCreation
         // and robust resource initialization.
@@ -263,7 +261,7 @@ namespace dawn_native { namespace vulkan {
 
         CommandRecordingContext* recordingContext = device->GetPendingRecordingContext();
 
-        // TODO(jiawei.shao@intel.com): initialize mapped buffer in CPU side.
+        // TODO(crbug.com/dawn/852): initialize mapped buffer in CPU side.
         EnsureDataInitialized(recordingContext);
 
         if (mode & wgpu::MapMode::Read) {
@@ -352,8 +350,8 @@ namespace dawn_native { namespace vulkan {
         TransitionUsageNow(recordingContext, wgpu::BufferUsage::CopyDst);
 
         Device* device = ToBackend(GetDevice());
-        // TODO(jiawei.shao@intel.com): find out why VK_WHOLE_SIZE doesn't work on old Windows Intel
+        // This code is fine. According to jiawei.shao@intel.com, VK_WHOLE_SIZE doesn't work
-        // Vulkan drivers.
+        // on old Windows Intel Vulkan drivers, so we don't use it.
         device->fn.CmdFillBuffer(recordingContext->commandBuffer, mHandle, 0, GetSize(),
                                  clearValue);
     }

src/dawn_native/vulkan/BufferVk.h

@@ -35,7 +35,7 @@ namespace dawn_native { namespace vulkan {
 
         // Transitions the buffer to be used as `usage`, recording any necessary barrier in
         // `commands`.
-        // TODO(cwallez@chromium.org): coalesce barriers and do them early when possible.
+        // TODO(crbug.com/dawn/851): coalesce barriers and do them early when possible.
         void TransitionUsageNow(CommandRecordingContext* recordingContext, wgpu::BufferUsage usage);
         bool TransitionUsageAndGetResourceBarrier(wgpu::BufferUsage usage,
                                                   VkBufferMemoryBarrier* barrier,

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -91,7 +91,7 @@ namespace dawn_native { namespace vulkan {
                     region.srcOffset.z = srcCopy.origin.z;
                     break;
                 case wgpu::TextureDimension::e1D:
-                    // TODO(jiawei.shao@intel.com): support 1D textures
+                    // TODO(crbug.com/dawn/814): support 1D textures
                     UNREACHABLE();
             }
 
@@ -110,7 +110,7 @@ namespace dawn_native { namespace vulkan {
                     region.dstOffset.z = dstCopy.origin.z;
                     break;
                 case wgpu::TextureDimension::e1D:
-                    // TODO(jiawei.shao@intel.com): support 1D textures
+                    // TODO(crbug.com/dawn/814): support 1D textures
                     UNREACHABLE();
             }
 

src/dawn_native/vulkan/DeviceVk.cpp

@@ -449,7 +449,7 @@ namespace dawn_native { namespace vulkan {
     }
 
     void Device::InitTogglesFromDriver() {
-        // TODO(jiawei.shao@intel.com): tighten this workaround when this issue is fixed in both
+        // TODO(crbug.com/dawn/857): tighten this workaround when this issue is fixed in both
         // Vulkan SPEC and drivers.
         SetToggle(Toggle::UseTemporaryBufferInCompressedTextureToTextureCopy, true);
 

src/dawn_native/vulkan/NativeSwapChainImplVk.cpp

@@ -48,13 +48,13 @@ namespace dawn_native { namespace vulkan {
             if (!chooseSwapPresentMode(info.presentModes, turnOffVsync, &presentMode)) {
                 return false;
             }
-            // TODO(cwallez@chromium.org): For now this is hardcoded to what works with one NVIDIA
+            // TODO(crbug.com/dawn/269): For now this is hardcoded to what works with one NVIDIA
             // driver. Need to generalize
             config->nativeFormat = VK_FORMAT_B8G8R8A8_UNORM;
             config->colorSpace = VK_COLOR_SPACE_SRGB_NONLINEAR_KHR;
             config->format = wgpu::TextureFormat::BGRA8Unorm;
             config->minImageCount = 3;
-            // TODO(cwallez@chromium.org): This is upside down compared to what we want, at least
+            // TODO(crbug.com/dawn/269): This is upside down compared to what we want, at least
             // on Linux
             config->preTransform = info.capabilities.currentTransform;
             config->presentMode = presentMode;
@@ -109,7 +109,7 @@ namespace dawn_native { namespace vulkan {
         ASSERT(mInfo.capabilities.maxImageExtent.height >= height);
 
         ASSERT(format == static_cast<WGPUTextureFormat>(GetPreferredFormat()));
-        // TODO(cwallez@chromium.org): need to check usage works too
+        // TODO(crbug.com/dawn/269): need to check usage works too
 
         // Create the swapchain with the configuration we chose
         VkSwapchainKHR oldSwapchain = mSwapChain;

src/dawn_native/vulkan/PipelineLayoutVk.cpp

@@ -33,7 +33,7 @@ namespace dawn_native { namespace vulkan {
 
     MaybeError PipelineLayout::Initialize() {
         // Compute the array of VkDescriptorSetLayouts that will be chained in the create info.
-        // TODO(cwallez@chromium.org) Vulkan doesn't allow holes in this array, should we expose
+        // TODO(crbug.com/dawn/277) Vulkan doesn't allow holes in this array, should we expose
         // this constraints at the Dawn level?
         uint32_t numSetLayouts = 0;
         std::array<VkDescriptorSetLayout, kMaxBindGroups> setLayouts;

src/dawn_native/vulkan/RenderPipelineVk.cpp

@@ -457,7 +457,6 @@ namespace dawn_native { namespace vulkan {
         // LogicOp isn't supported so we disable it.
         colorBlend.logicOpEnable = VK_FALSE;
         colorBlend.logicOp = VK_LOGIC_OP_CLEAR;
-        // TODO(cwallez@chromium.org): Do we allow holes in the color attachments?
         colorBlend.attachmentCount = static_cast<uint32_t>(GetColorAttachmentsMask().count());
         colorBlend.pAttachments = colorBlendAttachments.data();
         // The blend constant is always dynamic so we fill in a dummy value

src/dawn_native/vulkan/ResourceMemoryAllocatorVk.cpp

@@ -26,7 +26,7 @@ namespace dawn_native { namespace vulkan {
 
     namespace {
 
-        // TODO(cwallez@chromium.org): This is a hardcoded heurstic to choose when to
+        // TODO(crbug.com/dawn/849): This is a hardcoded heurstic to choose when to
         // suballocate but it should ideally depend on the size of the memory heaps and other
         // factors.
         constexpr uint64_t kMaxSizeForSubAllocation = 4ull * 1024ull * 1024ull;  // 4MiB
@@ -134,7 +134,7 @@ namespace dawn_native { namespace vulkan {
 
         // Sub-allocate non-mappable resources because at the moment the mapped pointer
         // is part of the resource and not the heap, which doesn't match the Vulkan model.
-        // TODO(cwallez@chromium.org): allow sub-allocating mappable resources, maybe.
+        // TODO(crbug.com/dawn/849): allow sub-allocating mappable resources, maybe.
         if (requirements.size < kMaxSizeForSubAllocation && !mappable) {
             ResourceMemoryAllocation subAllocation;
             DAWN_TRY_ASSIGN(subAllocation,
@@ -185,7 +185,7 @@ namespace dawn_native { namespace vulkan {
 
             // Suballocations aren't freed immediately, otherwise another resource allocation could
             // happen just after that aliases the old one and would require a barrier.
-            // TODO(cwallez@chromium.org): Maybe we can produce the correct barriers to reduce the
+            // TODO(crbug.com/dawn/851): Maybe we can produce the correct barriers to reduce the
             // latency to reclaim memory.
             case AllocationMethod::kSubAllocated:
                 mSubAllocationsToDelete.Enqueue(*allocation, mDevice->GetPendingCommandSerial());

src/dawn_native/vulkan/SwapChainVk.cpp

@@ -233,19 +233,19 @@ namespace dawn_native { namespace vulkan {
         VkSwapchainKHR previousVkSwapChain = VK_NULL_HANDLE;
 
         if (previousSwapChain != nullptr) {
-            // TODO(cwallez@chromium.org): The first time a surface is used with a Device, check
+            // TODO(crbug.com/dawn/269): The first time a surface is used with a Device, check
             // it is supported with vkGetPhysicalDeviceSurfaceSupportKHR.
 
-            // TODO(cwallez@chromium.org): figure out what should happen when surfaces are used by
+            // TODO(crbug.com/dawn/269): figure out what should happen when surfaces are used by
             // multiple backends one after the other. It probably needs to block until the backend
             // and GPU are completely finished with the previous swapchain.
             if (previousSwapChain->GetBackendType() != wgpu::BackendType::Vulkan) {
                 return DAWN_VALIDATION_ERROR("vulkan::SwapChain cannot switch between APIs");
             }
-            // TODO(cwallez@chromium.org): use ToBackend once OldSwapChainBase is removed.
+            // TODO(crbug.com/dawn/269): use ToBackend once OldSwapChainBase is removed.
             SwapChain* previousVulkanSwapChain = static_cast<SwapChain*>(previousSwapChain);
 
-            // TODO(cwallez@chromium.org): Figure out switching a single surface between multiple
+            // TODO(crbug.com/dawn/269): Figure out switching a single surface between multiple
             // Vulkan devices on different VkInstances. Probably needs to block too!
             VkInstance previousInstance =
                 ToBackend(previousSwapChain->GetDevice())->GetVkInstance();
@@ -447,7 +447,7 @@ namespace dawn_native { namespace vulkan {
                 config.extent = surfaceInfo.capabilities.currentExtent;
             }
 
-            // TODO(cwallez@chromium.org): If the swapchain image doesn't support TRANSFER_DST
+            // TODO(crbug.com/dawn/269): If the swapchain image doesn't support TRANSFER_DST
             // then we'll need to have a second fallback that uses a blit shader :(
             if ((supportedUsages & VK_IMAGE_USAGE_TRANSFER_DST_BIT) == 0) {
                 return DAWN_INTERNAL_ERROR(
@@ -492,14 +492,14 @@ namespace dawn_native { namespace vulkan {
                                     mTexture->GetHandle(), mTexture->GetCurrentLayoutForSwapChain(),
                                     1, &region, VK_FILTER_LINEAR);
 
-            // TODO(cwallez@chromium.org): Find a way to reuse the blit texture between frames
+            // TODO(crbug.com/dawn/269): Find a way to reuse the blit texture between frames
             // instead of creating a new one every time. This will involve "un-destroying" the
             // texture or making the blit texture "external".
             mBlitTexture->APIDestroy();
             mBlitTexture = nullptr;
         }
 
-        // TODO(cwallez@chromium.org): Remove the need for this by eagerly transitioning the
+        // TODO(crbug.com/dawn/269): Remove the need for this by eagerly transitioning the
         // presentable texture to present at the end of submits that use them and ideally even
         // folding that in the free layout transition at the end of render passes.
         mTexture->TransitionUsageNow(recordingContext, kPresentTextureUsage,
@@ -510,7 +510,7 @@ namespace dawn_native { namespace vulkan {
         // Assuming that the present queue is the same as the graphics queue, the proper
         // synchronization has already been done on the queue so we don't need to wait on any
         // semaphores.
-        // TODO(cwallez@chromium.org): Support the present queue not being the main queue.
+        // TODO(crbug.com/dawn/269): Support the present queue not being the main queue.
         VkPresentInfoKHR presentInfo;
         presentInfo.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR;
         presentInfo.pNext = nullptr;
@@ -541,7 +541,7 @@ namespace dawn_native { namespace vulkan {
             case VK_ERROR_OUT_OF_DATE_KHR:
                 return Initialize(this);
 
-            // TODO(cwallez@chromium.org): Allow losing the surface at Dawn's API level?
+            // TODO(crbug.com/dawn/269): Allow losing the surface at Dawn's API level?
             case VK_ERROR_SURFACE_LOST_KHR:
             default:
                 return CheckVkSuccess(::VkResult(result), "QueuePresent");
@@ -572,7 +572,7 @@ namespace dawn_native { namespace vulkan {
             VkFence{}, &mLastImageIndex));
 
         if (result == VK_SUCCESS) {
-            // TODO(cwallez@chromium.org) put the semaphore on the texture so it is waited on when
+            // TODO(crbug.com/dawn/269) put the semaphore on the texture so it is waited on when
             // used instead of directly on the recording context?
             device->GetPendingRecordingContext()->waitSemaphores.push_back(semaphore);
         } else {
@@ -582,7 +582,7 @@ namespace dawn_native { namespace vulkan {
         }
 
         switch (result) {
-            // TODO(cwallez@chromium.org): Introduce a mechanism to notify the application that
+            // TODO(crbug.com/dawn/269): Introduce a mechanism to notify the application that
             // the swapchain is in a suboptimal state?
             case VK_SUBOPTIMAL_KHR:
             case VK_SUCCESS:
@@ -592,7 +592,7 @@ namespace dawn_native { namespace vulkan {
                 // Prevent infinite recursive calls to GetCurrentTextureViewInternal when the
                 // swapchains always return that they are out of date.
                 if (isReentrant) {
-                    // TODO(cwallez@chromium.org): Allow losing the surface instead?
+                    // TODO(crbug.com/dawn/269): Allow losing the surface instead?
                     return DAWN_INTERNAL_ERROR(
                         "Wasn't able to recuperate the surface after a VK_ERROR_OUT_OF_DATE_KHR");
                 }
@@ -602,7 +602,7 @@ namespace dawn_native { namespace vulkan {
                 return GetCurrentTextureViewInternal(true);
             }
 
-            // TODO(cwallez@chromium.org): Allow losing the surface at Dawn's API level?
+            // TODO(crbug.com/dawn/269): Allow losing the surface at Dawn's API level?
             case VK_ERROR_SURFACE_LOST_KHR:
             default:
                 DAWN_TRY(CheckVkSuccess(::VkResult(result), "AcquireNextImage"));

src/dawn_native/vulkan/TextureVk.cpp

@@ -55,7 +55,7 @@ namespace dawn_native { namespace vulkan {
         }
 
         // Computes which vulkan access type could be required for the given Dawn usage.
-        // TODO(cwallez@chromium.org): We shouldn't need any access usages for srcAccessMask when
+        // TODO(crbug.com/dawn/269): We shouldn't need any access usages for srcAccessMask when
         // the previous usage is readonly because an execution dependency is sufficient.
         VkAccessFlags VulkanAccessFlags(wgpu::TextureUsage usage, const Format& format) {
             VkAccessFlags flags = 0;
@@ -119,7 +119,7 @@ namespace dawn_native { namespace vulkan {
                 flags |= VK_PIPELINE_STAGE_TRANSFER_BIT;
             }
             if (usage & (wgpu::TextureUsage::Sampled | kReadOnlyStorageTexture)) {
-                // TODO(cwallez@chromium.org): Only transition to the usage we care about to avoid
+                // TODO(crbug.com/dawn/851): Only transition to the usage we care about to avoid
                 // introducing FS -> VS dependencies that would prevent parallelization on tiler
                 // GPUs
                 flags |= VK_PIPELINE_STAGE_VERTEX_SHADER_BIT |
@@ -134,7 +134,7 @@ namespace dawn_native { namespace vulkan {
                 if (format.HasDepthOrStencil()) {
                     flags |= VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT |
                              VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT;
-                    // TODO(cwallez@chromium.org): This is missing the stage where the depth and
+                    // TODO(crbug.com/dawn/853): This is missing the stage where the depth and
                     // stencil values are written, but it isn't clear which one it is.
                 } else {
                     flags |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
@@ -410,7 +410,7 @@ namespace dawn_native { namespace vulkan {
                 // or a combination with something else, the texture could be in a combination of
                 // GENERAL and TRANSFER_SRC_OPTIMAL. This would be a problem, so we make CopySrc use
                 // GENERAL.
-                // TODO(cwallez@chromium.org): We no longer need to transition resources all at
+                // TODO(crbug.com/dawn/851): We no longer need to transition resources all at
                 // once and can instead track subresources so we should lift this limitation.
             case wgpu::TextureUsage::CopySrc:
                 // Read-only and write-only storage textures must use general layout because load

src/dawn_native/vulkan/TextureVk.h

@@ -66,17 +66,10 @@ namespace dawn_native { namespace vulkan {
 
         // Transitions the texture to be used as `usage`, recording any necessary barrier in
         // `commands`.
-        // TODO(cwallez@chromium.org): coalesce barriers and do them early when possible.
+        // TODO(crbug.com/dawn/851): coalesce barriers and do them early when possible.
         void TransitionUsageNow(CommandRecordingContext* recordingContext,
                                 wgpu::TextureUsage usage,
                                 const SubresourceRange& range);
-        // TODO(cwallez@chromium.org): This function should be an implementation detail of
-        // vulkan::Texture but it is currently used by the barrier tracking for compute passes.
-        void TransitionUsageAndGetResourceBarrier(wgpu::TextureUsage usage,
-                                                  const SubresourceRange& range,
-                                                  std::vector<VkImageMemoryBarrier>* imageBarriers,
-                                                  VkPipelineStageFlags* srcStages,
-                                                  VkPipelineStageFlags* dstStages);
         void TransitionUsageForPass(CommandRecordingContext* recordingContext,
                                     const TextureSubresourceUsage& textureUsages,
                                     std::vector<VkImageMemoryBarrier>* imageBarriers,
@@ -115,6 +108,11 @@ namespace dawn_native { namespace vulkan {
                                 TextureBase::ClearValue);
 
         // Implementation details of the barrier computations for the texture.
+        void TransitionUsageAndGetResourceBarrier(wgpu::TextureUsage usage,
+                                                  const SubresourceRange& range,
+                                                  std::vector<VkImageMemoryBarrier>* imageBarriers,
+                                                  VkPipelineStageFlags* srcStages,
+                                                  VkPipelineStageFlags* dstStages);
         void TransitionUsageForPassImpl(
             CommandRecordingContext* recordingContext,
             const SubresourceStorage<wgpu::TextureUsage>& subresourceUsages,

src/utils/TerribleCommandBuffer.cpp

@@ -34,9 +34,7 @@ namespace utils {
     }
 
     void* TerribleCommandBuffer::GetCmdSpace(size_t size) {
-        // TODO(kainino@chromium.org): Should we early-out if size is 0?
+        // Note: This returns non-null even if size is zero.
-        //   (Here and/or in the caller?) It might be good to make the wire receiver get a nullptr
-        //   instead of pointer to zero-sized allocation in mBuffer.
         if (size > sizeof(mBuffer)) {
             return nullptr;
         }

src/utils/TestUtils.cpp

@@ -36,7 +36,7 @@ namespace utils {
         wgpu::Extent3D textureSizeAtLevel0,
         uint32_t mipmapLevel,
         uint32_t rowsPerImage) {
-        // TODO(jiawei.shao@intel.com): support compressed texture formats
+        // Compressed texture formats not supported in this function yet.
         ASSERT(utils::GetTextureFormatBlockWidth(format) == 1);
 
         TextureDataCopyLayout layout;