Skip to content

Commit

Permalink
Fix an error that occurs while changing texture filtering.
Browse files Browse the repository at this point in the history
Also add a test for this.
  • Loading branch information
Flone-dnb committed Nov 23, 2024
1 parent ae0d9b3 commit f9fa35e
Show file tree
Hide file tree
Showing 22 changed files with 387 additions and 301 deletions.
15 changes: 9 additions & 6 deletions src/engine_lib/private/render/directx/DirectXRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,7 @@ namespace ne {
}

// Bind pipeline's descriptor tables.
for (const auto& [iRootParameterIndex, pDescriptorRange] : pipelineData.descriptorTablesToBind) {
for (const auto& [iRootParameterIndex, pDescriptorRange] : pipelineData.descriptorRangesToBind) {
pCommandList->SetGraphicsRootDescriptorTable(
iRootParameterIndex, pDescriptorRange->getGpuDescriptorHandleToRangeStart());
}
Expand Down Expand Up @@ -2012,19 +2012,21 @@ namespace ne {
}

if (vFilteredModes.empty()) {
Logger::get().info(std::format(
Logger::get().warn(std::format(
"video mode with resolution {}x{} and refresh rate "
"{}/{} is not supported, using default video mode",
preferredRenderResolution.first,
preferredRenderResolution.second,
preferredRefreshRate.first,
preferredRefreshRate.second));
// use last display mode

// Use the last (most suitable) display mode.
pickedDisplayMode = vVideoModes.back();
} else if (vFilteredModes.size() == 1) {
// found specified display mode
// Found the specified display mode.
pickedDisplayMode = vFilteredModes[0];
} else {
// Found multiple matches.
std::string sErrorMessage = std::format(
"video mode with resolution {}x{} and refresh rate "
"{}/{} matched multiple supported modes:\n",
Expand All @@ -2044,8 +2046,9 @@ namespace ne {
static_cast<int>(mode.ScanlineOrdering),
static_cast<int>(mode.Scaling));
}
Logger::get().info(std::format("{}\nusing default video mode", sErrorMessage));
// use the last display mode
Logger::get().warn(std::format("{}\nusing default video mode", sErrorMessage));

// Use the last (most suitable) display mode.
pickedDisplayMode = vVideoModes.back();
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace ne {
pResource = nullptr;

// Notify heap.
pHeap->onDescriptorBeingDestroyed(this, pRange);
pHeap->onDescriptorBeingDestroyed(this, pRange.get());
}

DirectXDescriptor::DirectXDescriptor(
Expand All @@ -22,7 +22,7 @@ namespace ne {
DirectXResource* pResource,
int iDescriptorOffsetInDescriptors,
std::optional<size_t> referencedCubemapFaceIndex,
ContinuousDirectXDescriptorRange* pRange)
const std::shared_ptr<ContinuousDirectXDescriptorRange>& pRange)
: pResource(pResource), pHeap(pHeap), pRange(pRange),
referencedCubemapFaceIndex(referencedCubemapFaceIndex), descriptorType(descriptorType) {
// Save index.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

// Standard.
#include <memory>
#include <optional>

// Custom.
Expand Down Expand Up @@ -39,14 +40,14 @@ namespace ne {
*
* @return Descriptor offset.
*/
inline int getDescriptorOffsetInDescriptors() const { return iDescriptorOffsetInDescriptors; }
int getDescriptorOffsetInDescriptors() const { return iDescriptorOffsetInDescriptors; }

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

/**
* Returns resource that owns this descriptor.
Expand All @@ -67,15 +68,15 @@ namespace ne {
* @param referencedCubemapFaceIndex Specify empty if this descriptor does not reference
* a cubemap, otherwise index of cubemap's face that it references.
* @param pRange Range that this descriptor was allocated from.
* `nullptr` if allocated as a single descriptor (not part of some range).
* `nullptr` if allocated not from a range.
*/
DirectXDescriptor(
DirectXDescriptorHeap* pHeap,
DirectXDescriptorType descriptorType,
DirectXResource* pResource,
int iDescriptorOffsetInDescriptors,
std::optional<size_t> referencedCubemapFaceIndex,
ContinuousDirectXDescriptorRange* pRange = nullptr);
const std::shared_ptr<ContinuousDirectXDescriptorRange>& pRange = nullptr);

private:
/**
Expand All @@ -90,11 +91,8 @@ namespace ne {
/** Do not delete. Heap of this descriptor. */
DirectXDescriptorHeap* const pHeap = nullptr;

/**
* Do not delete. Range that allocated this descriptor (`nullptr` if allocated as a single
* descriptor).
*/
ContinuousDirectXDescriptorRange* const pRange = nullptr;
/** Range that allocated this descriptor (`nullptr` if allocated not from a range). */
std::shared_ptr<ContinuousDirectXDescriptorRange> const pRange = nullptr;

/** Not empty if this descriptor references a cubemap's face. */
const std::optional<size_t> referencedCubemapFaceIndex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include "render/directx/DirectXRenderer.h"
#include "io/Logger.h"
#include "render/directx/resource/DirectXResource.h"
#include "render/RenderSettings.h"

namespace ne {
std::variant<std::unique_ptr<DirectXDescriptorHeap>, Error>
Expand All @@ -21,13 +20,13 @@ namespace ne {
return pManager;
}

std::variant<std::unique_ptr<ContinuousDirectXDescriptorRange>, Error>
std::variant<std::shared_ptr<ContinuousDirectXDescriptorRange>, Error>
DirectXDescriptorHeap::allocateContinuousDescriptorRange(
const std::string& sRangeName, const std::function<void()>& onRangeIndicesChanged) {
std::scoped_lock guard(mtxInternalData.first);

// Create a new range.
auto pRange = std::unique_ptr<ContinuousDirectXDescriptorRange>(
auto pRange = std::shared_ptr<ContinuousDirectXDescriptorRange>(
new ContinuousDirectXDescriptorRange(this, onRangeIndicesChanged, sRangeName));

// Add to the heap.
Expand All @@ -47,7 +46,7 @@ namespace ne {
std::optional<Error> DirectXDescriptorHeap::assignDescriptor(
DirectXResource* pResource,
DirectXDescriptorType descriptorType,
ContinuousDirectXDescriptorRange* pRange,
const std::shared_ptr<ContinuousDirectXDescriptorRange>& pRange,
bool bBindDescriptorsToCubemapFaces) {
// Check if this heap handles the specified descriptor type.
const auto vHandledDescriptorTypes = getDescriptorTypesHandledByThisHeap();
Expand Down Expand Up @@ -78,7 +77,7 @@ namespace ne {

if (pRange != nullptr) {
// Make sure the specified range exists.
const auto rangeIt = mtxInternalData.second.continuousDescriptorRanges.find(pRange);
const auto rangeIt = mtxInternalData.second.continuousDescriptorRanges.find(pRange.get());
if (rangeIt == mtxInternalData.second.continuousDescriptorRanges.end()) [[unlikely]] {
return Error(std::format(
"resource \"{}\" attempted to assign a descriptor in {} heap with "
Expand All @@ -101,7 +100,7 @@ namespace ne {

if (!optionalHeapIndex.has_value()) {
// Expand range.
auto optionalError = expandRange(pRange);
auto optionalError = expandRange(pRange.get());
if (optionalError.has_value()) [[unlikely]] {
optionalError->addCurrentLocationToErrorStack();
return optionalError;
Expand Down Expand Up @@ -862,16 +861,17 @@ namespace ne {
// Prepare log message.
auto sLogMessage = std::format(
"waiting for the GPU to finish work up to this point to (re)create {} descriptor heap from "
"capacity {} to {} (current actual heap size: {})",
"capacity {} to {} (current actual heap size: {}) (range count: {})",
sHeapType,
mtxInternalData.second.iHeapCapacity,
iCapacity,
mtxInternalData.second.iHeapSize);
mtxInternalData.second.iHeapSize,
mtxInternalData.second.continuousDescriptorRanges.size());

// Add range info (if specified).
if (pChangedRange != nullptr) {
sLogMessage +=
std::format(" due to changes in descriptor range \"{}\"", pChangedRange->sRangeName);
std::format(" due to changes in a descriptor range \"{}\"", pChangedRange->sRangeName);
}

// Log message.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,12 @@ namespace ne {
/** Descriptor heap. */
ComPtr<ID3D12DescriptorHeap> pHeap;

/** Descriptor ranges that were allocated in this heap. */
/**
* Descriptor ranges that were allocated in this heap.
*
* @remark It's safe to store raw pointers here because ranges will notify the heap in their
* destructor.
*/
std::unordered_set<ContinuousDirectXDescriptorRange*> continuousDescriptorRanges;

/** Current heap capacity. */
Expand Down Expand Up @@ -302,7 +307,7 @@ namespace ne {
*
* @return Error if something went wrong, otherwise allocated descriptor range.
*/
std::variant<std::unique_ptr<ContinuousDirectXDescriptorRange>, Error>
std::variant<std::shared_ptr<ContinuousDirectXDescriptorRange>, Error>
allocateContinuousDescriptorRange(
const std::string& sRangeName, const std::function<void()>& onRangeIndicesChanged);

Expand Down Expand Up @@ -338,14 +343,14 @@ namespace ne {
*
* @return Descriptor size.
*/
inline UINT getDescriptorSize() const { return iDescriptorSize; }
UINT getDescriptorSize() const { return iDescriptorSize; }

/**
* Returns internal DirectX heap.
*
* @return DirectX heap.
*/
inline ID3D12DescriptorHeap* getInternalHeap() const { return mtxInternalData.second.pHeap.Get(); }
ID3D12DescriptorHeap* getInternalHeap() const { return mtxInternalData.second.pHeap.Get(); }

/**
* Returns internal data of the object.
Expand Down Expand Up @@ -485,7 +490,9 @@ namespace ne {
*
* @param pResource Resource to point new descriptor to.
* @param descriptorType Type of the new descriptor.
* @param pRange Specify in order to allocate a descriptor from this range.
* @param pRange Specify in order to allocate a descriptor from this range. The specified
* shared pointer will be copied and saved in the resource so that the range will not be destroyed
* while this resource references a descriptor from it.
* @param bBindDescriptorsToCubemapFaces If this resource is a cubemap, specify `true`
* to also bind descriptors that reference specific cubemap faces, specify `false` to only bind 1
* descriptor that references the entire resource.
Expand All @@ -495,7 +502,7 @@ namespace ne {
[[nodiscard]] std::optional<Error> assignDescriptor(
DirectXResource* pResource,
DirectXDescriptorType descriptorType,
ContinuousDirectXDescriptorRange* pRange = nullptr,
const std::shared_ptr<ContinuousDirectXDescriptorRange>& pRange = nullptr,
bool bBindDescriptorsToCubemapFaces = true);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ namespace ne {

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

#if defined(DEBUG)
static_assert(
Expand Down
6 changes: 3 additions & 3 deletions src/engine_lib/private/render/directx/pipeline/DirectXPso.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ namespace ne {
globalShaderResourceSrvs;

/**
* Stores pairs of "root parameter index" - "descriptor table (range) to bind".
* Stores pairs of "root parameter index" - "descriptor range to bind".
*
* @remark Shader resources modify this map.
*/
std::unordered_map<UINT, std::unique_ptr<ContinuousDirectXDescriptorRange>>
descriptorTablesToBind;
std::unordered_map<UINT, std::shared_ptr<ContinuousDirectXDescriptorRange>>
descriptorRangesToBind;

/** Whether fields of this struct are initialized or not. */
bool bIsReadyForUsage = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ namespace ne {

std::optional<Error> DirectXResource::bindDescriptor(
DirectXDescriptorType descriptorType,
ContinuousDirectXDescriptorRange* pRange,
const std::shared_ptr<ContinuousDirectXDescriptorRange>& pRange,
bool bBindDescriptorsToCubemapFaces) {
std::scoped_lock guard(mtxHeapDescriptors.first);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,12 @@ namespace ne {
/**
* Creates a new descriptor and binds it to this resource.
*
* @remark Does nothing if a descriptor of this type is already binded.
* @remark Does nothing if a descriptor of this type is already bound.
*
* @param descriptorType Type of descriptor to bind.
* @param pRange Specify in order to allocate a descriptor from this range.
* The specified shared pointer will be copied and saved in the resource so that the range will not be
* destroyed while this resource references a descriptor from it.
* @param bBindDescriptorsToCubemapFaces If this resource is a cubemap, specify `true`
* to also bind descriptors that reference specific cubemap faces, specify `false` to only bind 1
* descriptor that references the entire resource.
Expand All @@ -60,7 +62,7 @@ namespace ne {
*/
[[nodiscard]] std::optional<Error> bindDescriptor(
DirectXDescriptorType descriptorType,
ContinuousDirectXDescriptorRange* pRange = nullptr,
const std::shared_ptr<ContinuousDirectXDescriptorRange>& pRange = nullptr,
bool bBindDescriptorsToCubemapFaces = true);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace ne {
error.addCurrentLocationToErrorStack();
return error;
}
auto pSrvRange = std::get<std::unique_ptr<ContinuousDirectXDescriptorRange>>(std::move(rangeResult));
auto pSrvRange = std::get<std::shared_ptr<ContinuousDirectXDescriptorRange>>(std::move(rangeResult));

// Save SRV range.
pIndexManager->pSrvRange = std::move(pSrvRange);
Expand Down Expand Up @@ -119,7 +119,7 @@ namespace ne {
}

// Bind SRV from the range.
optionalError = pSrvResource->bindDescriptor(DirectXDescriptorType::SRV, pSrvRange.get(), false);
optionalError = pSrvResource->bindDescriptor(DirectXDescriptorType::SRV, pSrvRange, false);
if (optionalError.has_value()) [[unlikely]] {
optionalError->addCurrentLocationToErrorStack();
return optionalError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ namespace ne {
*
* @remark Always valid, never changing pointer.
*/
std::unique_ptr<ContinuousDirectXDescriptorRange> pSrvRange;
std::shared_ptr<ContinuousDirectXDescriptorRange> pSrvRange;

/** Info about shadows maps that take place in this array. */
std::pair<std::recursive_mutex, std::unordered_set<ShadowMapHandle*>> mtxRegisteredShadowMaps;
Expand Down
Loading

0 comments on commit f9fa35e

Please sign in to comment.