Skip to content

Commit

Permalink
[DebugLayer] Report error when Execute() is called on unfinished comm…
Browse files Browse the repository at this point in the history
…and buffer.

Secondary command buffers can only be used in Execute() command when they have finished their encoding.
  • Loading branch information
LukasBanana committed Jun 9, 2024
1 parent b8121ec commit e626209
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 77 deletions.
2 changes: 1 addition & 1 deletion include/LLGL/CommandBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class LLGL_EXPORT CommandBuffer : public RenderSystemChild
/**
\brief Executes the specified secondary command buffer by inlining its commands into this command buffer.
\param[in] secondaryCommandBuffer Specifies the secondary command buffer which is meant to be inlined.
This command buffer must have been created with the CommandBufferFlags::Secondary flag.
This command buffer must have been created with the CommandBufferFlags::Secondary flag and its must also have finished encoding.
\remarks This function can only be used by primary command buffers, i.e. command buffers that have \e not been created with the flag CommandBufferFlags::Secondary.
\remarks Once this command buffer is submitted for execution to one or more primary command buffers,
it <b>must not</b> be updated unless all of such primary command buffers are also updated before their next submission to the command queue.
Expand Down
68 changes: 55 additions & 13 deletions sources/Renderer/DebugLayer/DbgCommandBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ DbgCommandBuffer::DbgCommandBuffer(
:
instance { commandBufferInstance },
desc { desc },
label { LLGL_DBG_LABEL(desc) },
debugger_ { debugger },
commonProfile_ { commonProfile },
features_ { caps.features },
Expand All @@ -85,6 +86,11 @@ DbgCommandBuffer::DbgCommandBuffer(
{
}

void DbgCommandBuffer::SetDebugName(const char* name)
{
DbgSetObjectName(*this, name);
}

/* ----- Encoding ----- */

void DbgCommandBuffer::Begin()
Expand All @@ -100,7 +106,10 @@ void DbgCommandBuffer::Begin()

/* Begin with command recording */
if (debugger_)
EnableRecording(true);
{
LLGL_DBG_SOURCE();
ValidateBeginOfRecording();
}

instance.Begin();

Expand All @@ -111,7 +120,10 @@ void DbgCommandBuffer::End()
{
/* End with command recording */
if (debugger_)
EnableRecording(false);
{
LLGL_DBG_SOURCE();
ValidateEndOfRecording();
}
instance.End();

/* Resolve timer query results for performance profiler */
Expand Down Expand Up @@ -147,6 +159,8 @@ void DbgCommandBuffer::Execute(CommandBuffer& secondaryCommandBuffer)
CommandBufferFlags::Secondary,
"LLGL::CommandBuffer"
);

ValidateCommandBufferForExecute(commandBufferDbg.states_, GetLabelOrDefault(commandBufferDbg.label, "LLGL::CommandBuffer"));
}

LLGL_DBG_COMMAND( "Execute", instance.Execute(commandBufferDbg.instance) );
Expand Down Expand Up @@ -1370,19 +1384,47 @@ void DbgCommandBuffer::ValidateSubmit()
* ======= Private: =======
*/

void DbgCommandBuffer::EnableRecording(bool enable)
void DbgCommandBuffer::ValidateBeginOfRecording()
{
if (debugger_)
{
if (states_.recording)
LLGL_DBG_ERROR(ErrorType::InvalidState, "cannot begin nested recording of command buffer");
states_.recording = true;
}
}

void DbgCommandBuffer::ValidateEndOfRecording()
{
if (debugger_)
{
if (enable == states_.recording)
if (!states_.recording)
LLGL_DBG_ERROR(ErrorType::InvalidState, "cannot end recording of command buffer while no recording is currently active");
states_.recording = false;
states_.finishedRecording = true;
}
}

void DbgCommandBuffer::ValidateCommandBufferForExecute(const States& cmdBufferStates, const char* cmdBufferName)
{
if (!cmdBufferStates.finishedRecording)
{
if (cmdBufferStates.recording)
{
LLGL_DBG_SOURCE();
if (enable)
LLGL_DBG_ERROR(ErrorType::InvalidState, "cannot begin nested recording of command buffer");
else
LLGL_DBG_ERROR(ErrorType::InvalidState, "cannot end recording of command buffer while no recording is currently active");
LLGL_DBG_ERROR(
ErrorType::InvalidState,
"cannot run Execute() on %s that is currently in recording mode; Begin() but no subsequent End() invocation",
cmdBufferName
);
}
else
{
LLGL_DBG_ERROR(
ErrorType::InvalidState,
"cannot run Execute() on %s that is not encoded yetl; no Begin()/End() invocations",
cmdBufferName
);
}
states_.recording = enable;
}
}

Expand Down Expand Up @@ -2240,7 +2282,7 @@ void DbgCommandBuffer::ValidateDynamicStates()

void DbgCommandBuffer::ValidateBindingTable()
{
auto ValidateBindingTableWithLayout = [this](const DbgPipelineState& pso, const Bindings::BindingTable& table, const PipelineLayoutDescriptor& layoutDesc)
auto ValidateBindingTableWithLayout = [this](const DbgPipelineState& pso, const BindingTable& table, const PipelineLayoutDescriptor& layoutDesc)
{
const std::string psoLabel = (!pso.label.empty() ? " \'" + pso.label + '\'' : "");
LLGL_ASSERT(table.resources.size() == layoutDesc.bindings.size());
Expand Down Expand Up @@ -2422,7 +2464,7 @@ void DbgCommandBuffer::ResetRecords()

void DbgCommandBuffer::ResetBindingTable(const DbgPipelineLayout* pipelineLayoutDbg)
{
auto ResetBindingTableWithLayout = [](Bindings::BindingTable& table, const PipelineLayoutDescriptor& layoutDesc)
auto ResetBindingTableWithLayout = [](BindingTable& table, const PipelineLayoutDescriptor& layoutDesc)
{
table.resourceHeap = nullptr;
table.resources.clear();
Expand All @@ -2431,7 +2473,7 @@ void DbgCommandBuffer::ResetBindingTable(const DbgPipelineLayout* pipelineLayout
table.uniforms.resize(layoutDesc.uniforms.size(), 0);
};

auto ResetBindingTableZero = [](Bindings::BindingTable& table)
auto ResetBindingTableZero = [](BindingTable& table)
{
table.resourceHeap = nullptr;
table.resources.clear();
Expand Down
133 changes: 70 additions & 63 deletions sources/Renderer/DebugLayer/DbgCommandBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ class DbgCommandBuffer final : public CommandBuffer

#include <LLGL/Backend/CommandBuffer.inl>

public:

void SetDebugName(const char* name) override;

public:

DbgCommandBuffer(
Expand All @@ -61,10 +65,69 @@ class DbgCommandBuffer final : public CommandBuffer

CommandBuffer& instance;
const CommandBufferDescriptor desc;
std::string label;

private:

struct BindingTable
{
ResourceHeap* resourceHeap = nullptr;
std::vector<Resource*> resources;
std::vector<char> uniforms;
};

struct Bindings
{
// Framebuffers
DbgSwapChain* swapChain = nullptr;
DbgRenderTarget* renderTarget = nullptr;
std::uint32_t numViewports = 0;

// Stream inputs/outputs
DbgBuffer* vertexBufferStore[1] = {};
DbgBuffer* const * vertexBuffers = nullptr;
std::uint32_t numVertexBuffers = 0;
bool anyShaderAttributes = false;
DbgBuffer* indexBuffer = nullptr;
std::uint64_t indexBufferFormatSize = 0;
std::uint64_t indexBufferOffset = 0;
DbgBuffer* streamOutputs[LLGL_MAX_NUM_SO_BUFFERS] = {};
std::uint32_t numStreamOutputs = 0;

// PSO
DbgPipelineState* pipelineState = nullptr;
const DbgShader* vertexShader = nullptr;
bool blendFactorSet = false;
bool stencilRefSet = false;
Scissor scissorRects[LLGL_MAX_NUM_VIEWPORTS_AND_SCISSORS];
std::uint32_t numScissorRects = 0;
BindingTable bindingTable;
};

struct States
{
bool recording = false;
bool finishedRecording = false;
bool insideRenderPass = false;
bool streamOutputBusy = false;
};

struct SwapChainFramePair
{
DbgSwapChain* swapChain;
std::uint64_t frame; // Frame index when the swap-chain render-pass was encoded
};

struct Records
{
std::vector<SwapChainFramePair> swapChainFrames;
};

private:

void EnableRecording(bool enable);
void ValidateBeginOfRecording();
void ValidateEndOfRecording();
void ValidateCommandBufferForExecute(const States& cmdBufferStates, const char* cmdBufferName = nullptr);

void ValidateGenerateMips(DbgTexture& textureDbg, const TextureSubresource* subresource = nullptr);
void ValidateViewport(const Viewport& viewport);
Expand Down Expand Up @@ -146,19 +209,11 @@ class DbgCommandBuffer final : public CommandBuffer

void SetAndValidateScissorRects(std::uint32_t numScissors, const Scissor* scissors);

private:

struct SwapChainFramePair
{
DbgSwapChain* swapChain;
std::uint64_t frame; // Frame index when the swap-chain render-pass was encoded
};

private:

/* ----- Common objects ----- */

RenderingDebugger* debugger_ = nullptr;
RenderingDebugger* debugger_ = nullptr;
FrameProfile& commonProfile_;

const RenderingFeatures& features_;
Expand All @@ -167,63 +222,15 @@ class DbgCommandBuffer final : public CommandBuffer
std::stack<std::string> debugGroups_;

DbgQueryTimerPool queryTimerPool_;
bool perfProfilerEnabled_ = false;
bool perfProfilerEnabled_ = false;

/* ----- Render states ----- */

FrameProfile profile_;

PrimitiveTopology topology_ = PrimitiveTopology::TriangleList;

struct Bindings
{
// Framebuffers
DbgSwapChain* swapChain = nullptr;
DbgRenderTarget* renderTarget = nullptr;
std::uint32_t numViewports = 0;

// Stream inputs/outputs
DbgBuffer* vertexBufferStore[1] = {};
DbgBuffer* const * vertexBuffers = nullptr;
std::uint32_t numVertexBuffers = 0;
bool anyShaderAttributes = false;
DbgBuffer* indexBuffer = nullptr;
std::uint64_t indexBufferFormatSize = 0;
std::uint64_t indexBufferOffset = 0;
DbgBuffer* streamOutputs[LLGL_MAX_NUM_SO_BUFFERS] = {};
std::uint32_t numStreamOutputs = 0;

// PSO
DbgPipelineState* pipelineState = nullptr;
const DbgShader* vertexShader = nullptr;
bool blendFactorSet = false;
bool stencilRefSet = false;
Scissor scissorRects[LLGL_MAX_NUM_VIEWPORTS_AND_SCISSORS];
std::uint32_t numScissorRects = 0;

struct BindingTable
{
ResourceHeap* resourceHeap = nullptr;
std::vector<Resource*> resources;
std::vector<char> uniforms;
}
bindingTable;
}
bindings_;

struct States
{
bool recording = false;
bool insideRenderPass = false;
bool streamOutputBusy = false;
}
states_;

struct Records
{
std::vector<SwapChainFramePair> swapChainFrames;
}
records_;
PrimitiveTopology topology_ = PrimitiveTopology::TriangleList;
Bindings bindings_;
States states_;
Records records_;

};

Expand Down

0 comments on commit e626209

Please sign in to comment.