From 679757942fc76c18e8b6edadef295024f3d495ce Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Fri, 17 Feb 2023 16:28:48 +0100 Subject: [PATCH] Fix GPU Buffer Map/Unmap pair to prevent stall if map fails on DX11 #942 --- Source/Engine/Graphics/GPUBuffer.cpp | 3 --- Source/Engine/Graphics/GPUBuffer.h | 1 + .../GraphicsDevice/DirectX/DX11/GPUBufferDX11.cpp | 13 +++++++------ Source/Engine/Particles/ParticlesSimulation.cpp | 13 ++++++++----- .../Engine/Renderer/GI/GlobalSurfaceAtlasPass.cpp | 2 +- Source/Engine/ShadowsOfMordor/Builder.Charts.cpp | 2 +- Source/Engine/ShadowsOfMordor/Builder.cpp | 1 + 7 files changed, 19 insertions(+), 16 deletions(-) diff --git a/Source/Engine/Graphics/GPUBuffer.cpp b/Source/Engine/Graphics/GPUBuffer.cpp index a38c88dd0..6aaa939b3 100644 --- a/Source/Engine/Graphics/GPUBuffer.cpp +++ b/Source/Engine/Graphics/GPUBuffer.cpp @@ -377,12 +377,9 @@ void GPUBuffer::SetData(const void* data, uint32 size) Log::ArgumentOutOfRangeException(TEXT("Buffer.SetData")); return; } - void* mapped = Map(GPUResourceMapMode::Write); if (!mapped) - { return; - } Platform::MemoryCopy(mapped, data, size); Unmap(); } diff --git a/Source/Engine/Graphics/GPUBuffer.h b/Source/Engine/Graphics/GPUBuffer.h index 8a5418570..4d14124b3 100644 --- a/Source/Engine/Graphics/GPUBuffer.h +++ b/Source/Engine/Graphics/GPUBuffer.h @@ -190,6 +190,7 @@ public: /// /// Gets a CPU pointer to the resource by mapping its contents. Denies the GPU access to that resource. /// + /// Always call Unmap if the returned pointer is valid to release resources. /// The map operation mode. /// The pointer of the mapped CPU buffer with resource data or null if failed. API_FUNCTION() virtual void* Map(GPUResourceMapMode mode) = 0; diff --git a/Source/Engine/GraphicsDevice/DirectX/DX11/GPUBufferDX11.cpp b/Source/Engine/GraphicsDevice/DirectX/DX11/GPUBufferDX11.cpp index 5c98bf579..b83ab6a08 100644 --- a/Source/Engine/GraphicsDevice/DirectX/DX11/GPUBufferDX11.cpp +++ b/Source/Engine/GraphicsDevice/DirectX/DX11/GPUBufferDX11.cpp @@ -48,18 +48,19 @@ void* GPUBufferDX11::Map(GPUResourceMapMode mode) const HRESULT result = _device->GetIM()->Map(_resource, 0, mapType, mapFlags, &map); if (result != DXGI_ERROR_WAS_STILL_DRAWING) LOG_DIRECTX_RESULT(result); + _mapped = map.pData != nullptr; + if (!_mapped && !isMainThread) + _device->Locker.Unlock(); + return map.pData; } void GPUBufferDX11::Unmap() { - if (_mapped) - { - _mapped = false; - _device->GetIM()->Unmap(_resource, 0); - } - + ASSERT(_mapped); + _mapped = false; + _device->GetIM()->Unmap(_resource, 0); if (!IsInMainThread()) _device->Locker.Unlock(); } diff --git a/Source/Engine/Particles/ParticlesSimulation.cpp b/Source/Engine/Particles/ParticlesSimulation.cpp index 2d52a7f76..c68862925 100644 --- a/Source/Engine/Particles/ParticlesSimulation.cpp +++ b/Source/Engine/Particles/ParticlesSimulation.cpp @@ -104,15 +104,18 @@ int32 ParticleSystemInstance::GetParticlesCount() const if (GPUParticlesCountReadback && GPUParticlesCountReadback->IsAllocated()) { auto data = static_cast(GPUParticlesCountReadback->Map(GPUResourceMapMode::Read)); - for (const auto& emitter : Emitters) + if (data) { - if (emitter.Buffer && emitter.Buffer->Mode == ParticlesSimulationMode::GPU && emitter.Buffer->GPU.HasValidCount) + for (const auto& emitter : Emitters) { - result += *data; + if (emitter.Buffer && emitter.Buffer->Mode == ParticlesSimulationMode::GPU && emitter.Buffer->GPU.HasValidCount) + { + result += *data; + } + ++data; } - ++data; + GPUParticlesCountReadback->Unmap(); } - GPUParticlesCountReadback->Unmap(); } else if (Emitters.HasItems()) { diff --git a/Source/Engine/Renderer/GI/GlobalSurfaceAtlasPass.cpp b/Source/Engine/Renderer/GI/GlobalSurfaceAtlasPass.cpp index 5d87858b6..860c1b285 100644 --- a/Source/Engine/Renderer/GI/GlobalSurfaceAtlasPass.cpp +++ b/Source/Engine/Renderer/GI/GlobalSurfaceAtlasPass.cpp @@ -689,8 +689,8 @@ bool GlobalSurfaceAtlasPass::Render(RenderContext& renderContext, GPUContext* co objectsBufferCapacity = counter; notReady = false; } + _culledObjectsSizeBuffer->Unmap(); } - _culledObjectsSizeBuffer->Unmap(); // Allow to be ready if the buffer was already used if (notReady && surfaceAtlasData.CulledObjectsBuffer && surfaceAtlasData.CulledObjectsBuffer->IsAllocated()) diff --git a/Source/Engine/ShadowsOfMordor/Builder.Charts.cpp b/Source/Engine/ShadowsOfMordor/Builder.Charts.cpp index f6ecfca00..bd9974bc2 100644 --- a/Source/Engine/ShadowsOfMordor/Builder.Charts.cpp +++ b/Source/Engine/ShadowsOfMordor/Builder.Charts.cpp @@ -152,7 +152,7 @@ void ShadowsOfMordor::Builder::updateLightmaps() { auto texture = textures[textureIndex]; GPUDevice::Instance->Locker.Unlock(); - if (!texture || texture->WaitForLoaded()) + if (texture == nullptr || texture->WaitForLoaded()) { LOG(Error, "Lightmap load failed."); return; diff --git a/Source/Engine/ShadowsOfMordor/Builder.cpp b/Source/Engine/ShadowsOfMordor/Builder.cpp index ccf956d47..1ce6670d8 100644 --- a/Source/Engine/ShadowsOfMordor/Builder.cpp +++ b/Source/Engine/ShadowsOfMordor/Builder.cpp @@ -276,6 +276,7 @@ void ShadowsOfMordor::Builder::saveState() context->Flush(); Platform::Sleep(10); void* mapped = lightmapDataStaging->Map(GPUResourceMapMode::Read); + ASSERT(mapped); stream->WriteInt32(lightmapDataSize); stream->WriteBytes(mapped, lightmapDataSize); lightmapDataStaging->Unmap();