Skip to content

Commit

Permalink
Fix an error introduced in f9fa35e caused by Range refactoring.
Browse files Browse the repository at this point in the history
  • Loading branch information
Flone-dnb committed Nov 24, 2024
1 parent d9d53c3 commit b05e3c4
Show file tree
Hide file tree
Showing 9 changed files with 235 additions and 181 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,40 @@ namespace ne {
this->iDescriptorOffsetInDescriptors = iDescriptorOffsetInDescriptors;
}

std::variant<unsigned int, Error> DirectXDescriptor::getDescriptorOffsetFromRangeStart() {
// Make sure this descriptor was allocated from a range.
if (pRange == nullptr) [[unlikely]] {
return Error("expected the descriptor to be allocated from a range");
}

const int iDescriptorOffsetFromHeapStart = iDescriptorOffsetInDescriptors;
const int iRangeOffsetFromHeapStart = pRange->getRangeStartInHeap();

// Make sure range offset is valid.
if (iRangeOffsetFromHeapStart < 0) [[unlikely]] {
return Error(std::format(
"descriptor range \"{}\" has a negative start index in the heap", pRange->getRangeName()));
}

// Calculate offset from range start.
const int iDescriptorOffsetFromRangeStart =
iDescriptorOffsetFromHeapStart - iRangeOffsetFromHeapStart;

// Self check: make sure resulting value is non-negative.
if (iDescriptorOffsetFromRangeStart < 0) [[unlikely]] {
return Error(std::format(
"failed to calculate descriptor offset from descriptor range start (range: \"{}\") for "
"resource \"{}\" (descriptor offset from heap: {}, range offset from heap: {}) because "
"resulting descriptor offset from range start is negative: {})",
pRange->getRangeName(),
pResource->getResourceName(),
iDescriptorOffsetFromHeapStart,
iRangeOffsetFromHeapStart,
iDescriptorOffsetFromRangeStart));
}

return static_cast<unsigned int>(iDescriptorOffsetFromRangeStart);
}

DirectXResource* DirectXDescriptor::getOwnerResource() const { return pResource; }
} // namespace ne
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
// Standard.
#include <memory>
#include <optional>
#include <variant>

// Custom.
#include "DirectXDescriptorType.hpp"
#include "misc/Error.h"

namespace ne {
class DirectXDescriptorHeap;
Expand Down Expand Up @@ -35,24 +37,54 @@ namespace ne {
DirectXDescriptor& operator=(DirectXDescriptor&& other) noexcept = delete;

/**
* Returns offset of this descriptor from the heap start
* (offset is specified in descriptors, not an actual index).
* Returns offset of this descriptor from the heap start (offset is specified in descriptors, not an
* actual index).
*
* @warning Returned value is only valid until the descriptor heap has not resized, so it's only safe
* to call this function when you know that the descriptor heap will not resize. This function
* is generally used during rendering when we know that the descriptor heap will not resize.
*
* @warning You shouldn't store returned offset for more that 1 frame as it might change after a frame
* is submitted (because the descriptor heap may resize).
*
* @return Descriptor offset.
*/
int getDescriptorOffsetInDescriptors() const { return iDescriptorOffsetInDescriptors; }

/**
* Calculates an offset of the descriptor from the start of the range (see @ref getDescriptorRange)
* that this descriptor was allocated from.
*
* @warning Returned value is only valid until the descriptor heap has not resized, so it's only safe
* to call this function when you know that the descriptor heap will not resize. This function
* is generally used during rendering when we know that the descriptor heap will not resize.
*
* @warning You shouldn't store returned offset for more that 1 frame as it might change after a frame
* is submitted (because the descriptor heap may resize).
*
* @return Error if this descriptor was not allocated from a range or something went wrong, otherwise
* offset (in descriptors) of the descriptor from range start.
*/
std::variant<unsigned int, Error> getDescriptorOffsetFromRangeStart();

/**
* Returns heap that this descriptor uses.
*
* @return Descriptor heap.
*/
DirectXDescriptorHeap* getDescriptorHeap() const { return pHeap; }

/**
* Returns descriptor range that this descriptor was allocated from.
*
* @return `nullptr` if this descriptor was not allocated from a range, otherwise a valid pointer.
*/
std::shared_ptr<ContinuousDirectXDescriptorRange> getDescriptorRange() const { return pRange; }

/**
* Returns resource that owns this descriptor.
*
* @return Owner resource.
* @return `nullptr` if this descriptor is being destroyed, otherwise owner resource.
*/
DirectXResource* getOwnerResource() const;

Expand Down Expand Up @@ -85,7 +117,7 @@ namespace ne {
*/
int iDescriptorOffsetInDescriptors;

/** Do not delete. Owner resource of this descriptor. */
/** Do not delete. Always valid unless we are in the destructor. Owner resource of this descriptor. */
DirectXResource* pResource = nullptr;

/** Do not delete. Heap of this descriptor. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1044,49 +1044,6 @@ namespace ne {
const std::string& sRangeName)
: onRangeIndicesChanged(onRangeIndicesChanged), sRangeName(sRangeName), pHeap(pHeap) {}

std::variant<unsigned int, Error>
ContinuousDirectXDescriptorRange::getResourceDescriptorOffsetFromRangeStart(
DirectXResource* pResource, DirectXDescriptorType descriptorType) {
// Get descriptor.
const auto pDescriptor = pResource->getDescriptor(descriptorType);
if (pDescriptor == nullptr) [[unlikely]] {
return Error(std::format(
"expected the resource \"{}\" to have a descriptor already binded in the descriptor range "
"\"{}\"",
pResource->getResourceName(),
sRangeName));
}

std::scoped_lock guard(mtxInternalData.first);

const int iDescriptorOffsetFromHeapStart = pDescriptor->getDescriptorOffsetInDescriptors();
const int iRangeOffsetFromHeapStart = mtxInternalData.second.iRangeStartInHeap;

// Check our offset in the heap.
if (iRangeOffsetFromHeapStart < 0) [[unlikely]] {
return Error(
std::format("descriptor range \"{}\" has a negative start index in the heap", sRangeName));
}

// Calculate offset from range start.
const int iDescriptorOffsetFromRangeStart =
iDescriptorOffsetFromHeapStart - iRangeOffsetFromHeapStart;

// Self check: make sure resulting value is non-negative.
if (iDescriptorOffsetFromRangeStart < 0) [[unlikely]] {
return Error(std::format(
"descriptor range \"{}\" failed to calculate descriptor offset from range start (descriptor "
"offset from heap: {}, range offset from heap: {}) because resulting descriptor offset from "
"range start is negative: {})",
sRangeName,
iDescriptorOffsetFromHeapStart,
iRangeOffsetFromHeapStart,
iDescriptorOffsetFromRangeStart));
}

return static_cast<unsigned int>(iDescriptorOffsetFromRangeStart);
}

size_t ContinuousDirectXDescriptorRange::getRangeSize() {
std::scoped_lock guard(mtxInternalData.first);
return mtxInternalData.second.allocatedDescriptors.size();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,6 @@ namespace ne {
*/
static constexpr INT getRangeGrowSize() { return iRangeGrowSize; }

/**
* Calculates an offset of the descriptor (of the specified resource) from the start of this
* range.
*
* @param pResource Resource with already binded descriptor within this range.
* @param descriptorType Type of the descriptor to look for.
*
* @return Error if something went wrong, otherwise offset (in descriptors) of the descriptor from
* range start.
*/
std::variant<unsigned int, Error> getResourceDescriptorOffsetFromRangeStart(
DirectXResource* pResource, DirectXDescriptorType descriptorType);

/**
* Returns the number of active (currently in-use) descriptors that were allocated from this range.
*
Expand Down Expand Up @@ -111,6 +98,13 @@ namespace ne {
*/
D3D12_GPU_DESCRIPTOR_HANDLE getGpuDescriptorHandleToRangeStart() const;

/**
* Returns name of this range (used for logging).
*
* @return Name of the range.
*/
std::string getRangeName() const { return sRangeName; }

private:
/** Groups mutex guarded data. */
struct InternalData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ namespace ne {
*
* @return Internal resource.
*/
inline ID3D12Resource* getInternalResource() const { return pInternalResource; }
ID3D12Resource* getInternalResource() const { return pInternalResource; }

/**
* Returns a raw (non-owning) pointer to a binded descriptor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ namespace ne {
if (pSrvResource->getDescriptor(DirectXDescriptorType::SRV) != nullptr) [[unlikely]] {
return Error(std::format(
"\"{}\" was requested to register a shadow map handle \"{}\" but the GPU resource of this "
"shadow map handle already has an SRV binded to it which is unexpected",
"shadow map handle already has an SRV bound to it which is unexpected",
getShaderArrayResourceName(),
pSrvResource->getResourceName()));
}
Expand All @@ -125,15 +125,20 @@ namespace ne {
return optionalError;
}

// Get bound SRV.
const auto pSrvDescriptor = pSrvResource->getDescriptor(DirectXDescriptorType::SRV);
if (pSrvDescriptor == nullptr) [[unlikely]] {
return Error("expected an SRV to be bound");
}

// Get descriptor offset from range start.
auto descriptorOffsetResult = pSrvRange->getResourceDescriptorOffsetFromRangeStart(
reinterpret_cast<DirectXResource*>(pSrvResource), DirectXDescriptorType::SRV);
if (std::holds_alternative<Error>(descriptorOffsetResult)) [[unlikely]] {
auto error = std::get<Error>(std::move(descriptorOffsetResult));
auto result = pSrvDescriptor->getDescriptorOffsetFromRangeStart();
if (std::holds_alternative<Error>(result)) [[unlikely]] {
auto error = std::get<Error>(std::move(result));
error.addCurrentLocationToErrorStack();
return error;
}
const auto iDescriptorOffsetFromRangeStart = std::get<unsigned int>(descriptorOffsetResult);
const auto iDescriptorOffsetFromRangeStart = std::get<unsigned int>(result);

// Save resource to internal array.
mtxRegisteredShadowMaps.second.insert(pShadowMapHandle);
Expand Down Expand Up @@ -184,21 +189,36 @@ namespace ne {
const auto pMtxResources = pShadowMapHandle->getResources();

// Determine which resource to use to bind SRV.
auto pSrvResource = pMtxResources->second.pDepthTexture;
auto pResource = pMtxResources->second.pDepthTexture;
if (pMtxResources->second.pColorTexture != nullptr) {
pSrvResource = pMtxResources->second.pColorTexture;
pResource = pMtxResources->second.pColorTexture;
}

// Cast type.
const auto pSrvResource = dynamic_cast<DirectXResource*>(pResource);
if (pSrvResource == nullptr) [[unlikely]] {
Error error("expected a DirectX resource");
error.showError();
throw std::runtime_error(error.getFullErrorMessage());
}

// Get bound SRV.
const auto pSrvDescriptor = pSrvResource->getDescriptor(DirectXDescriptorType::SRV);
if (pSrvDescriptor == nullptr) [[unlikely]] {
Error error("expected an SRV to be bound");
error.showError();
throw std::runtime_error(error.getFullErrorMessage());
}

// Get descriptor offset from range start.
auto descriptorOffsetResult = pSrvRange->getResourceDescriptorOffsetFromRangeStart(
reinterpret_cast<DirectXResource*>(pSrvResource), DirectXDescriptorType::SRV);
if (std::holds_alternative<Error>(descriptorOffsetResult)) [[unlikely]] {
auto error = std::get<Error>(std::move(descriptorOffsetResult));
auto result = pSrvDescriptor->getDescriptorOffsetFromRangeStart();
if (std::holds_alternative<Error>(result)) [[unlikely]] {
auto error = std::get<Error>(std::move(result));
error.addCurrentLocationToErrorStack();
error.showError();
throw std::runtime_error(error.getFullErrorMessage());
}
const auto iDescriptorOffsetFromRangeStart = std::get<unsigned int>(descriptorOffsetResult);
const auto iDescriptorOffsetFromRangeStart = std::get<unsigned int>(result);

// Notify shadow map user about array index initialized.
changeShadowMapArrayIndex(pShadowMapHandle, iDescriptorOffsetFromRangeStart);
Expand Down
Loading

0 comments on commit b05e3c4

Please sign in to comment.