From b065463226d4b227be35b491bc1cbd8b30edd9bd Mon Sep 17 00:00:00 2001 From: StillHammer Date: Wed, 26 Nov 2025 11:50:05 +0800 Subject: [PATCH] fix(BgfxRenderer): Implement CommandBuffer execution and topological sort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add executeCommandBuffer() to IRHIDevice interface - Implement full command buffer execution in BgfxDevice (175 lines) - Handles SetState, SetTexture, SetUniform, SetScissor - Handles SetVertexBuffer, SetIndexBuffer, SetInstanceBuffer - Handles Draw, DrawIndexed, DrawInstanced, Submit - Rewrite RenderGraph::compile() with Kahn's algorithm for topological sort - Now respects pass dependencies correctly - Uses sortOrder as secondary key via priority queue - Detects cycles and throws exception - Call device.executeCommandBuffer() in RenderGraph::execute() - Make OpenSSL optional with std::hash fallback for SHA256 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- CMakeLists.txt | 19 +- modules/BgfxRenderer/RHI/BgfxDevice.cpp | 177 ++++++++++++++++++ modules/BgfxRenderer/RHI/RHIDevice.h | 3 + .../BgfxRenderer/RenderGraph/RenderGraph.cpp | 74 ++++++-- src/JsonDataNode.cpp | 18 +- 5 files changed, 273 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 862ec63..183376d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -112,8 +112,15 @@ add_library(GroveEngine::core ALIAS grove_core) option(GROVE_BUILD_IMPLEMENTATIONS "Build GroveEngine implementations" ON) if(GROVE_BUILD_IMPLEMENTATIONS) - # Find OpenSSL for SHA256 hashing - find_package(OpenSSL REQUIRED) + # Find OpenSSL for SHA256 hashing (optional on MinGW) + find_package(OpenSSL QUIET) + if(OpenSSL_FOUND) + message(STATUS "OpenSSL found - using hardware-accelerated SHA256") + set(GROVE_USE_OPENSSL ON) + else() + message(STATUS "OpenSSL not found - using fallback SHA256 implementation") + set(GROVE_USE_OPENSSL OFF) + endif() add_library(grove_impl STATIC # --- Working files (IDataNode-based) --- @@ -140,11 +147,17 @@ if(GROVE_BUILD_IMPLEMENTATIONS) GroveEngine::core topictree::topictree stillhammer_logger - OpenSSL::Crypto spdlog::spdlog ${CMAKE_DL_LIBS} ) + if(GROVE_USE_OPENSSL) + target_link_libraries(grove_impl PUBLIC OpenSSL::Crypto) + target_compile_definitions(grove_impl PUBLIC GROVE_USE_OPENSSL=1) + else() + target_compile_definitions(grove_impl PUBLIC GROVE_USE_OPENSSL=0) + endif() + # Enable position-independent code for static library (needed for .so modules) set_target_properties(grove_impl PROPERTIES POSITION_INDEPENDENT_CODE ON) diff --git a/modules/BgfxRenderer/RHI/BgfxDevice.cpp b/modules/BgfxRenderer/RHI/BgfxDevice.cpp index 8ef6df9..4e6e401 100644 --- a/modules/BgfxRenderer/RHI/BgfxDevice.cpp +++ b/modules/BgfxRenderer/RHI/BgfxDevice.cpp @@ -257,6 +257,183 @@ public: bgfx::frame(); } + void executeCommandBuffer(const RHICommandBuffer& cmdBuffer) override { + // Track current state for bgfx calls + RenderState currentState; + BufferHandle currentVB; + BufferHandle currentIB; + BufferHandle currentInstBuffer; + uint32_t instStart = 0; + uint32_t instCount = 0; + + for (const Command& cmd : cmdBuffer.getCommands()) { + switch (cmd.type) { + case CommandType::SetState: { + currentState = cmd.setState.state; + // Build bgfx state flags + uint64_t state = BGFX_STATE_WRITE_RGB | BGFX_STATE_WRITE_A; + + switch (currentState.blend) { + case BlendMode::Alpha: + state |= BGFX_STATE_BLEND_ALPHA; + break; + case BlendMode::Additive: + state |= BGFX_STATE_BLEND_ADD; + break; + case BlendMode::Multiply: + state |= BGFX_STATE_BLEND_MULTIPLY; + break; + case BlendMode::None: + default: + break; + } + + switch (currentState.cull) { + case CullMode::CW: + state |= BGFX_STATE_CULL_CW; + break; + case CullMode::CCW: + state |= BGFX_STATE_CULL_CCW; + break; + case CullMode::None: + default: + break; + } + + if (currentState.depthTest) { + state |= BGFX_STATE_DEPTH_TEST_LESS; + } + if (currentState.depthWrite) { + state |= BGFX_STATE_WRITE_Z; + } + + bgfx::setState(state); + break; + } + + case CommandType::SetTexture: { + bgfx::TextureHandle tex = { cmd.setTexture.texture.id }; + bgfx::UniformHandle sampler = { cmd.setTexture.sampler.id }; + bgfx::setTexture(cmd.setTexture.slot, sampler, tex); + break; + } + + case CommandType::SetUniform: { + bgfx::UniformHandle uniform = { cmd.setUniform.uniform.id }; + bgfx::setUniform(uniform, cmd.setUniform.data, cmd.setUniform.numVec4s); + break; + } + + case CommandType::SetVertexBuffer: { + currentVB = cmd.setVertexBuffer.buffer; + break; + } + + case CommandType::SetIndexBuffer: { + currentIB = cmd.setIndexBuffer.buffer; + break; + } + + case CommandType::SetInstanceBuffer: { + currentInstBuffer = cmd.setInstanceBuffer.buffer; + instStart = cmd.setInstanceBuffer.start; + instCount = cmd.setInstanceBuffer.count; + break; + } + + case CommandType::SetScissor: { + bgfx::setScissor(cmd.setScissor.x, cmd.setScissor.y, + cmd.setScissor.w, cmd.setScissor.h); + break; + } + + case CommandType::Draw: { + // Set vertex buffer before draw + if (currentVB.isValid()) { + bool isDynamic = (currentVB.id & 0x8000) != 0; + uint16_t idx = currentVB.id & 0x7FFF; + if (isDynamic) { + bgfx::DynamicVertexBufferHandle h = { idx }; + bgfx::setVertexBuffer(0, h, 0, cmd.draw.vertexCount); + } else { + bgfx::VertexBufferHandle h = { idx }; + bgfx::setVertexBuffer(0, h, cmd.draw.startVertex, cmd.draw.vertexCount); + } + } + break; + } + + case CommandType::DrawIndexed: { + // Set vertex and index buffers before draw + if (currentVB.isValid()) { + bool isDynamic = (currentVB.id & 0x8000) != 0; + uint16_t idx = currentVB.id & 0x7FFF; + if (isDynamic) { + bgfx::DynamicVertexBufferHandle h = { idx }; + bgfx::setVertexBuffer(0, h); + } else { + bgfx::VertexBufferHandle h = { idx }; + bgfx::setVertexBuffer(0, h); + } + } + if (currentIB.isValid()) { + bool isDynamic = (currentIB.id & 0x8000) != 0; + uint16_t idx = currentIB.id & 0x7FFF; + if (isDynamic) { + bgfx::DynamicIndexBufferHandle h = { idx }; + bgfx::setIndexBuffer(h, cmd.drawIndexed.startIndex, cmd.drawIndexed.indexCount); + } else { + bgfx::IndexBufferHandle h = { idx }; + bgfx::setIndexBuffer(h, cmd.drawIndexed.startIndex, cmd.drawIndexed.indexCount); + } + } + break; + } + + case CommandType::DrawInstanced: { + // Set vertex, index, and instance buffers + if (currentVB.isValid()) { + bool isDynamic = (currentVB.id & 0x8000) != 0; + uint16_t idx = currentVB.id & 0x7FFF; + if (isDynamic) { + bgfx::DynamicVertexBufferHandle h = { idx }; + bgfx::setVertexBuffer(0, h); + } else { + bgfx::VertexBufferHandle h = { idx }; + bgfx::setVertexBuffer(0, h); + } + } + if (currentIB.isValid()) { + bool isDynamic = (currentIB.id & 0x8000) != 0; + uint16_t idx = currentIB.id & 0x7FFF; + if (isDynamic) { + bgfx::DynamicIndexBufferHandle h = { idx }; + bgfx::setIndexBuffer(h, 0, cmd.drawInstanced.indexCount); + } else { + bgfx::IndexBufferHandle h = { idx }; + bgfx::setIndexBuffer(h, 0, cmd.drawInstanced.indexCount); + } + } + if (currentInstBuffer.isValid()) { + bool isDynamic = (currentInstBuffer.id & 0x8000) != 0; + uint16_t idx = currentInstBuffer.id & 0x7FFF; + if (isDynamic) { + bgfx::DynamicVertexBufferHandle h = { idx }; + bgfx::setInstanceDataBuffer(h, instStart, instCount); + } + } + break; + } + + case CommandType::Submit: { + bgfx::ProgramHandle program = { cmd.submit.shader.id }; + bgfx::submit(cmd.submit.view, program, cmd.submit.depth); + break; + } + } + } + } + private: uint16_t m_width = 0; uint16_t m_height = 0; diff --git a/modules/BgfxRenderer/RHI/RHIDevice.h b/modules/BgfxRenderer/RHI/RHIDevice.h index 3c2c86b..3821ff9 100644 --- a/modules/BgfxRenderer/RHI/RHIDevice.h +++ b/modules/BgfxRenderer/RHI/RHIDevice.h @@ -60,6 +60,9 @@ public: // Frame virtual void frame() = 0; + // Command buffer execution + virtual void executeCommandBuffer(const class RHICommandBuffer& cmdBuffer) = 0; + // Factory static std::unique_ptr create(); }; diff --git a/modules/BgfxRenderer/RenderGraph/RenderGraph.cpp b/modules/BgfxRenderer/RenderGraph/RenderGraph.cpp index a66cc1b..8062760 100644 --- a/modules/BgfxRenderer/RenderGraph/RenderGraph.cpp +++ b/modules/BgfxRenderer/RenderGraph/RenderGraph.cpp @@ -2,6 +2,8 @@ #include "../RHI/RHIDevice.h" #include #include +#include +#include #include namespace grove { @@ -20,25 +22,69 @@ void RenderGraph::setup(rhi::IRHIDevice& device) { void RenderGraph::compile() { if (m_compiled) return; + const size_t n = m_passes.size(); + if (n == 0) { + m_compiled = true; + return; + } + // Build name to index map std::unordered_map nameToIndex; - for (size_t i = 0; i < m_passes.size(); ++i) { + for (size_t i = 0; i < n; ++i) { nameToIndex[m_passes[i]->getName()] = i; } - // Create sorted indices based on sort order and dependencies - m_sortedIndices.clear(); - m_sortedIndices.reserve(m_passes.size()); + // Build adjacency list and in-degree count for topological sort + std::vector> adjacency(n); + std::vector inDegree(n, 0); - for (size_t i = 0; i < m_passes.size(); ++i) { - m_sortedIndices.push_back(i); + for (size_t i = 0; i < n; ++i) { + auto deps = m_passes[i]->getDependencies(); + for (const char* depName : deps) { + auto it = nameToIndex.find(depName); + if (it != nameToIndex.end()) { + size_t depIdx = it->second; + adjacency[depIdx].push_back(i); // depIdx -> i (dep must run before i) + inDegree[i]++; + } + } } - // Sort by sort order (topological sort would be more complete but this is simpler) - std::sort(m_sortedIndices.begin(), m_sortedIndices.end(), - [this](size_t a, size_t b) { - return m_passes[a]->getSortOrder() < m_passes[b]->getSortOrder(); - }); + // Kahn's algorithm for topological sort with sort order as secondary key + // Use a priority queue to respect sortOrder among nodes with same in-degree + auto compare = [this](size_t a, size_t b) { + return m_passes[a]->getSortOrder() > m_passes[b]->getSortOrder(); // min-heap + }; + std::priority_queue, decltype(compare)> readyQueue(compare); + + // Initialize with nodes that have no dependencies + for (size_t i = 0; i < n; ++i) { + if (inDegree[i] == 0) { + readyQueue.push(i); + } + } + + m_sortedIndices.clear(); + m_sortedIndices.reserve(n); + + while (!readyQueue.empty()) { + size_t current = readyQueue.top(); + readyQueue.pop(); + m_sortedIndices.push_back(current); + + // Decrease in-degree of dependents + for (size_t dependent : adjacency[current]) { + inDegree[dependent]--; + if (inDegree[dependent] == 0) { + readyQueue.push(dependent); + } + } + } + + // Check for cycles + if (m_sortedIndices.size() != n) { + throw std::runtime_error("RenderGraph: Cycle detected in pass dependencies!"); + } m_compiled = true; } @@ -51,13 +97,13 @@ void RenderGraph::execute(const FramePacket& frame, rhi::IRHIDevice& device) { // Single command buffer for single-threaded execution rhi::RHICommandBuffer cmdBuffer; - // Execute passes in order + // Execute passes in topologically sorted order for (size_t idx : m_sortedIndices) { m_passes[idx]->execute(frame, cmdBuffer); } - // TODO: Execute command buffer on device - // For now, passes directly call bgfx through the device + // Execute the recorded command buffer on the device + device.executeCommandBuffer(cmdBuffer); } void RenderGraph::shutdown(rhi::IRHIDevice& device) { diff --git a/src/JsonDataNode.cpp b/src/JsonDataNode.cpp index a490886..746e025 100644 --- a/src/JsonDataNode.cpp +++ b/src/JsonDataNode.cpp @@ -2,8 +2,15 @@ #include #include #include -#include #include +#include + +#if GROVE_USE_OPENSSL +#include +#else +// Simple fallback hash using std::hash - NOT cryptographically secure +// but sufficient for change detection in render graph +#endif namespace grove { @@ -271,6 +278,7 @@ void JsonDataNode::setBool(const std::string& name, bool value) { // ======================================== std::string JsonDataNode::computeHash(const std::string& input) const { +#if GROVE_USE_OPENSSL unsigned char hash[SHA256_DIGEST_LENGTH]; SHA256(reinterpret_cast(input.c_str()), input.length(), hash); @@ -279,6 +287,14 @@ std::string JsonDataNode::computeHash(const std::string& input) const { ss << std::hex << std::setw(2) << std::setfill('0') << static_cast(hash[i]); } return ss.str(); +#else + // Fallback: use std::hash (not cryptographic, but good enough for change detection) + std::hash hasher; + size_t hashVal = hasher(input); + std::stringstream ss; + ss << std::hex << std::setw(16) << std::setfill('0') << hashVal; + return ss.str(); +#endif } std::string JsonDataNode::getDataHash() {