Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions code/framework/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ list(APPEND FRAMEWORK_CLIENT_SRC
src/gui/view.cpp
src/gui/sdk.cpp
src/gui/backend/view_d3d11.cpp
src/gui/backend/view_d3d12.cpp
src/gui/cef/app.cpp
src/gui/cef/renderer_app.cpp
src/gui/cef/client.cpp
src/gui/cef/disk_resource_handler.cpp
src/gui/cef/render_handler.cpp
src/gui/cef/life_span_handler.cpp
src/gui/cef/load_handler.cpp
Expand Down
148 changes: 140 additions & 8 deletions code/framework/src/graphics/backend/d3d12.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,25 @@
#include "d3d12.h"

#include <logging/logger.h>
#include <utils/process_shutdown.h>

#include <wrl/client.h>

namespace Framework::Graphics {
bool D3D12Backend::Init(const Framework::Graphics::RendererConfiguration &opts) {
const auto swapChain = opts.d3d12.swapchain;
const auto commandQueue = opts.d3d12.commandQueue;
_device = opts.d3d12.device;
_context = opts.d3d12.deviceContext;
// #1 get device from swapchain (maybe different device)
// #1 get device from swapchain — and use it as THE device everywhere:
// descriptor heaps/RTVs below are created from it, so SRVs, textures
// and fences created later through GetDevice() must match or D3D12
// rejects the mixed-device usage. opts.d3d12.device is ignored if it
// differs.
ID3D12Device *pD3DDevice;
if (FAILED(swapChain->GetDevice(__uuidof(ID3D12Device), (void **)&pD3DDevice))) {
return false;
}
_device = pD3DDevice;

_swapChain = swapChain;
_commandQueue = commandQueue;
Expand All @@ -39,12 +46,21 @@ namespace Framework::Graphics {
{
D3D12_DESCRIPTOR_HEAP_DESC desc {};
desc.Type = D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV;
desc.NumDescriptors = _frameBufferCount;
desc.NumDescriptors = _frameBufferCount + kExtraSrvSlots;
desc.Flags = D3D12_DESCRIPTOR_HEAP_FLAG_SHADER_VISIBLE;

if (pD3DDevice->CreateDescriptorHeap(&desc, IID_PPV_ARGS(&_srvHeap)) != S_OK) {
return false;
}

_srvDescriptorSize = pD3DDevice->GetDescriptorHandleIncrementSize(D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV);

// Slots [0, _frameBufferCount) stay reserved for ImGui (its font SRV
// sits at the heap start); the rest are up for allocation
_freeSrvSlots.clear();
for (UINT i = 0; i < kExtraSrvSlots; i++) {
_freeSrvSlots.push_back(_frameBufferCount + i);
}
}

// #3 create rtv heap
Expand All @@ -62,10 +78,11 @@ namespace Framework::Graphics {
const auto rtvDescriptorSize = pD3DDevice->GetDescriptorHandleIncrementSize(D3D12_DESCRIPTOR_HEAP_TYPE_RTV);
D3D12_CPU_DESCRIPTOR_HANDLE rtvHandle = _rtvHeap->GetCPUDescriptorHandleForHeapStart();

// Reserve one RTV descriptor slot per frame index; the back-buffer
// resource (and its RTV) is bound fresh each frame in Begin(), so we
// don't grab the buffers here.
for (UINT i = 0; i < _frameBufferCount; i++) {
_frameContext[i]._mainRenderTargetDescriptor = rtvHandle;
swapChain->GetBuffer(i, IID_PPV_ARGS(&_frameContext[i]._mainRenderTargetResource));
pD3DDevice->CreateRenderTargetView(_frameContext[i]._mainRenderTargetResource, nullptr, rtvHandle);
rtvHandle.ptr += rtvDescriptorSize;
}
}
Expand Down Expand Up @@ -93,22 +110,37 @@ namespace Framework::Graphics {

void D3D12Backend::Shutdown() {
// release objects
if (_currentBackBuffer) {
_currentBackBuffer->Release();
_currentBackBuffer = nullptr;
}
_rtvHeap->Release();
_srvHeap->Release();
_commandList->Release();
for (const auto &frameContext : _frameContext) {
frameContext._commandAllocator->Release();
frameContext._mainRenderTargetResource->Release();
}
}

void D3D12Backend::Begin() {
const auto &currentFrameContext = _frameContext[_swapChain->GetCurrentBackBufferIndex()];
const UINT idx = _swapChain->GetCurrentBackBufferIndex();
const auto &currentFrameContext = _frameContext[idx];
currentFrameContext._commandAllocator->Reset();

// Acquire the current back buffer fresh (AddRef'd) and bind its RTV. It is
// released in End() — we never hold it across frames. The swapchain owns
// the buffer, so dropping our reference doesn't free it; the GPU keeps
// reading it for this frame via the swapchain's own hold.
_currentBackBuffer = nullptr;
if (FAILED(_swapChain->GetBuffer(idx, IID_PPV_ARGS(&_currentBackBuffer)))) {
_currentBackBuffer = nullptr;
return;
}
_device->CreateRenderTargetView(_currentBackBuffer, nullptr, currentFrameContext._mainRenderTargetDescriptor);

_barrier.Type = D3D12_RESOURCE_BARRIER_TYPE_TRANSITION;
_barrier.Flags = D3D12_RESOURCE_BARRIER_FLAG_NONE;
_barrier.Transition.pResource = currentFrameContext._mainRenderTargetResource;
_barrier.Transition.pResource = _currentBackBuffer;
_barrier.Transition.Subresource = D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES;
_barrier.Transition.StateBefore = D3D12_RESOURCE_STATE_PRESENT;
_barrier.Transition.StateAfter = D3D12_RESOURCE_STATE_RENDER_TARGET;
Expand All @@ -120,16 +152,116 @@ namespace Framework::Graphics {
}

void D3D12Backend::End() {
if (!_currentBackBuffer) {
return; // Begin() failed to acquire this frame
}

_barrier.Transition.StateBefore = D3D12_RESOURCE_STATE_RENDER_TARGET;
_barrier.Transition.StateAfter = D3D12_RESOURCE_STATE_PRESENT;
_commandList->ResourceBarrier(1, &_barrier);
_commandList->Close();
_commandQueue->ExecuteCommandLists(1, (ID3D12CommandList **)&_commandList);

// Drop our per-frame reference now that the work is submitted. Safe: the
// swapchain still owns the buffer, so it stays alive for the GPU.
_currentBackBuffer->Release();
_currentBackBuffer = nullptr;
}

void D3D12Backend::Update() {}

int D3D12Backend::NumFramesInFlight() const {
return _frameBufferCount;
}

int D3D12Backend::AllocateSRVSlot() {
if (!_srvHeap) {
Framework::Logging::GetLogger(FRAMEWORK_INNER_GRAPHICS)->error("D3D12Backend::AllocateSRVSlot, no descriptor heap");
return -1;
}
if (_freeSrvSlots.empty()) {
Framework::Logging::GetLogger(FRAMEWORK_INNER_GRAPHICS)->error("D3D12Backend::AllocateSRVSlot, heap exhausted");
return -1;
}
const auto slot = _freeSrvSlots.back();
_freeSrvSlots.pop_back();
return static_cast<int>(slot);
}

void D3D12Backend::FreeSRVSlot(int slot) {
if (slot < 0) {
return;
}
_freeSrvSlots.push_back(static_cast<UINT>(slot));
}
Comment on lines +191 to +196

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate SRV slot bounds and prevent duplicate frees

Line 195 currently accepts any non-negative slot, including ImGui-reserved indices and out-of-range values. Combined with Lines 203/212, that can generate invalid descriptor handles and alias descriptors after accidental double-free.

Suggested minimal hardening
+#include <algorithm>
...
     void D3D12Backend::FreeSRVSlot(int slot) {
-        if (slot < 0) {
+        const int minSlot = static_cast<int>(_frameBufferCount);
+        const int maxSlot = static_cast<int>(_frameBufferCount + kExtraSrvSlots);
+        if (slot < minSlot || slot >= maxSlot) {
             return;
         }
-        _freeSrvSlots.push_back(static_cast<UINT>(slot));
+        const UINT uSlot = static_cast<UINT>(slot);
+        if (std::find(_freeSrvSlots.begin(), _freeSrvSlots.end(), uSlot) != _freeSrvSlots.end()) {
+            return;
+        }
+        _freeSrvSlots.push_back(uSlot);
     }
 
     D3D12_CPU_DESCRIPTOR_HANDLE D3D12Backend::GetSRVSlotCPUHandle(int slot) const {
-        if (!_srvHeap) {
+        const int minSlot = static_cast<int>(_frameBufferCount);
+        const int maxSlot = static_cast<int>(_frameBufferCount + kExtraSrvSlots);
+        if (!_srvHeap || slot < minSlot || slot >= maxSlot) {
             return {};
         }
...
     D3D12_GPU_DESCRIPTOR_HANDLE D3D12Backend::GetSRVSlotGPUHandle(int slot) const {
-        if (!_srvHeap) {
+        const int minSlot = static_cast<int>(_frameBufferCount);
+        const int maxSlot = static_cast<int>(_frameBufferCount + kExtraSrvSlots);
+        if (!_srvHeap || slot < minSlot || slot >= maxSlot) {
             return {};
         }

Also applies to: 198-205, 207-214

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/framework/src/graphics/backend/d3d12.cpp` around lines 191 - 196, The
FreeSRVSlot method currently only validates that the slot is non-negative but
fails to check if the slot is within the valid range, if it has already been
freed, or if it is a reserved index for ImGui. Add bounds checking to ensure the
slot is within the maximum valid descriptor range, add a check to prevent
duplicate frees by verifying the slot is not already in the _freeSrvSlots list,
and add validation to skip any ImGui-reserved slot indices. Apply identical
validation logic to the other slot-freeing methods referenced at lines 198-205
and 207-214 to prevent invalid descriptor handles and aliasing issues across all
similar functions.


D3D12_CPU_DESCRIPTOR_HANDLE D3D12Backend::GetSRVSlotCPUHandle(int slot) const {
if (!_srvHeap) {
return {};
}
auto handle = _srvHeap->GetCPUDescriptorHandleForHeapStart();
handle.ptr += static_cast<SIZE_T>(slot) * _srvDescriptorSize;
return handle;
}

D3D12_GPU_DESCRIPTOR_HANDLE D3D12Backend::GetSRVSlotGPUHandle(int slot) const {
if (!_srvHeap) {
return {};
}
auto handle = _srvHeap->GetGPUDescriptorHandleForHeapStart();
handle.ptr += static_cast<UINT64>(slot) * _srvDescriptorSize;
return handle;
}

bool D3D12Backend::WaitForGpu() {
if (!_commandQueue || !_device) {
return true; // nothing to drain
}

// During process teardown the game has already destroyed its device and
// queue; touching them is UB and the fence would never signal anyway.
// Freeing without the drain is safe here - in-flight work died with
// the queue's threads
if (Framework::Utils::IsProcessShutdownInProgress()) {
return true;
}

Microsoft::WRL::ComPtr<ID3D12Fence> fence;
if (FAILED(_device->CreateFence(0, D3D12_FENCE_FLAG_NONE, IID_PPV_ARGS(&fence)))) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Framework::Logging::GetLogger(FRAMEWORK_INNER_GRAPHICS)->error("D3D12Backend::WaitForGpu, CreateFence failed");
return false;
}

const HANDLE event = CreateEventW(nullptr, FALSE, FALSE, nullptr);
if (!event) {
Framework::Logging::GetLogger(FRAMEWORK_INNER_GRAPHICS)->error("D3D12Backend::WaitForGpu, CreateEvent failed");
return false;
}

bool drained = false;
if (FAILED(_commandQueue->Signal(fence.Get(), 1))) {
// Generally means the device was removed; in-flight work is dead,
// but we can't prove it, so report failure and let callers decide
Framework::Logging::GetLogger(FRAMEWORK_INNER_GRAPHICS)->error("D3D12Backend::WaitForGpu, queue Signal failed (device removed?)");
}
else if (fence->GetCompletedValue() >= 1) {
drained = true;
}
else if (SUCCEEDED(fence->SetEventOnCompletion(1, event))) {
// A live GPU signals in single-digit ms; the timeout only bites
// when the queue is wedged - which is precisely when freeing
// GPU-referenced resources is unsafe, hence the failure return
if (WaitForSingleObject(event, 500) == WAIT_OBJECT_0 || fence->GetCompletedValue() >= 1) {
drained = true;
}
else {
Framework::Logging::GetLogger(FRAMEWORK_INNER_GRAPHICS)->error("D3D12Backend::WaitForGpu, fence wait timed out");
}
}
else {
Framework::Logging::GetLogger(FRAMEWORK_INNER_GRAPHICS)->error("D3D12Backend::WaitForGpu, SetEventOnCompletion failed");
}
CloseHandle(event);
return drained;
}
} // namespace Framework::Graphics
49 changes: 48 additions & 1 deletion code/framework/src/graphics/backend/d3d12.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@

namespace Framework::Graphics {
class D3D12Backend: public Backend<ID3D12Device *, ID3D12DeviceContext *, IDXGISwapChain3 *, ID3D12CommandQueue *> {
public:
// Extra shader-visible SRV slots reserved after the ImGui font slot(s),
// handed out to web views via AllocateSRVSlot
static constexpr UINT kExtraSrvSlots = 64;

private:
IDXGISwapChain3 *_swapChain = nullptr;
UINT _frameBufferCount = 0;
Expand All @@ -23,15 +28,23 @@ namespace Framework::Graphics {
ID3D12GraphicsCommandList *_commandList = nullptr;
ID3D12CommandQueue *_commandQueue = nullptr;

UINT _srvDescriptorSize = 0;
std::vector<UINT> _freeSrvSlots;

struct FrameContext {
ID3D12CommandAllocator *_commandAllocator = nullptr;
ID3D12Resource *_mainRenderTargetResource = nullptr;
D3D12_CPU_DESCRIPTOR_HANDLE _mainRenderTargetDescriptor;
};

std::vector<FrameContext> _frameContext;
D3D12_RESOURCE_BARRIER _barrier {};

// The current frame's back buffer, acquired fresh in Begin() and released
// in End(). We deliberately never hold a swapchain reference across
// frames so the game can resize/recreate the swapchain (which happens
// between presents) without our reference blocking DXGI's ResizeBuffers.
ID3D12Resource *_currentBackBuffer = nullptr;

public:
bool Init(const Framework::Graphics::RendererConfiguration &opts) override;
void Shutdown() override;
Expand All @@ -40,6 +53,16 @@ namespace Framework::Graphics {
void End();
int NumFramesInFlight() const;

// Since we no longer cache the back buffers, a swapchain resize or even a
// full swapchain recreation needs nothing from us beyond pointing at the
// current swapchain — Begin() re-acquires the back buffer each frame.
IDXGISwapChain3 *GetSwapChain() const {
return _swapChain;
}
void SetSwapChain(IDXGISwapChain3 *swapChain) {
_swapChain = swapChain;
}

// TODO: Backend not implemented yet
void BeginDrawing() {}
void EndDrawing() {}
Expand All @@ -66,5 +89,29 @@ namespace Framework::Graphics {
ID3D12GraphicsCommandList *GetGraphicsCommandList() const {
return _commandList;
}

UINT GetCurrentFrameIndex() const {
return _swapChain ? _swapChain->GetCurrentBackBufferIndex() : 0;
}

// Allocates a shader-visible SRV slot from the backend heap (the one ImGui
// is initialized against, so handles are usable as ImTextureID). Returns -1
// when exhausted.
int AllocateSRVSlot();
void FreeSRVSlot(int slot);
D3D12_CPU_DESCRIPTOR_HANDLE GetSRVSlotCPUHandle(int slot) const;
D3D12_GPU_DESCRIPTOR_HANDLE GetSRVSlotGPUHandle(int slot) const;

// Blocks until the GPU has drained the queue. Used before destroying
// resources that in-flight command lists may still reference. Returns
// false when the drain could not be confirmed (fence timeout/failure)
// so callers can refuse to free still-referenced resources; returns
// true during process teardown, where waiting is impossible and
// freeing is safe (the queue's threads are already gone).
[[nodiscard]] bool WaitForGpu();

size_t GetFreeSRVSlotCount() const {
return _freeSrvSlots.size();
}
};
} // namespace Framework::Graphics
3 changes: 2 additions & 1 deletion code/framework/src/graphics/renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ namespace Framework::Graphics {
return RendererError::RENDERER_ALREADY_INITIALIZED;
}

_config = config;
_config = config;
_backend = config.backend;

if (_config.backend == RendererBackend::BACKEND_D3D_11) {
_d3d11Backend = std::make_unique<D3D11Backend>();
Expand Down
42 changes: 24 additions & 18 deletions code/framework/src/gui/backend/view_d3d11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,25 +175,31 @@ namespace Framework::GUI {
else {
// CPU path: use pixel data from CEF's OnPaint
auto *renderHandler = GetRenderHandler();
if (renderHandler && !renderHandler->GetPixelData().empty()) {
Graphics::Bitmap bmp;
bmp.format = Graphics::BitmapFormat::BGRA8;
bmp.width = _width;
bmp.height = _height;
bmp.pitch = _width * 4;
bmp.size = static_cast<uint32_t>(renderHandler->GetPixelData().size());
bmp.pixels = const_cast<uint8_t *>(renderHandler->GetPixelData().data());

if (_cpuTextureID == 0) {
_cpuTextureID = backend->NextTextureId();
backend->CreateTexture(_cpuTextureID, bmp);
}
else if (renderHandler->IsPixelDataDirty()) {
backend->UpdateTexture(_cpuTextureID, bmp);
renderHandler->ClearPixelDataDirty();
}
if (renderHandler) {
// Held for the whole read — OnPaint reallocates the buffer on
// resize
const auto pixelLock = renderHandler->LockPixels();

if (!renderHandler->GetPixelData().empty()) {
Graphics::Bitmap bmp;
bmp.format = Graphics::BitmapFormat::BGRA8;
bmp.width = _width;
bmp.height = _height;
bmp.pitch = _width * 4;
bmp.size = static_cast<uint32_t>(renderHandler->GetPixelData().size());
bmp.pixels = const_cast<uint8_t *>(renderHandler->GetPixelData().data());

if (_cpuTextureID == 0) {
_cpuTextureID = backend->NextTextureId();
backend->CreateTexture(_cpuTextureID, bmp);
}
else if (renderHandler->IsPixelDataDirty()) {
backend->UpdateTexture(_cpuTextureID, bmp);
renderHandler->ClearPixelDataDirty();
}

gpuState.texture_1_id = _cpuTextureID;
gpuState.texture_1_id = _cpuTextureID;
}
}
}

Expand Down
Loading
Loading