diff --git a/BUILD.gn b/BUILD.gn index d36d2a56c8..9ccee8885a 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -587,12 +587,12 @@ source_set("libdawn_native_sources") { "src/dawn_native/metal/QueueMTL.mm", "src/dawn_native/metal/RenderPipelineMTL.h", "src/dawn_native/metal/RenderPipelineMTL.mm", - "src/dawn_native/metal/ResourceUploader.h", - "src/dawn_native/metal/ResourceUploader.mm", "src/dawn_native/metal/SamplerMTL.h", "src/dawn_native/metal/SamplerMTL.mm", "src/dawn_native/metal/ShaderModuleMTL.h", "src/dawn_native/metal/ShaderModuleMTL.mm", + "src/dawn_native/metal/StagingBufferMTL.h", + "src/dawn_native/metal/StagingBufferMTL.mm", "src/dawn_native/metal/SwapChainMTL.h", "src/dawn_native/metal/SwapChainMTL.mm", "src/dawn_native/metal/TextureMTL.h", diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index d076f5396b..b70ae11053 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp @@ -16,6 +16,7 @@ #include "common/Assert.h" #include "dawn_native/Device.h" +#include "dawn_native/DynamicUploader.h" #include "dawn_native/ValidationUtils_autogen.h" #include @@ -183,6 +184,26 @@ namespace dawn_native { MapReadAsyncImpl(mMapSerial); } + MaybeError BufferBase::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) { + DynamicUploader* uploader = nullptr; + DAWN_TRY_ASSIGN(uploader, GetDevice()->GetDynamicUploader()); + + // TODO(bryan.bernhart@intel.com): Remove once alignment constraint is added to validation + // (dawn:73). D3D12 does not specify so we assume 4-byte alignment to be safe. + static constexpr size_t kDefaultAlignment = 4; + + UploadHandle uploadHandle; + DAWN_TRY_ASSIGN(uploadHandle, uploader->Allocate(count, kDefaultAlignment)); + ASSERT(uploadHandle.mappedBuffer != nullptr); + + memcpy(uploadHandle.mappedBuffer, data, count); + + DAWN_TRY(GetDevice()->CopyFromStagingToBuffer( + uploadHandle.stagingBuffer, uploadHandle.startOffset, this, start, count)); + + return {}; + } + void BufferBase::MapWriteAsync(dawnBufferMapWriteCallback callback, dawnCallbackUserdata userdata) { if (GetDevice()->ConsumedError(ValidateMap(dawn::BufferUsageBit::MapWrite))) { diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h index 9ca002b2df..22bd170f53 100644 --- a/src/dawn_native/Buffer.h +++ b/src/dawn_native/Buffer.h @@ -72,7 +72,7 @@ namespace dawn_native { uint32_t dataLength); private: - virtual MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) = 0; + virtual MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data); virtual void MapReadAsyncImpl(uint32_t serial) = 0; virtual void MapWriteAsyncImpl(uint32_t serial) = 0; virtual void UnmapImpl() = 0; diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 068c753893..48e33989f9 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -21,6 +21,7 @@ #include "dawn_native/CommandBuffer.h" #include "dawn_native/CommandEncoder.h" #include "dawn_native/ComputePipeline.h" +#include "dawn_native/DynamicUploader.h" #include "dawn_native/ErrorData.h" #include "dawn_native/Fence.h" #include "dawn_native/FenceSignalTracker.h" @@ -54,9 +55,12 @@ namespace dawn_native { DeviceBase::DeviceBase(AdapterBase* adapter) : mAdapter(adapter) { mCaches = std::make_unique(); mFenceSignalTracker = std::make_unique(this); + mDynamicUploader = std::make_unique(this); } DeviceBase::~DeviceBase() { + // Devices must explicitly free the uploader + ASSERT(mDynamicUploader == nullptr); } void DeviceBase::HandleError(const char* message) { @@ -377,4 +381,11 @@ namespace dawn_native { delete error; } + ResultOrError DeviceBase::GetDynamicUploader() const { + if (mDynamicUploader->IsEmpty()) { + DAWN_TRY(mDynamicUploader->CreateAndAppendBuffer()); + } + return mDynamicUploader.get(); + } + } // namespace dawn_native diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index ac5b55011f..75ebf785ba 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -124,6 +124,11 @@ namespace dawn_native { uint32_t destinationOffset, uint32_t size) = 0; + ResultOrError GetDynamicUploader() const; + + protected: + std::unique_ptr mDynamicUploader; + private: virtual ResultOrError CreateBindGroupImpl( const BindGroupDescriptor* descriptor) = 0; diff --git a/src/dawn_native/DynamicUploader.cpp b/src/dawn_native/DynamicUploader.cpp index e127db453d..b5310e1bad 100644 --- a/src/dawn_native/DynamicUploader.cpp +++ b/src/dawn_native/DynamicUploader.cpp @@ -46,7 +46,7 @@ namespace dawn_native { newMaxSize *= 2; } - // TODO(b-brber): Fall-back to no sub-allocations should this fail. + // TODO(bryan.bernhart@intel.com): Fall-back to no sub-allocations should this fail. DAWN_TRY(CreateAndAppendBuffer(newMaxSize)); largestRingBuffer = GetLargestBuffer(); uploadHandle = largestRingBuffer->SubAllocate(alignedSize); diff --git a/src/dawn_native/DynamicUploader.h b/src/dawn_native/DynamicUploader.h index 32ed864ddd..ef4990804b 100644 --- a/src/dawn_native/DynamicUploader.h +++ b/src/dawn_native/DynamicUploader.h @@ -32,11 +32,14 @@ namespace dawn_native { RingBuffer* GetLargestBuffer(); - MaybeError CreateAndAppendBuffer(size_t size); + MaybeError CreateAndAppendBuffer(size_t size = kBaseUploadBufferSize); bool IsEmpty() const; private: + // TODO(bryan.bernhart@intel.com): Figure out this value. + static constexpr size_t kBaseUploadBufferSize = 64000; + std::vector> mRingBuffers; DeviceBase* mDevice; }; diff --git a/src/dawn_native/RingBuffer.cpp b/src/dawn_native/RingBuffer.cpp index 51e97c2841..cae3170647 100644 --- a/src/dawn_native/RingBuffer.cpp +++ b/src/dawn_native/RingBuffer.cpp @@ -27,7 +27,7 @@ // 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(b-brber): Follow-up with ringbuffer optimization. +// TODO(bryan.bernhart@intel.com): Follow-up with ringbuffer optimization. namespace dawn_native { static constexpr size_t INVALID_OFFSET = std::numeric_limits::max(); diff --git a/src/dawn_native/d3d12/BufferD3D12.cpp b/src/dawn_native/d3d12/BufferD3D12.cpp index 53a9c840d9..c24ce83a7d 100644 --- a/src/dawn_native/d3d12/BufferD3D12.cpp +++ b/src/dawn_native/d3d12/BufferD3D12.cpp @@ -17,7 +17,6 @@ #include "common/Assert.h" #include "common/Constants.h" #include "common/Math.h" -#include "dawn_native/DynamicUploader.h" #include "dawn_native/d3d12/DeviceD3D12.h" #include "dawn_native/d3d12/ResourceAllocator.h" @@ -161,24 +160,6 @@ namespace dawn_native { namespace d3d12 { } } - MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) { - Device* device = ToBackend(GetDevice()); - - DynamicUploader* uploader = nullptr; - DAWN_TRY_ASSIGN(uploader, device->GetDynamicUploader()); - - UploadHandle uploadHandle; - DAWN_TRY_ASSIGN(uploadHandle, uploader->Allocate(count, kDefaultAlignment)); - ASSERT(uploadHandle.mappedBuffer != nullptr); - - memcpy(uploadHandle.mappedBuffer, data, count); - - DAWN_TRY(device->CopyFromStagingToBuffer(uploadHandle.stagingBuffer, - uploadHandle.startOffset, this, start, count)); - - return {}; - } - void Buffer::MapReadAsyncImpl(uint32_t serial) { mWrittenMappedRange = {}; D3D12_RANGE readRange = {0, GetSize()}; diff --git a/src/dawn_native/d3d12/BufferD3D12.h b/src/dawn_native/d3d12/BufferD3D12.h index a4ac2ee4c8..9398fb574b 100644 --- a/src/dawn_native/d3d12/BufferD3D12.h +++ b/src/dawn_native/d3d12/BufferD3D12.h @@ -39,15 +39,10 @@ namespace dawn_native { namespace d3d12 { private: // Dawn API - MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override; void MapReadAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override; void UnmapImpl() override; - // TODO(b-brber): Remove once alignment constraint is added to validation (dawn:73). - static constexpr size_t kDefaultAlignment = - 4; // D3D does not specify so we assume 4-byte alignment to be safe. - ComPtr mResource; bool mFixedResourceState = false; dawn::BufferUsageBit mLastUsage = dawn::BufferUsageBit::None; diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp index ec3593cd13..593aaf8684 100644 --- a/src/dawn_native/d3d12/DeviceD3D12.cpp +++ b/src/dawn_native/d3d12/DeviceD3D12.cpp @@ -63,7 +63,6 @@ namespace dawn_native { namespace d3d12 { mDescriptorHeapAllocator = std::make_unique(this); mMapRequestTracker = std::make_unique(this); mResourceAllocator = std::make_unique(this); - mDynamicUploader = std::make_unique(this); NextSerial(); } @@ -73,6 +72,14 @@ namespace dawn_native { namespace d3d12 { WaitForSerial(mLastSubmittedSerial); // Wait for all in-flight commands to finish executing TickImpl(); // Call tick one last time so resources are cleaned up + // Free services explicitly so that they can free D3D12 resources before destruction of the + // device. + mDynamicUploader = nullptr; + + // Releasing the uploader enqueues buffers to be released. + // Call Tick() again to clear them before releasing the allocator. + mResourceAllocator->Tick(mCompletedSerial); + ASSERT(mUsedComObjectRefs.Empty()); ASSERT(mPendingCommands.commandList == nullptr); } @@ -143,12 +150,16 @@ namespace dawn_native { namespace d3d12 { void Device::TickImpl() { // Perform cleanup operations to free unused objects mCompletedSerial = mFence->GetCompletedValue(); + + // Uploader should tick before the resource allocator + // as it enqueues resources to be released. + mDynamicUploader->Tick(mCompletedSerial); + mResourceAllocator->Tick(mCompletedSerial); mCommandAllocatorManager->Tick(mCompletedSerial); mDescriptorHeapAllocator->Tick(mCompletedSerial); mMapRequestTracker->Tick(mCompletedSerial); mUsedComObjectRefs.ClearUpTo(mCompletedSerial); - mDynamicUploader->Tick(mCompletedSerial); ExecuteCommandLists({}); NextSerial(); } @@ -265,12 +276,4 @@ namespace dawn_native { namespace d3d12 { return {}; } - ResultOrError Device::GetDynamicUploader() const { - // TODO(b-brber): Refactor this into device init once moved into DeviceBase. - if (mDynamicUploader->IsEmpty()) { - DAWN_TRY(mDynamicUploader->CreateAndAppendBuffer(kDefaultUploadBufferSize)); - } - return mDynamicUploader.get(); - } - }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/DeviceD3D12.h b/src/dawn_native/d3d12/DeviceD3D12.h index 7a90b44e26..2d753d1bd6 100644 --- a/src/dawn_native/d3d12/DeviceD3D12.h +++ b/src/dawn_native/d3d12/DeviceD3D12.h @@ -77,8 +77,6 @@ namespace dawn_native { namespace d3d12 { uint32_t destinationOffset, uint32_t size) override; - ResultOrError GetDynamicUploader() const; - private: ResultOrError CreateBindGroupImpl( const BindGroupDescriptor* descriptor) override; @@ -121,11 +119,8 @@ namespace dawn_native { namespace d3d12 { std::unique_ptr mDescriptorHeapAllocator; std::unique_ptr mMapRequestTracker; std::unique_ptr mResourceAllocator; - std::unique_ptr mDynamicUploader; dawn_native::PCIInfo mPCIInfo; - - static constexpr size_t kDefaultUploadBufferSize = 64000; // DXGI min heap size is 64kB. }; }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/StagingBufferD3D12.cpp b/src/dawn_native/d3d12/StagingBufferD3D12.cpp index bb711f41ca..da5db48539 100644 --- a/src/dawn_native/d3d12/StagingBufferD3D12.cpp +++ b/src/dawn_native/d3d12/StagingBufferD3D12.cpp @@ -39,7 +39,7 @@ namespace dawn_native { namespace d3d12 { mUploadHeap = mDevice->GetResourceAllocator()->Allocate( D3D12_HEAP_TYPE_UPLOAD, resourceDescriptor, D3D12_RESOURCE_STATE_GENERIC_READ); - // TODO(b-brber): Record the GPU pointer for generic non-upload usage. + // TODO(bryan.bernhart@intel.com): Record the GPU pointer for generic non-upload usage. if (FAILED(mUploadHeap->Map(0, nullptr, &mMappedPointer))) { return DAWN_CONTEXT_LOST_ERROR("Unable to map staging buffer."); diff --git a/src/dawn_native/metal/BufferMTL.h b/src/dawn_native/metal/BufferMTL.h index f68d2f7c9b..1d9a0117be 100644 --- a/src/dawn_native/metal/BufferMTL.h +++ b/src/dawn_native/metal/BufferMTL.h @@ -34,7 +34,6 @@ namespace dawn_native { namespace metal { void OnMapCommandSerialFinished(uint32_t mapSerial, bool isWrite); private: - MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override; void MapReadAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override; void UnmapImpl() override; diff --git a/src/dawn_native/metal/BufferMTL.mm b/src/dawn_native/metal/BufferMTL.mm index 1c6bea0934..6d0aa9c79c 100644 --- a/src/dawn_native/metal/BufferMTL.mm +++ b/src/dawn_native/metal/BufferMTL.mm @@ -15,7 +15,6 @@ #include "dawn_native/metal/BufferMTL.h" #include "dawn_native/metal/DeviceMTL.h" -#include "dawn_native/metal/ResourceUploader.h" namespace dawn_native { namespace metal { @@ -49,12 +48,6 @@ namespace dawn_native { namespace metal { } } - MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) { - auto* uploader = ToBackend(GetDevice())->GetResourceUploader(); - uploader->BufferSubData(mMtlBuffer, start, count, data); - return {}; - } - void Buffer::MapReadAsyncImpl(uint32_t serial) { MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapTracker(); tracker->Track(this, serial, false); diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h index 7aa39974ca..cd45168b05 100644 --- a/src/dawn_native/metal/DeviceMTL.h +++ b/src/dawn_native/metal/DeviceMTL.h @@ -30,7 +30,6 @@ namespace dawn_native { namespace metal { class MapRequestTracker; - class ResourceUploader; class Device : public DeviceBase { public: @@ -53,7 +52,6 @@ namespace dawn_native { namespace metal { void SubmitPendingCommandBuffer(); MapRequestTracker* GetMapTracker() const; - ResourceUploader* GetResourceUploader() const; ResultOrError> CreateStagingBuffer(size_t size) override; MaybeError CopyFromStagingToBuffer(StagingBufferBase* source, @@ -90,7 +88,6 @@ namespace dawn_native { namespace metal { id mMtlDevice = nil; id mCommandQueue = nil; std::unique_ptr mMapTracker; - std::unique_ptr mResourceUploader; Serial mCompletedSerial = 0; Serial mLastSubmittedSerial = 0; diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm index 6a0f9ab5dc..26230110d1 100644 --- a/src/dawn_native/metal/DeviceMTL.mm +++ b/src/dawn_native/metal/DeviceMTL.mm @@ -26,9 +26,9 @@ #include "dawn_native/metal/PipelineLayoutMTL.h" #include "dawn_native/metal/QueueMTL.h" #include "dawn_native/metal/RenderPipelineMTL.h" -#include "dawn_native/metal/ResourceUploader.h" #include "dawn_native/metal/SamplerMTL.h" #include "dawn_native/metal/ShaderModuleMTL.h" +#include "dawn_native/metal/StagingBufferMTL.h" #include "dawn_native/metal/SwapChainMTL.h" #include "dawn_native/metal/TextureMTL.h" @@ -37,8 +37,7 @@ namespace dawn_native { namespace metal { Device::Device(AdapterBase* adapter, id mtlDevice) : DeviceBase(adapter), mMtlDevice([mtlDevice retain]), - mMapTracker(new MapRequestTracker(this)), - mResourceUploader(new ResourceUploader(this)) { + mMapTracker(new MapRequestTracker(this)) { [mMtlDevice retain]; mCommandQueue = [mMtlDevice newCommandQueue]; } @@ -58,7 +57,7 @@ namespace dawn_native { namespace metal { mPendingCommands = nil; mMapTracker = nullptr; - mResourceUploader = nullptr; + mDynamicUploader = nullptr; [mCommandQueue release]; mCommandQueue = nil; @@ -136,7 +135,7 @@ namespace dawn_native { namespace metal { } void Device::TickImpl() { - mResourceUploader->Tick(mCompletedSerial); + mDynamicUploader->Tick(mCompletedSerial); mMapTracker->Tick(mCompletedSerial); if (mPendingCommands != nil) { @@ -185,12 +184,10 @@ namespace dawn_native { namespace metal { return mMapTracker.get(); } - ResourceUploader* Device::GetResourceUploader() const { - return mResourceUploader.get(); - } - ResultOrError> Device::CreateStagingBuffer(size_t size) { - return DAWN_UNIMPLEMENTED_ERROR("Device unable to create staging buffer."); + std::unique_ptr stagingBuffer = + std::make_unique(size, this); + return std::move(stagingBuffer); } MaybeError Device::CopyFromStagingToBuffer(StagingBufferBase* source, @@ -198,7 +195,18 @@ namespace dawn_native { namespace metal { BufferBase* destination, uint32_t destinationOffset, uint32_t size) { - return DAWN_UNIMPLEMENTED_ERROR("Device unable to copy from staging buffer."); + id uploadBuffer = ToBackend(source)->GetBufferHandle(); + id buffer = ToBackend(destination)->GetMTLBuffer(); + id commandBuffer = GetPendingCommandBuffer(); + id encoder = [commandBuffer blitCommandEncoder]; + [encoder copyFromBuffer:uploadBuffer + sourceOffset:sourceOffset + toBuffer:buffer + destinationOffset:destinationOffset + size:size]; + [encoder endEncoding]; + + return {}; } }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/Forward.h b/src/dawn_native/metal/Forward.h index df8b1d021b..fb9c4c8654 100644 --- a/src/dawn_native/metal/Forward.h +++ b/src/dawn_native/metal/Forward.h @@ -40,6 +40,7 @@ namespace dawn_native { namespace metal { class RenderPipeline; class Sampler; class ShaderModule; + class StagingBuffer; class SwapChain; class Texture; class TextureView; @@ -59,6 +60,7 @@ namespace dawn_native { namespace metal { using RenderPipelineType = RenderPipeline; using SamplerType = Sampler; using ShaderModuleType = ShaderModule; + using StagingBufferType = StagingBuffer; using SwapChainType = SwapChain; using TextureType = Texture; using TextureViewType = TextureView; diff --git a/src/dawn_native/metal/ResourceUploader.mm b/src/dawn_native/metal/ResourceUploader.mm deleted file mode 100644 index fd990374a5..0000000000 --- a/src/dawn_native/metal/ResourceUploader.mm +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2017 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/metal/ResourceUploader.h" - -#include "dawn_native/metal/DeviceMTL.h" - -namespace dawn_native { namespace metal { - - ResourceUploader::ResourceUploader(Device* device) : mDevice(device) { - } - - ResourceUploader::~ResourceUploader() { - ASSERT(mInflightUploadBuffers.Empty()); - } - - void ResourceUploader::BufferSubData(id buffer, - uint32_t start, - uint32_t size, - const void* data) { - // TODO(cwallez@chromium.org) use a ringbuffer instead of creating a small buffer for each - // update - id uploadBuffer = - [mDevice->GetMTLDevice() newBufferWithLength:size options:MTLResourceStorageModeShared]; - memcpy([uploadBuffer contents], data, size); - - id commandBuffer = mDevice->GetPendingCommandBuffer(); - id encoder = [commandBuffer blitCommandEncoder]; - [encoder copyFromBuffer:uploadBuffer - sourceOffset:0 - toBuffer:buffer - destinationOffset:start - size:size]; - [encoder endEncoding]; - - mInflightUploadBuffers.Enqueue(uploadBuffer, mDevice->GetPendingCommandSerial()); - } - - void ResourceUploader::Tick(Serial finishedSerial) { - for (id buffer : mInflightUploadBuffers.IterateUpTo(finishedSerial)) { - [buffer release]; - } - mInflightUploadBuffers.ClearUpTo(finishedSerial); - } - -}} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/ResourceUploader.h b/src/dawn_native/metal/StagingBufferMTL.h similarity index 58% rename from src/dawn_native/metal/ResourceUploader.h rename to src/dawn_native/metal/StagingBufferMTL.h index d6e057b8ad..e2d1ecc100 100644 --- a/src/dawn_native/metal/ResourceUploader.h +++ b/src/dawn_native/metal/StagingBufferMTL.h @@ -1,4 +1,4 @@ -// Copyright 2017 The Dawn Authors +// Copyright 2018 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. @@ -12,11 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef DAWNNATIVE_METAL_RESOURCEUPLOADER_H_ -#define DAWNNATIVE_METAL_RESOURCEUPLOADER_H_ +#ifndef DAWNNATIVE_STAGINGBUFFERMETAL_H_ +#define DAWNNATIVE_STAGINGBUFFERMETAL_H_ -#include "common/Serial.h" -#include "common/SerialQueue.h" +#include "dawn_native/StagingBuffer.h" #import @@ -24,19 +23,19 @@ namespace dawn_native { namespace metal { class Device; - class ResourceUploader { + class StagingBuffer : public StagingBufferBase { public: - ResourceUploader(Device* device); - ~ResourceUploader(); + StagingBuffer(size_t size, Device* device); + ~StagingBuffer(); - void BufferSubData(id buffer, uint32_t start, uint32_t size, const void* data); - void Tick(Serial finishedSerial); + id GetBufferHandle() const; + + MaybeError Initialize() override; private: Device* mDevice; - SerialQueue> mInflightUploadBuffers; + id mBuffer = nil; }; - }} // namespace dawn_native::metal -#endif // DAWNNATIVE_METAL_RESOURCEUPLOADER_H_ +#endif // DAWNNATIVE_STAGINGBUFFERMETAL_H_ diff --git a/src/dawn_native/metal/StagingBufferMTL.mm b/src/dawn_native/metal/StagingBufferMTL.mm new file mode 100644 index 0000000000..f03a7691c5 --- /dev/null +++ b/src/dawn_native/metal/StagingBufferMTL.mm @@ -0,0 +1,41 @@ +// Copyright 2018 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/metal/StagingBufferMTL.h" +#include "dawn_native/metal/DeviceMTL.h" + +namespace dawn_native { namespace metal { + + StagingBuffer::StagingBuffer(size_t size, Device* device) + : StagingBufferBase(size), mDevice(device) { + } + + MaybeError StagingBuffer::Initialize() { + const size_t bufferSize = GetSize(); + mBuffer = [mDevice->GetMTLDevice() newBufferWithLength:bufferSize + options:MTLResourceStorageModeShared]; + mMappedPointer = [mBuffer contents]; + return {}; + } + + StagingBuffer::~StagingBuffer() { + [mBuffer release]; + mBuffer = nil; + } + + id StagingBuffer::GetBufferHandle() const { + return mBuffer; + } + +}} // namespace dawn_native::metal \ No newline at end of file diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp index 070a817c9b..338d581b4a 100644 --- a/src/dawn_native/null/DeviceNull.cpp +++ b/src/dawn_native/null/DeviceNull.cpp @@ -58,10 +58,10 @@ namespace dawn_native { namespace null { // Device Device::Device(Adapter* adapter) : DeviceBase(adapter) { - mDynamicUploader = std::make_unique(this); } Device::~Device() { + mDynamicUploader = nullptr; } ResultOrError Device::CreateBindGroupImpl( diff --git a/src/dawn_native/null/DeviceNull.h b/src/dawn_native/null/DeviceNull.h index 770b61e997..052a624e3b 100644 --- a/src/dawn_native/null/DeviceNull.h +++ b/src/dawn_native/null/DeviceNull.h @@ -136,7 +136,6 @@ namespace dawn_native { namespace null { Serial mCompletedSerial = 0; Serial mLastSubmittedSerial = 0; std::vector> mPendingOperations; - std::unique_ptr mDynamicUploader; }; class Buffer : public BufferBase { diff --git a/src/dawn_native/opengl/DeviceGL.cpp b/src/dawn_native/opengl/DeviceGL.cpp index 42f8f26f86..dd0f262bff 100644 --- a/src/dawn_native/opengl/DeviceGL.cpp +++ b/src/dawn_native/opengl/DeviceGL.cpp @@ -44,6 +44,9 @@ namespace dawn_native { namespace opengl { // on a serial that doesn't have a corresponding fence enqueued. Force all // operations to look as if they were completed (because they were). mCompletedSerial = mLastSubmittedSerial + 1; + + mDynamicUploader = nullptr; + Tick(); } diff --git a/src/dawn_native/vulkan/BufferVk.cpp b/src/dawn_native/vulkan/BufferVk.cpp index ac4ef65505..3b7ef7820d 100644 --- a/src/dawn_native/vulkan/BufferVk.cpp +++ b/src/dawn_native/vulkan/BufferVk.cpp @@ -14,7 +14,6 @@ #include "dawn_native/vulkan/BufferVk.h" -#include "dawn_native/DynamicUploader.h" #include "dawn_native/vulkan/DeviceVk.h" #include "dawn_native/vulkan/FencedDeleter.h" @@ -196,24 +195,6 @@ namespace dawn_native { namespace vulkan { mLastUsage = usage; } - MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) { - Device* device = ToBackend(GetDevice()); - - DynamicUploader* uploader = nullptr; - DAWN_TRY_ASSIGN(uploader, device->GetDynamicUploader()); - - UploadHandle uploadHandle; - DAWN_TRY_ASSIGN(uploadHandle, uploader->Allocate(count, kDefaultAlignment)); - ASSERT(uploadHandle.mappedBuffer != nullptr); - - memcpy(uploadHandle.mappedBuffer, data, count); - - DAWN_TRY(device->CopyFromStagingToBuffer(uploadHandle.stagingBuffer, - uploadHandle.startOffset, this, start, count)); - - return {}; - } - void Buffer::MapReadAsyncImpl(uint32_t serial) { Device* device = ToBackend(GetDevice()); diff --git a/src/dawn_native/vulkan/BufferVk.h b/src/dawn_native/vulkan/BufferVk.h index 7ab37791f0..ee703b4067 100644 --- a/src/dawn_native/vulkan/BufferVk.h +++ b/src/dawn_native/vulkan/BufferVk.h @@ -41,14 +41,10 @@ namespace dawn_native { namespace vulkan { void TransitionUsageNow(VkCommandBuffer commands, dawn::BufferUsageBit usage); private: - MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override; void MapReadAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override; void UnmapImpl() override; - // TODO(b-brber): Remove once alignment constraint is added to validation (dawn:73). - static constexpr size_t kDefaultAlignment = 4; // TODO(b-brber): Figure out this value. - VkBuffer mHandle = VK_NULL_HANDLE; DeviceMemoryAllocation mMemoryAllocation; diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index 559a95462c..56222146c2 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/src/dawn_native/vulkan/DeviceVk.cpp @@ -63,8 +63,6 @@ namespace dawn_native { namespace vulkan { mDeleter = std::make_unique(this); mMapRequestTracker = std::make_unique(this); mMemoryAllocator = std::make_unique(this); - mDynamicUploader = std::make_unique(this); - mRenderPassCache = std::make_unique(this); return {}; @@ -117,8 +115,8 @@ namespace dawn_native { namespace vulkan { // Free services explicitly so that they can free Vulkan objects before vkDestroyDevice mDynamicUploader = nullptr; - // Releasing the uploader enqueues buffers to be deleted. - // Call Tick() again to allow the deleter to clear them prior to being released. + // Releasing the uploader enqueues buffers to be released. + // Call Tick() again to clear them before releasing the deleter. mDeleter->Tick(mCompletedSerial); mDeleter = nullptr; @@ -209,7 +207,11 @@ namespace dawn_native { namespace vulkan { RecycleCompletedCommands(); mMapRequestTracker->Tick(mCompletedSerial); + + // Uploader should tick before the resource allocator + // as it enqueues resources to be released. mDynamicUploader->Tick(mCompletedSerial); + mMemoryAllocator->Tick(mCompletedSerial); mDeleter->Tick(mCompletedSerial); @@ -529,13 +531,4 @@ namespace dawn_native { namespace vulkan { return {}; } - - ResultOrError Device::GetDynamicUploader() const { - // TODO(b-brber): Refactor this into device init once moved into DeviceBase. - if (mDynamicUploader->IsEmpty()) { - DAWN_TRY(mDynamicUploader->CreateAndAppendBuffer(kDefaultUploadBufferSize)); - } - return mDynamicUploader.get(); - } - }} // namespace dawn_native::vulkan diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h index ddbfc4c188..4d6e03508b 100644 --- a/src/dawn_native/vulkan/DeviceVk.h +++ b/src/dawn_native/vulkan/DeviceVk.h @@ -79,9 +79,6 @@ namespace dawn_native { namespace vulkan { BufferBase* destination, uint32_t destinationOffset, uint32_t size) override; - - ResultOrError GetDynamicUploader() const; - private: ResultOrError CreateBindGroupImpl( const BindGroupDescriptor* descriptor) override; @@ -117,7 +114,6 @@ namespace dawn_native { namespace vulkan { uint32_t mQueueFamily = 0; VkQueue mQueue = VK_NULL_HANDLE; - std::unique_ptr mDynamicUploader; std::unique_ptr mDeleter; std::unique_ptr mMapRequestTracker; std::unique_ptr mMemoryAllocator; @@ -148,9 +144,6 @@ namespace dawn_native { namespace vulkan { std::vector mUnusedCommands; CommandPoolAndBuffer mPendingCommands; std::vector mWaitSemaphores; - - static constexpr size_t kDefaultUploadBufferSize = - 64000; // TODO(b-brber): Figure out this value. }; }} // namespace dawn_native::vulkan diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp index 901ada8b06..e677ebc83e 100644 --- a/src/tests/end2end/BufferTests.cpp +++ b/src/tests/end2end/BufferTests.cpp @@ -180,10 +180,6 @@ TEST_P(BufferSetSubDataTests, SmallDataAtOffset) { // Stress test for many calls to SetSubData TEST_P(BufferSetSubDataTests, ManySetSubData) { - // TODO(cwallez@chromium.org): Use ringbuffers for SetSubData on explicit APIs. - // otherwise this creates too many resources and can take freeze the driver(?) - DAWN_SKIP_TEST_IF(IsMetal()); - // Note: Increasing the size of the buffer will likely cause timeout issues. // In D3D12, timeout detection occurs when the GPU scheduler tries but cannot preempt the task // executing these commands in-flight. If this takes longer than ~2s, a device reset occurs and diff --git a/src/tests/end2end/CopyTests.cpp b/src/tests/end2end/CopyTests.cpp index 1031f37537..7077ae7d9d 100644 --- a/src/tests/end2end/CopyTests.cpp +++ b/src/tests/end2end/CopyTests.cpp @@ -396,7 +396,7 @@ TEST_P(CopyTests_T2B, Texture2DArrayRegion) { // Test that copying texture 2D array mips with 256-byte aligned sizes works TEST_P(CopyTests_T2B, Texture2DArrayMip) { - // TODO(b-brber): Figure out why this test fails on Intel Linux. + // TODO(bryan.bernhart@intel.com): Figure out why this test fails on Intel Linux. // See https://bugs.chromium.org/p/dawn/issues/detail?id=101 DAWN_SKIP_TEST_IF(IsLinux() && IsVulkan() && IsIntel()); constexpr uint32_t kWidth = 256; diff --git a/src/tests/unittests/RingBufferTests.cpp b/src/tests/unittests/RingBufferTests.cpp index 1b8fc7bcf5..9e903a6153 100644 --- a/src/tests/unittests/RingBufferTests.cpp +++ b/src/tests/unittests/RingBufferTests.cpp @@ -33,7 +33,7 @@ namespace { class RingBufferTests : public testing::Test { protected: void SetUp() override { - // TODO(b-brber): Create this device through the adapter. + // TODO(bryan.bernhart@intel.com): Create this device through the adapter. mDevice = std::make_unique(/*adapter*/ nullptr); } @@ -165,7 +165,7 @@ TEST_F(RingBufferTests, RingBufferSubAlloc) { // In this example, Tick(8) could not reclaim the wasted bytes. The wasted bytes // were add to F9's sub-allocation. - // TODO(b-brber): Decide if Tick(8) should free these wasted bytes. + // TODO(bryan.bernhart@intel.com): Decide if Tick(8) should free these wasted bytes. ASSERT_EQ(offset, 0u); ASSERT_EQ(buffer->GetUsedSize(), frameSizeInBytes * maxNumOfFrames);