Skip to content

New debug draw extension for AABBs #900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ option(NBL_FAST_MATH "Enable fast low-precision math" ON)
option(NBL_BUILD_EXAMPLES "Enable building examples" ON)
option(NBL_BUILD_MITSUBA_LOADER "Enable nbl::ext::MitsubaLoader?" OFF) # TODO: once it compies turn this ON by default!
option(NBL_BUILD_IMGUI "Enable nbl::ext::ImGui?" ON)
option(NBL_BUILD_DEBUG_DRAW "Enable Nabla Debug Draw extension?" OFF)

option(NBL_BUILD_OPTIX "Enable nbl::ext::OptiX?" OFF)
if(NBL_COMPILE_WITH_CUDA)
Expand Down
2 changes: 2 additions & 0 deletions include/nbl/config/BuildConfigOptions.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@

#cmakedefine _NBL_BUILD_DPL_

#cmakedefine NBL_BUILD_DEBUG_DRAW

// TODO: This has to disapppear from the main header and go to the OptiX extension header + config
#cmakedefine OPTIX_INCLUDE_DIR "@OPTIX_INCLUDE_DIR@"

Expand Down
86 changes: 86 additions & 0 deletions include/nbl/ext/DebugDraw/CDrawAABB.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Copyright (C) 2018-2025 - DevSH Graphics Programming Sp. z O.O.
// This file is part of the "Nabla Engine".
// For conditions of distribution and use, see copyright notice in nabla.h

#ifndef _NBL_EXT_DRAW_AABB_H_
#define _NBL_EXT_DRAW_AABB_H_

#include "nbl/video/declarations.h"
#include "nbl/builtin/hlsl/cpp_compat.hlsl"
#include "nbl/builtin/hlsl/shapes/aabb.hlsl"
#include "nbl/ext/DebugDraw/builtin/hlsl/common.hlsl"

namespace nbl::ext::debugdraw

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either DebugDraw or debug_draw

{
class DrawAABB final : public core::IReferenceCounted
{
public:
struct SCachedCreationParameters
Comment on lines +17 to +18

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more indent please

{
using streaming_buffer_t = video::StreamingTransientDataBufferST<core::allocator<uint8_t>>;

static constexpr inline auto RequiredAllocateFlags = core::bitflag<video::IDeviceMemoryAllocation::E_MEMORY_ALLOCATE_FLAGS>(video::IDeviceMemoryAllocation::EMAF_DEVICE_ADDRESS_BIT);
static constexpr inline auto RequiredUsageFlags = core::bitflag(asset::IBuffer::EUF_STORAGE_BUFFER_BIT) | asset::IBuffer::EUF_SHADER_DEVICE_ADDRESS_BIT;

core::smart_refctd_ptr<video::IUtilities> utilities;

//! optional, default MDI buffer allocated if not provided
core::smart_refctd_ptr<streaming_buffer_t> streamingBuffer = nullptr;
};

struct SCreationParameters : SCachedCreationParameters
{
core::smart_refctd_ptr<asset::IAssetManager> assetManager = nullptr;

core::smart_refctd_ptr<video::IGPUPipelineLayout> pipelineLayout;
core::smart_refctd_ptr<video::IGPURenderpass> renderpass = nullptr;
};

// creates an instance that can draw one AABB via push constant or multiple using streaming buffer
static core::smart_refctd_ptr<DrawAABB> create(SCreationParameters&& params);

// creates default pipeline layout for push constant version
static core::smart_refctd_ptr<video::IGPUPipelineLayout> createDefaultPipelineLayout(video::ILogicalDevice* device, const asset::SPushConstantRange& pcRange);

// creates default pipeline layout for streaming version
static core::smart_refctd_ptr<video::IGPUPipelineLayout> createDefaultPipelineLayout(video::ILogicalDevice* device);

static core::smart_refctd_ptr<video::IGPUGraphicsPipeline> createDefaultPipeline(video::ILogicalDevice* device, video::IGPUPipelineLayout* layout, video::IGPURenderpass* renderpass, video::IGPUGraphicsPipeline::SShaderSpecInfo& vertex, video::IGPUGraphicsPipeline::SShaderSpecInfo& fragment);

//! mounts the extension's archive to given system - useful if you want to create your own shaders with common header included
static const core::smart_refctd_ptr<system::IFileArchive> mount(core::smart_refctd_ptr<system::ILogger> logger, system::ISystem* system, const std::string_view archiveAlias = "");

inline const SCachedCreationParameters& getCreationParameters() const { return m_cachedCreationParams; }

// records draw command for single AABB, user has to set pipeline outside
bool renderSingle(video::IGPUCommandBuffer* commandBuffer);

bool render(video::IGPUCommandBuffer* commandBuffer, video::ISemaphore::SWaitInfo waitInfo, float* cameraMat3x4);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take the camera MVP as const hlsl::float32_t4x4&


static std::array<hlsl::float32_t3, 24> getVerticesFromAABB(const core::aabbox3d<float>& aabb);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you need it for?


void addAABB(const core::aabbox3d<float>& aabb, const hlsl::float32_t4& color = { 1,0,0,1 });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't provide legacy API with legacy types

void addAABB(const hlsl::shapes::AABB<3,float>& aabb, const hlsl::float32_t4& color = { 1,0,0,1 });

void addOBB(const hlsl::shapes::AABB<3, float>& aabb, const hlsl::float32_t3x4 transform, const hlsl::float32_t4& color = { 1,0,0,1 });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the transform shall be the OBB, adding an AABB into the mix is confusing


void clearAABBs();

protected:
DrawAABB(SCreationParameters&& _params, core::smart_refctd_ptr<video::IGPUGraphicsPipeline> pipeline);
~DrawAABB() override;

private:
static core::smart_refctd_ptr<video::IGPUGraphicsPipeline> createPipeline(SCreationParameters& params);
static bool createStreamingBuffer(SCreationParameters& params);

std::vector<debugdraw::InstanceData> m_instances;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

immediate mode for GUI is ok, but I'd like to avoid it here, get rid of add and make render take a span<const InstanceData>

std::array<hlsl::float32_t3, 24> m_unitAABBVertices;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indexed line list takes 8 verts


SCachedCreationParameters m_cachedCreationParams;

core::smart_refctd_ptr<video::IGPUGraphicsPipeline> m_pipeline;
};
}

#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#pragma shader_stage(fragment)

#include "common.hlsl"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nbl/ext/DebugDraw/builtin/hlsl/common.hlsl is preferred


using namespace nbl::ext::debugdraw;

[shader("pixel")]
float32_t4 main(PSInput input) : SV_TARGET
{
float32_t4 outColor = input.color;

return outColor;
}
30 changes: 30 additions & 0 deletions include/nbl/ext/DebugDraw/builtin/hlsl/aabb_instances.vertex.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#pragma shader_stage(vertex)

#include "nbl/builtin/hlsl/glsl_compat/core.hlsl"
#include "nbl/builtin/hlsl/bda/__ptr.hlsl"
#include "common.hlsl"

using namespace nbl::hlsl;
using namespace nbl::ext::debugdraw;

[[vk::push_constant]] SPushConstants pc;

[shader("vertex")]
PSInput main()
{
PSInput output;

float32_t3 vertex = (bda::__ptr<float32_t3>::create(pc.pVertexBuffer) + glsl::gl_VertexIndex()).deref_restrict().load();
InstanceData instance = vk::RawBufferLoad<InstanceData>(pc.pInstanceBuffer + sizeof(InstanceData) * glsl::gl_InstanceIndex());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to use vk::BufferPointer instead of RawBufferLoad


float32_t4x4 transform;
transform[0] = instance.transform[0];
transform[1] = instance.transform[1];
transform[2] = instance.transform[2];
transform[3] = float32_t4(0, 0, 0, 1);
float32_t4 position = mul(transform, float32_t4(vertex, 1));
Comment on lines +20 to +25

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a util in math::linalg to carry out a fast multiple like that, see how I use it in my simpleDebugRenderer's shaders in Examples-Tests sumbouldes' common folder

output.position = mul(pc.MVP, position);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the instance data is emphemeral/transient (uploaded just before the draw) can you not pre-multiply the camera MVP with the instance transform on the CPU ?

Such that each instance stores combined MVP*transform ?

output.color = instance.color;

return output;
}
45 changes: 45 additions & 0 deletions include/nbl/ext/DebugDraw/builtin/hlsl/common.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#ifndef _DRAW_AABB_COMMON_HLSL
#define _DRAW_AABB_COMMON_HLSL
Comment on lines +1 to +2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full header guard, with all namespaces prefixed


#include "nbl/builtin/hlsl/cpp_compat.hlsl"

namespace nbl
{
namespace ext
{
namespace debugdraw
{

struct InstanceData
{
#ifdef __HLSL_VERSION
float32_t3x4 transform;
#else
float transform[3*4];
#endif
Comment on lines +15 to +19

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cpp_compat gives you float32_t3x4 in C++ too

nbl::hlsl::float32_t4 color;
};

struct SPushConstants
{
#ifdef __HLSL_VERSION
float32_t4x4 MVP;
#else
float MVP[4*4];
#endif
Comment on lines +25 to +29

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again

uint64_t pVertexBuffer;
uint64_t pInstanceBuffer;
};

#ifdef __HLSL_VERSION
struct PSInput
{
float32_t4 position : SV_Position;
float32_t4 color : TEXCOORD0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you using HW attributes for color? the color is per-instance

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah these are Vx-Px inter-stage shenanigans, I'll allow, make sure you label color flat though

};
#endif

}
}
}
#endif
12 changes: 12 additions & 0 deletions src/nbl/ext/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,16 @@ if(NBL_BUILD_TEXT_RENDERING)
add_subdirectory(TextRendering)
endif()

if(NBL_BUILD_DEBUG_DRAW)
add_subdirectory(DebugDraw)
set(NBL_EXT_DEBUG_DRAW_INCLUDE_DIRS
${NBL_EXT_DEBUG_DRAW_INCLUDE_DIRS}
PARENT_SCOPE
)
set(NBL_EXT_DEBUG_DRAW_LIB
${NBL_EXT_DEBUG_DRAW_LIB}
PARENT_SCOPE
)
endif()

propagate_changed_variables_to_parent_scope()
Loading
Loading