Skip to content

Commit

Permalink
Rework FrameData resource to use global binding.
Browse files Browse the repository at this point in the history
  • Loading branch information
Flone-dnb committed Nov 17, 2024
1 parent d17637b commit ef74f25
Show file tree
Hide file tree
Showing 10 changed files with 203 additions and 101 deletions.
18 changes: 0 additions & 18 deletions src/engine_lib/private/render/directx/DirectXRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -634,15 +634,6 @@ namespace ne {
// Bind global shader resources.
pDirectXPso->bindGlobalShaderResourceViews(pCommandList, iCurrentFrameResourceIndex);

// Set CBV to frame constant buffer.
pCommandList->SetGraphicsRootConstantBufferView(
pMtxPsoResources->second
.vSpecialRootParameterIndices[static_cast<size_t>(SpecialRootParameterSlot::FRAME_DATA)],
reinterpret_cast<DirectXResource*>(
pCurrentFrameResource->pFrameConstantBuffer->getInternalResource())
->getInternalResource()
->GetGPUVirtualAddress());

for (const auto& materialInfo : pipelineInfo.vMaterials) {
// No need to bind material's shader resources since they are not used in vertex shader
// (since we are in depth prepass).
Expand Down Expand Up @@ -1396,15 +1387,6 @@ namespace ne {
// Bind global shader resources.
pDirectXPso->bindGlobalShaderResourceViews(pCommandList, iCurrentFrameResourceIndex);

// Set CBV to frame constant buffer.
pCommandList->SetGraphicsRootConstantBufferView(
pipelineData
.vSpecialRootParameterIndices[static_cast<size_t>(SpecialRootParameterSlot::FRAME_DATA)],
reinterpret_cast<DirectXResource*>(
pCurrentFrameResource->pFrameConstantBuffer->getInternalResource())
->getInternalResource()
->GetGPUVirtualAddress());

// Bind directional shadow maps.
pCommandList->SetGraphicsRootDescriptorTable(
pipelineData.vSpecialRootParameterIndices[static_cast<size_t>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,13 @@ namespace ne {
// getUniquePsoIdentifier()));
// }

mtxInternalResources.second.globalShaderResourceCbvs.clear();
mtxInternalResources.second.globalShaderResourceSrvs.clear();
mtxInternalResources.second.descriptorTablesToBind.clear();

#if defined(DEBUG)
static_assert(
sizeof(InternalResources) == 320, // NOLINT: current struct size
sizeof(InternalResources) == 400, // NOLINT: current struct size
"release new resources here");
#endif

Expand Down
31 changes: 27 additions & 4 deletions src/engine_lib/private/render/directx/pipeline/DirectXPso.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,25 @@ namespace ne {
vSpecialRootParameterIndices;

/**
* Global bindings that should be binded as SRVs. Stores pairs of "root parameter index" -
* Global bindings that should be bound as CBVs. Stores pairs of "root parameter index" -
* "resource to bind".
*
* @remark It's safe to store raw pointers here because the SRVs should be valid
* while the pipeline exists and has not re-created its internal resources (when internal
* resources are re-created these SRVs (resource pointers) are cleared).
* @remark It's safe to store raw pointers to resources here because the resources
* must be valid white they are used in the pipeline (so when a pipeline is no longer used it's
* destroyed and thus this array will be empty) but when the pipeline recreates its internal
* resources to apply some changes it clears this array and expects the resources to be
* rebound.
*/
std::unordered_map<
UINT,
std::array<DirectXResource*, FrameResourceManager::getFrameResourceCount()>>
globalShaderResourceCbvs;

/**
* Global bindings that should be bound as SRVs. Stores pairs of "root parameter index" -
* "resource to bind".
*
* @remark See @ref globalShaderResourceCbvs on why it's safe to store raw pointers here.
*/
std::unordered_map<
UINT,
Expand Down Expand Up @@ -178,6 +191,16 @@ namespace ne {
// No need to lock internal resources mutex since the caller is expected to lock that mutex
// because this function is expected to be called inside of the `draw` function.

// Bind global CBVs.
for (const auto& [iRootParameterIndex, vResourcesToBind] :
mtxInternalResources.second.globalShaderResourceCbvs) {
pCommandList->SetGraphicsRootConstantBufferView(
iRootParameterIndex,
vResourcesToBind[iCurrentFrameResourceIndex]
->getInternalResource()
->GetGPUVirtualAddress());
}

// Bind global SRVs.
for (const auto& [iRootParameterIndex, vResourcesToBind] :
mtxInternalResources.second.globalShaderResourceSrvs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,26 @@ namespace ne {
return Error(hResult);
}

// Make sure the resource is created.
if (pFrameConstantBuffer == nullptr) [[unlikely]] {
return Error("expected the resource to be created at this point");
}

// Cast to DirectX resource.
const auto pDirectXResource =
dynamic_cast<DirectXResource*>(pFrameConstantBuffer->getInternalResource());
if (pDirectXResource == nullptr) [[unlikely]] {
return Error("expected a DirectX resource");
}

// Bind CBV so that when the base class creates a global shader resource binding
// the binding type will be determined as a constant buffer.
auto optionalError = pDirectXResource->bindDescriptor(DirectXDescriptorType::CBV);
if (optionalError.has_value()) [[unlikely]] {
optionalError->addCurrentLocationToErrorStack();
return optionalError;
}

return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "render/directx/DirectXRenderer.h"
#include "render/directx/resource/DirectXFrameResource.h"
#endif
#include "shader/general/Shader.h"
#include "shader/general/resource/binding/global/GlobalShaderResourceBindingManager.h"
#include "render/general/resource/GpuResourceManager.h"
#include "render/vulkan/VulkanRenderer.h"
#include "render/vulkan/resource/VulkanFrameResource.h"
Expand Down Expand Up @@ -56,6 +58,8 @@ namespace ne {
FrameResourceManager::create(Renderer* pRenderer) {
auto pManager = std::unique_ptr<FrameResourceManager>(new FrameResourceManager(pRenderer));

std::array<GpuResource*, FrameResourceManager::getFrameResourceCount()> vFrameResources;

for (unsigned int i = 0; i < getFrameResourceCount(); i++) {
// Create a constant buffer with frame-global data per frame.
auto result = pRenderer->getResourceManager()->createResourceWithCpuWriteAccess(
Expand All @@ -74,6 +78,18 @@ namespace ne {
optionalError->addCurrentLocationToErrorStack();
return optionalError.value();
}

// Save to bind to pipelines later.
vFrameResources[i] = pManager->vFrameResources[i]->pFrameConstantBuffer->getInternalResource();
}

// Bind frame data to all pipelines.
auto optionalError = pRenderer->getGlobalShaderResourceBindingManager()
->createGlobalShaderResourceBindingResourcePerFrame(
Shader::getFrameConstantsShaderResourceName(), vFrameResources);
if (optionalError.has_value()) [[unlikely]] {
optionalError->addCurrentLocationToErrorStack();
return optionalError.value();
}

return pManager;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,19 @@ namespace ne {
* use @ref createGlobalShaderResourceBindingSingleResource that accepts a single GPU resource
* pointer (preferred) or just pass an array of the same pointers.
*
* @remark The actual type of the binding (cbuffer, uniform, structured buffer, storage buffer or
* etc) will be determined from the resource. For example, in DirectX in order to bind a `cbuffer`
* shader resource you need to pass a resource that already has a CBV binded and in Vulkan in
* order to bind a `uniform` you need to make sure that your resource was created with
* the "uniform" hint/flag.
*
* @param sShaderResourceName Name of the shader resource (name from shader code) to bind the
* resources.
* @param vResourcesToBind Resources to bind to pipelines. This function will create a binding
* that binds a separate GPU resource per frame (can be used for some CPU-write resources).
* If you have a CPU-write GPU buffer that you plan to update without CPU-GPU synchronization (for
* example each time the CPU is submitting a new frame) then you need to pass a separate buffer per
* frame resource to avoid modifying a buffer that may be used by the GPU.
* that binds a separate GPU resource per frame, this is generally used for some CPU-write resources
* so in this case you have a CPU-write GPU buffer that you plan to update without CPU-GPU
* synchronization (for example each time the CPU is submitting a new frame) then you need to pass a
* separate buffer per frame resource to avoid modifying a buffer that may be used by the GPU.
*
* @return Error if something went wrong.
*/
Expand All @@ -59,14 +65,14 @@ namespace ne {
* basis) and assigns it to the specified resources. When resources will be destroyed
* the binding will also be removed.
*
* @remark Also see @ref createGlobalShaderResourceBindingResourcePerFrame.
* @remark Also see @ref createGlobalShaderResourceBindingResourcePerFrame for important remarks.
*
* @param sShaderResourceName Name of the shader resource (name from shader code) to bind the
* resource.
* @param pResourceToBind Resources to bind to pipelines. This function will create a binding
* that binds the same GPU resource for all frames in-flight (this can be used for textures
* or some buffer resources). This is used when you guarantee the CPU-GPU synchronization or
* don't plan to update the resource's contents.
* don't plan to update the resource's contents from the CPU.
*
* @return Error if something went wrong.
*/
Expand Down
33 changes: 0 additions & 33 deletions src/engine_lib/private/shader/hlsl/RootSignatureGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,39 +288,6 @@ namespace ne {
vTableRanges.reserve(iMaxRootParameterCount);
const auto iInitialCapacity = vTableRanges.capacity();

// Prepare to add "frame data" root parameter.
bool bAddedFrameData = false;
const auto addFrameDataParameter = [&](RootParameter& rootParameter) {
// Add root parameter.
const auto iRootParameterIndex = static_cast<unsigned int>(vRootParameters.size());
vSpecialRootParameterIndices[static_cast<size_t>(SpecialRootParameterSlot::FRAME_DATA)] =
iRootParameterIndex;
vRootParameters.push_back(rootParameter.generateSingleDescriptorDescription());

addedRootParameterNames.insert(Shader::getFrameConstantsShaderResourceName());
rootParameterIndices[Shader::getFrameConstantsShaderResourceName()] = iRootParameterIndex;

bAddedFrameData = true;
};

// Check if vertex shader uses frame data cbuffer.
auto frameDataIndexIt =
vertexRootSigInfo.rootParameterIndices.find(Shader::getFrameConstantsShaderResourceName());
if (frameDataIndexIt != vertexRootSigInfo.rootParameterIndices.end()) {
// Add root parameter.
addFrameDataParameter(frameDataIndexIt->second.second);
}

if (!bAddedFrameData && pPixelRootSigInfo != nullptr) {
// Check for pixel shader.
frameDataIndexIt =
pPixelRootSigInfo->rootParameterIndices.find(Shader::getFrameConstantsShaderResourceName());
if (frameDataIndexIt != pPixelRootSigInfo->rootParameterIndices.end()) {
// Add root parameter.
addFrameDataParameter(frameDataIndexIt->second.second);
}
}

// Prepare indices of root parameters used by shaders.
auto shaderRootParameterIndices = vertexRootSigInfo.rootParameterIndices;
if (pPixelRootSigInfo != nullptr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ namespace ne {
* to query indices of root parameters of various non-user specified resources.
*/
enum class SpecialRootParameterSlot : unsigned int {
FRAME_DATA = 0,
GENERAL_LIGHTING,
GENERAL_LIGHTING = 0,
POINT_LIGHTS,
DIRECTIONAL_LIGHTS,
SPOT_LIGHTS,
Expand Down
Loading

0 comments on commit ef74f25

Please sign in to comment.