diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index 979c7c4dc7..cfea7074d7 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -205,8 +205,6 @@ source_set("dawn_native_sources") { "BuddyMemoryAllocator.h", "Buffer.cpp", "Buffer.h", - "BufferLocation.cpp", - "BufferLocation.h", "CachedObject.cpp", "CachedObject.h", "CallbackTaskManager.cpp", diff --git a/src/dawn_native/BufferLocation.cpp b/src/dawn_native/BufferLocation.cpp deleted file mode 100644 index 5ee24f2ff8..0000000000 --- a/src/dawn_native/BufferLocation.cpp +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2021 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "dawn_native/BufferLocation.h" - -namespace dawn_native { - - BufferLocation::BufferLocation() = default; - - BufferLocation::BufferLocation(BufferBase* buffer, uint64_t offset) - : mBuffer(buffer), mOffset(offset) { - } - - BufferLocation::~BufferLocation() = default; - - // static - Ref BufferLocation::New() { - return AcquireRef(new BufferLocation()); - } - - // static - Ref BufferLocation::New(BufferBase* buffer, uint64_t offset) { - return AcquireRef(new BufferLocation(buffer, offset)); - } - - bool BufferLocation::IsNull() const { - return mBuffer.Get() == nullptr; - } - - BufferBase* BufferLocation::GetBuffer() const { - return mBuffer.Get(); - } - - uint64_t BufferLocation::GetOffset() const { - return mOffset; - } - - void BufferLocation::Set(BufferBase* buffer, uint64_t offset) { - mBuffer = buffer; - mOffset = offset; - } - -} // namespace dawn_native diff --git a/src/dawn_native/BufferLocation.h b/src/dawn_native/BufferLocation.h deleted file mode 100644 index 4ff733d972..0000000000 --- a/src/dawn_native/BufferLocation.h +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2021 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef DAWNNATIVE_BUFFERLOCATION_H_ -#define DAWNNATIVE_BUFFERLOCATION_H_ - -#include "common/RefCounted.h" -#include "dawn_native/Buffer.h" - -#include - -namespace dawn_native { - - // A ref-counted wrapper around a Buffer ref and an offset into the buffer. - class BufferLocation : public RefCounted { - public: - BufferLocation(); - BufferLocation(BufferBase* buffer, uint64_t offset = 0); - ~BufferLocation(); - - static Ref New(); - static Ref New(BufferBase* buffer, uint64_t offset = 0); - - bool IsNull() const; - - BufferBase* GetBuffer() const; - uint64_t GetOffset() const; - - void Set(BufferBase* buffer, uint64_t offset); - - private: - Ref mBuffer; - uint64_t mOffset = 0; - }; - -} // namespace dawn_native - -#endif // DAWNNATIVE_BUFFERLOCATION_H_ diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index 6b90e2cdc5..2c2ead49d6 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt @@ -50,8 +50,6 @@ target_sources(dawn_native PRIVATE "BuddyMemoryAllocator.h" "Buffer.cpp" "Buffer.h" - "BufferLocation.cpp" - "BufferLocation.h" "CachedObject.cpp" "CachedObject.h" "CallbackTaskManager.cpp" diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp index a964f40bec..3fadb1a08a 100644 --- a/src/dawn_native/CommandBuffer.cpp +++ b/src/dawn_native/CommandBuffer.cpp @@ -39,14 +39,6 @@ namespace dawn_native { Destroy(); } - void CommandBufferBase::DoNextSetValidatedBufferLocationsInternal() { - SetValidatedBufferLocationsInternalCmd* cmd = - mCommands.NextCommand(); - for (const DeferredBufferLocationUpdate& update : cmd->updates) { - update.location->Set(update.buffer.Get(), update.offset); - } - } - // static CommandBufferBase* CommandBufferBase::MakeError(DeviceBase* device) { return new CommandBufferBase(device, ObjectBase::kError); diff --git a/src/dawn_native/CommandBuffer.h b/src/dawn_native/CommandBuffer.h index c6d47ae24c..023208bca4 100644 --- a/src/dawn_native/CommandBuffer.h +++ b/src/dawn_native/CommandBuffer.h @@ -48,8 +48,6 @@ namespace dawn_native { protected: ~CommandBufferBase() override; - void DoNextSetValidatedBufferLocationsInternal(); - CommandIterator mCommands; private: diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index c9510d0327..153ef7b721 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -1018,18 +1018,6 @@ namespace dawn_native { return commandBuffer.Detach(); } - void CommandEncoder::EncodeSetValidatedBufferLocationsInternal( - std::vector updates) { - ASSERT(GetDevice()->IsValidationEnabled()); - mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { - SetValidatedBufferLocationsInternalCmd* cmd = - allocator->Allocate( - Command::SetValidatedBufferLocationsInternal); - cmd->updates = std::move(updates); - return {}; - }); - } - ResultOrError> CommandEncoder::FinishInternal( const CommandBufferDescriptor* descriptor) { DeviceBase* device = GetDevice(); diff --git a/src/dawn_native/CommandEncoder.h b/src/dawn_native/CommandEncoder.h index 6547e7d936..d390d70c2c 100644 --- a/src/dawn_native/CommandEncoder.h +++ b/src/dawn_native/CommandEncoder.h @@ -78,9 +78,6 @@ namespace dawn_native { CommandBufferBase* APIFinish(const CommandBufferDescriptor* descriptor = nullptr); - void EncodeSetValidatedBufferLocationsInternal( - std::vector updates); - private: ResultOrError> FinishInternal( const CommandBufferDescriptor* descriptor); diff --git a/src/dawn_native/Commands.cpp b/src/dawn_native/Commands.cpp index ea122e7468..c2bd0dde6d 100644 --- a/src/dawn_native/Commands.cpp +++ b/src/dawn_native/Commands.cpp @@ -158,12 +158,6 @@ namespace dawn_native { cmd->~SetStencilReferenceCmd(); break; } - case Command::SetValidatedBufferLocationsInternal: { - SetValidatedBufferLocationsInternalCmd* cmd = - commands->NextCommand(); - cmd->~SetValidatedBufferLocationsInternalCmd(); - break; - } case Command::SetViewport: { SetViewportCmd* cmd = commands->NextCommand(); cmd->~SetViewportCmd(); @@ -319,10 +313,6 @@ namespace dawn_native { commands->NextCommand(); break; - case Command::SetValidatedBufferLocationsInternal: - commands->NextCommand(); - break; - case Command::SetViewport: commands->NextCommand(); break; diff --git a/src/dawn_native/Commands.h b/src/dawn_native/Commands.h index 09acd10abf..cee9bbd52e 100644 --- a/src/dawn_native/Commands.h +++ b/src/dawn_native/Commands.h @@ -19,7 +19,6 @@ #include "dawn_native/AttachmentState.h" #include "dawn_native/BindingInfo.h" -#include "dawn_native/BufferLocation.h" #include "dawn_native/Texture.h" #include "dawn_native/dawn_platform.h" @@ -63,7 +62,6 @@ namespace dawn_native { SetBlendConstant, SetBindGroup, SetIndexBuffer, - SetValidatedBufferLocationsInternal, SetVertexBuffer, WriteBuffer, WriteTimestamp, @@ -179,7 +177,8 @@ namespace dawn_native { }; struct DrawIndexedIndirectCmd { - Ref indirectBufferLocation; + Ref indirectBuffer; + uint64_t indirectOffset; }; struct EndComputePassCmd {}; @@ -225,16 +224,6 @@ namespace dawn_native { uint32_t reference; }; - struct DeferredBufferLocationUpdate { - Ref location; - Ref buffer; - uint64_t offset; - }; - - struct SetValidatedBufferLocationsInternalCmd { - std::vector updates; - }; - struct SetViewportCmd { float x, y, width, height, minDepth, maxDepth; }; diff --git a/src/dawn_native/IndirectDrawMetadata.cpp b/src/dawn_native/IndirectDrawMetadata.cpp index e150d78a19..9d514f33df 100644 --- a/src/dawn_native/IndirectDrawMetadata.cpp +++ b/src/dawn_native/IndirectDrawMetadata.cpp @@ -159,12 +159,11 @@ namespace dawn_native { } } - void IndirectDrawMetadata::AddIndexedIndirectDraw( - wgpu::IndexFormat indexFormat, - uint64_t indexBufferSize, - BufferBase* indirectBuffer, - uint64_t indirectOffset, - BufferLocation* drawCmdIndirectBufferLocation) { + void IndirectDrawMetadata::AddIndexedIndirectDraw(wgpu::IndexFormat indexFormat, + uint64_t indexBufferSize, + BufferBase* indirectBuffer, + uint64_t indirectOffset, + DrawIndexedIndirectCmd* cmd) { uint64_t numIndexBufferElements; switch (indexFormat) { case wgpu::IndexFormat::Uint16: @@ -187,7 +186,7 @@ namespace dawn_native { IndexedIndirectDraw draw; draw.clientBufferOffset = indirectOffset; - draw.bufferLocation = drawCmdIndirectBufferLocation; + draw.cmd = cmd; it->second.AddIndexedIndirectDraw(mMaxDrawCallsPerBatch, mMaxBatchOffsetRange, std::move(draw)); } diff --git a/src/dawn_native/IndirectDrawMetadata.h b/src/dawn_native/IndirectDrawMetadata.h index f55f3d317b..e6527502b3 100644 --- a/src/dawn_native/IndirectDrawMetadata.h +++ b/src/dawn_native/IndirectDrawMetadata.h @@ -18,7 +18,6 @@ #include "common/NonCopyable.h" #include "common/RefCounted.h" #include "dawn_native/Buffer.h" -#include "dawn_native/BufferLocation.h" #include "dawn_native/CommandBufferStateTracker.h" #include "dawn_native/Commands.h" @@ -45,7 +44,10 @@ namespace dawn_native { public: struct IndexedIndirectDraw { uint64_t clientBufferOffset; - Ref bufferLocation; + // This is a pointer to the command that should be populated with the validated + // indirect scratch buffer. It is only valid up until the encoded command buffer + // is submitted. + DrawIndexedIndirectCmd* cmd; }; struct IndexedIndirectValidationBatch { @@ -109,7 +111,7 @@ namespace dawn_native { uint64_t indexBufferSize, BufferBase* indirectBuffer, uint64_t indirectOffset, - BufferLocation* drawCmdIndirectBufferLocation); + DrawIndexedIndirectCmd* cmd); private: IndexedIndirectBufferValidationInfoMap mIndexedIndirectBufferValidationInfo; diff --git a/src/dawn_native/IndirectDrawValidationEncoder.cpp b/src/dawn_native/IndirectDrawValidationEncoder.cpp index 1eba1ba312..02cef8e12c 100644 --- a/src/dawn_native/IndirectDrawValidationEncoder.cpp +++ b/src/dawn_native/IndirectDrawValidationEncoder.cpp @@ -226,7 +226,6 @@ namespace dawn_native { // single pass as possible. Batches can be grouped together as long as they're validating // data from the same indirect buffer, but they may still be split into multiple passes if // the number of draw calls in a pass would exceed some (very high) upper bound. - uint64_t numTotalDrawCalls = 0; size_t validatedParamsSize = 0; std::vector passes; IndirectDrawMetadata::IndexedIndirectBufferValidationInfoMap& bufferInfoMap = @@ -257,7 +256,6 @@ namespace dawn_native { newBatch.clientIndirectOffset = minOffsetAlignedDown; newBatch.clientIndirectSize = batch.maxOffset + kDrawIndexedIndirectSize - minOffsetAlignedDown; - numTotalDrawCalls += batch.draws.size(); newBatch.validatedParamsSize = batch.draws.size() * kDrawIndexedIndirectSize; newBatch.validatedParamsOffset = @@ -306,10 +304,7 @@ namespace dawn_native { DAWN_TRY(validatedParamsBuffer.EnsureCapacity(validatedParamsSize)); usageTracker->BufferUsedAs(validatedParamsBuffer.GetBuffer(), wgpu::BufferUsage::Indirect); - // Now we allocate and populate host-side batch data to be copied to the GPU, and prepare to - // update all DrawIndexedIndirectCmd buffer references. - std::vector deferredBufferLocationUpdates; - deferredBufferLocationUpdates.reserve(numTotalDrawCalls); + // Now we allocate and populate host-side batch data to be copied to the GPU. for (Pass& pass : passes) { // We use std::malloc here because it guarantees maximal scalar alignment. pass.batchData = {std::malloc(pass.batchDataSize), std::free}; @@ -322,16 +317,13 @@ namespace dawn_native { uint32_t* indirectOffsets = reinterpret_cast(batch.batchInfo + 1); uint64_t validatedParamsOffset = batch.validatedParamsOffset; - for (const auto& draw : batch.metadata->draws) { + for (auto& draw : batch.metadata->draws) { // The shader uses this to index an array of u32, hence the division by 4 bytes. *indirectOffsets++ = static_cast( (draw.clientBufferOffset - batch.clientIndirectOffset) / 4); - DeferredBufferLocationUpdate deferredUpdate; - deferredUpdate.location = draw.bufferLocation; - deferredUpdate.buffer = validatedParamsBuffer.GetBuffer(); - deferredUpdate.offset = validatedParamsOffset; - deferredBufferLocationUpdates.push_back(std::move(deferredUpdate)); + draw.cmd->indirectBuffer = validatedParamsBuffer.GetBuffer(); + draw.cmd->indirectOffset = validatedParamsOffset; validatedParamsOffset += kDrawIndexedIndirectSize; } @@ -364,8 +356,6 @@ namespace dawn_native { // Finally, we can now encode our validation passes. Each pass first does a single // WriteBuffer to get batch data over to the GPU, followed by a single compute pass. The // compute pass encodes a separate SetBindGroup and Dispatch command for each batch. - commandEncoder->EncodeSetValidatedBufferLocationsInternal( - std::move(deferredBufferLocationUpdates)); for (const Pass& pass : passes) { commandEncoder->APIWriteBuffer(batchDataBuffer.GetBuffer(), 0, static_cast(pass.batchData.get()), diff --git a/src/dawn_native/RenderEncoderBase.cpp b/src/dawn_native/RenderEncoderBase.cpp index a8ef2ffbf1..f2a8dce2ff 100644 --- a/src/dawn_native/RenderEncoderBase.cpp +++ b/src/dawn_native/RenderEncoderBase.cpp @@ -17,7 +17,6 @@ #include "common/Constants.h" #include "common/Log.h" #include "dawn_native/Buffer.h" -#include "dawn_native/BufferLocation.h" #include "dawn_native/CommandEncoder.h" #include "dawn_native/CommandValidation.h" #include "dawn_native/Commands.h" @@ -198,14 +197,20 @@ namespace dawn_native { DrawIndexedIndirectCmd* cmd = allocator->Allocate(Command::DrawIndexedIndirect); if (IsValidationEnabled()) { - cmd->indirectBufferLocation = BufferLocation::New(); + // Later, EncodeIndirectDrawValidationCommands will allocate a scratch storage + // buffer which will store the validated indirect data. The buffer and offset + // will be updated to point to it. + // |EncodeIndirectDrawValidationCommands| is called at the end of encoding the + // render pass, while the |cmd| pointer is still valid. + cmd->indirectBuffer = nullptr; + mIndirectDrawMetadata.AddIndexedIndirectDraw( mCommandBufferState.GetIndexFormat(), mCommandBufferState.GetIndexBufferSize(), indirectBuffer, indirectOffset, - cmd->indirectBufferLocation.Get()); + cmd); } else { - cmd->indirectBufferLocation = - BufferLocation::New(indirectBuffer, indirectOffset); + cmd->indirectBuffer = indirectBuffer; + cmd->indirectOffset = indirectOffset; } // TODO(crbug.com/dawn/1166): Adding the indirectBuffer is needed for correct usage diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp index e7d12a51a8..445c7810b1 100644 --- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp +++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp @@ -993,10 +993,6 @@ namespace dawn_native { namespace d3d12 { break; } - case Command::SetValidatedBufferLocationsInternal: - DoNextSetValidatedBufferLocationsInternal(); - break; - case Command::WriteBuffer: { WriteBufferCmd* write = mCommands.NextCommand(); const uint64_t offset = write->offset; @@ -1414,7 +1410,6 @@ namespace dawn_native { namespace d3d12 { case Command::DrawIndexedIndirect: { DrawIndexedIndirectCmd* draw = iter->NextCommand(); - ASSERT(!draw->indirectBufferLocation->IsNull()); DAWN_TRY(bindingTracker->Apply(commandContext)); vertexBufferTracker.Apply(commandList, lastPipeline); @@ -1423,12 +1418,13 @@ namespace dawn_native { namespace d3d12 { // Zero the index offset values to avoid reusing values from the previous draw RecordFirstIndexOffset(commandList, lastPipeline, 0, 0); - Buffer* buffer = ToBackend(draw->indirectBufferLocation->GetBuffer()); + Buffer* buffer = ToBackend(draw->indirectBuffer.Get()); + ASSERT(buffer != nullptr); + ComPtr signature = ToBackend(GetDevice())->GetDrawIndexedIndirectSignature(); commandList->ExecuteIndirect(signature.Get(), 1, buffer->GetD3D12Resource(), - draw->indirectBufferLocation->GetOffset(), nullptr, - 0); + draw->indirectOffset, nullptr, 0); break; } diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index cacefa03d5..f1be722b45 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -986,10 +986,6 @@ namespace dawn_native { namespace metal { break; } - case Command::SetValidatedBufferLocationsInternal: - DoNextSetValidatedBufferLocationsInternal(); - break; - case Command::WriteBuffer: { WriteBufferCmd* write = mCommands.NextCommand(); const uint64_t offset = write->offset; @@ -1341,15 +1337,16 @@ namespace dawn_native { namespace metal { bindGroups.Apply(encoder); storageBufferLengths.Apply(encoder, lastPipeline, enableVertexPulling); - ASSERT(!draw->indirectBufferLocation->IsNull()); - Buffer* buffer = ToBackend(draw->indirectBufferLocation->GetBuffer()); + Buffer* buffer = ToBackend(draw->indirectBuffer.Get()); + ASSERT(buffer != nullptr); + id indirectBuffer = buffer->GetMTLBuffer(); [encoder drawIndexedPrimitives:lastPipeline->GetMTLPrimitiveTopology() indexType:indexBufferType indexBuffer:indexBuffer indexBufferOffset:indexBufferBaseOffset indirectBuffer:indirectBuffer - indirectBufferOffset:draw->indirectBufferLocation->GetOffset()]; + indirectBufferOffset:draw->indirectOffset]; break; } diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp index b34935a0c3..f6b80ab964 100644 --- a/src/dawn_native/opengl/CommandBufferGL.cpp +++ b/src/dawn_native/opengl/CommandBufferGL.cpp @@ -844,10 +844,6 @@ namespace dawn_native { namespace opengl { break; } - case Command::SetValidatedBufferLocationsInternal: - DoNextSetValidatedBufferLocationsInternal(); - break; - case Command::WriteBuffer: { WriteBufferCmd* write = mCommands.NextCommand(); uint64_t offset = write->offset; @@ -1187,17 +1183,17 @@ namespace dawn_native { namespace opengl { case Command::DrawIndexedIndirect: { DrawIndexedIndirectCmd* draw = iter->NextCommand(); - ASSERT(!draw->indirectBufferLocation->IsNull()); vertexStateBufferBindingTracker.Apply(gl); bindGroupTracker.Apply(gl); - Buffer* indirectBuffer = ToBackend(draw->indirectBufferLocation->GetBuffer()); + Buffer* indirectBuffer = ToBackend(draw->indirectBuffer.Get()); + ASSERT(indirectBuffer != nullptr); + gl.BindBuffer(GL_DRAW_INDIRECT_BUFFER, indirectBuffer->GetHandle()); gl.DrawElementsIndirect( lastPipeline->GetGLPrimitiveTopology(), indexBufferFormat, - reinterpret_cast( - static_cast(draw->indirectBufferLocation->GetOffset()))); + reinterpret_cast(static_cast(draw->indirectOffset))); break; } diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp index 30fd55e799..cfcdb36c74 100644 --- a/src/dawn_native/vulkan/CommandBufferVk.cpp +++ b/src/dawn_native/vulkan/CommandBufferVk.cpp @@ -826,10 +826,6 @@ namespace dawn_native { namespace vulkan { break; } - case Command::SetValidatedBufferLocationsInternal: - DoNextSetValidatedBufferLocationsInternal(); - break; - case Command::WriteBuffer: { WriteBufferCmd* write = mCommands.NextCommand(); const uint64_t offset = write->offset; @@ -1075,10 +1071,10 @@ namespace dawn_native { namespace vulkan { case Command::DrawIndirect: { DrawIndirectCmd* draw = iter->NextCommand(); - VkBuffer indirectBuffer = ToBackend(draw->indirectBuffer)->GetHandle(); + Buffer* buffer = ToBackend(draw->indirectBuffer.Get()); descriptorSets.Apply(device, recordingContext, VK_PIPELINE_BIND_POINT_GRAPHICS); - device->fn.CmdDrawIndirect(commands, indirectBuffer, + device->fn.CmdDrawIndirect(commands, buffer->GetHandle(), static_cast(draw->indirectOffset), 1, 0); break; @@ -1086,14 +1082,13 @@ namespace dawn_native { namespace vulkan { case Command::DrawIndexedIndirect: { DrawIndexedIndirectCmd* draw = iter->NextCommand(); - ASSERT(!draw->indirectBufferLocation->IsNull()); - VkBuffer indirectBuffer = - ToBackend(draw->indirectBufferLocation->GetBuffer())->GetHandle(); + Buffer* buffer = ToBackend(draw->indirectBuffer.Get()); + ASSERT(buffer != nullptr); descriptorSets.Apply(device, recordingContext, VK_PIPELINE_BIND_POINT_GRAPHICS); device->fn.CmdDrawIndexedIndirect( - commands, indirectBuffer, - static_cast(draw->indirectBufferLocation->GetOffset()), 1, 0); + commands, buffer->GetHandle(), + static_cast(draw->indirectOffset), 1, 0); break; }