Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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 include/nbl/asset/IAsset.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class IAsset : virtual public core::IReferenceCounted
ET_PIPELINE_CACHE = 1ull<<21, //!< asset::ICPUPipelineCache
ET_SCENE = 1ull<<22, //!< reserved, to implement later
ET_RAYTRACING_PIPELINE = 1ull << 23, //!< asset::ICPURayTracingPipeline
ET_MESH_PIPELINE = 1ull << 24,
ET_IMPLEMENTATION_SPECIFIC_METADATA = 1ull<<31u, //!< lights, etc.
//! Reserved special value used for things like terminating lists of this enum

Expand Down
145 changes: 145 additions & 0 deletions include/nbl/asset/ICPUMeshPipeline.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
#ifndef _NBL_I_CPU_MESH_PIPELINE_H_INCLUDED_
#define _NBL_I_CPU_MESH_PIPELINE_H_INCLUDED_


#include "nbl/asset/IMeshPipeline.h"
#include "nbl/asset/ICPURenderpass.h"
#include "nbl/asset/ICPUPipeline.h"


namespace nbl::asset
{

class ICPUMeshPipeline final : public ICPUPipeline<IMeshPipeline<ICPUPipelineLayout,ICPURenderpass>>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make a ICPURasterizationPipeline : IRasterizationPipeline<ICPUPipelineLayout,ICPUrenderpass>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I tried IRasterization earlier.
I took another look. I don't use polymorphism too much, so maybe there's an easy answer staring me in the face. Most of the issue is that IGPURasterization is an incomplete type unless it's specialized with IMesh or IGraphics, at which point it's not an effective abstraction.
image
Here is perhaps the simplest solution.
image

Another potential solution I have is removing IGraphics and IMesh and replacing with IRasterization. However, that introduces a new problem, the construction parameters would be above the IGPU/ICPU abstraction level, unless some virtual functions are used.
image

If none of these solutions are acceptable, my recommendation would be a larger rewrite of the Pipeline system as whole. I have ideas here, but I'll hold until further direction is given.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i bookmarked this and will look into it as soon as #1000 is merged.

If none of these solutions are acceptable, my recommendation would be a larger rewrite of the Pipeline system as whole. I have ideas here, but I'll hold until further direction is given.

I actually had the plan to rewrite the Pipeline stuff and introduce PipelineLibraries and binaries and deprecate Pipeline Caches, but I'm out of Nabla improvement time/budget and it would disturb some other workflows,

Most of the issue is that IGPURasterization is an incomplete type unless it's specialized with IMesh or IGraphics, at which point it's not an effective abstraction.

that would be fine.

I'll take a look first thing after PR 1000 and decide on one of your prosals.

{
using pipeline_base_t = IMeshPipeline<ICPUPipelineLayout, ICPURenderpass>;
using base_t = ICPUPipeline<pipeline_base_t>;

public:

static core::smart_refctd_ptr<ICPUMeshPipeline> create(ICPUPipelineLayout* layout, ICPURenderpass* renderpass = nullptr)
{
auto retval = new ICPUMeshPipeline(layout, renderpass);
return core::smart_refctd_ptr<ICPUMeshPipeline>(retval,core::dont_grab);
}

constexpr static inline auto AssetType = ET_MESH_PIPELINE;
inline E_TYPE getAssetType() const override { return AssetType; }

inline const SCachedCreationParams& getCachedCreationParams() const
{
return pipeline_base_t::getCachedCreationParams();
}

inline SCachedCreationParams& getCachedCreationParams()
{
assert(isMutable());
return m_params;
}
Comment on lines +29 to +38

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if the creation params are identical for Gfx and MEsh, should go in ICPURasterizationPipeline

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

99168b9#diff-45681900933e70545ee1a41c9ad2419f9cd40ef479ad7afe82dc97f29095c36aR13.
If Programmable Vertex Pulling is enforced, VertexParams could be removed from GraphicsPipeline. With preprocessor branching, primitive type could potentially be moved out the shader and into the creation params, allowng the creation params to be the same for both rasterization pipelines.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I take that back, I only thought about outputtopology. Preprocessor branching the rest of the shader per primitive type is unreasonable.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can use composition ot inheritance to split the SCachedCreationParams.

we will support vertex inputs as long as we support graphics pipelines.


inline std::span<const SShaderSpecInfo> getSpecInfos(const hlsl::ShaderStage stage) const override final
{
switch (stage) {
case hlsl::ShaderStage::ESS_TASK: return { &m_specInfos[0], 1 };
case hlsl::ShaderStage::ESS_MESH: return { &m_specInfos[1], 1 };
case hlsl::ShaderStage::ESS_FRAGMENT: return { &m_specInfos[2], 1 };
}
return {};
}

inline std::span<SShaderSpecInfo> getSpecInfos(const hlsl::ShaderStage stage)
{
return base_t::getSpecInfos(stage);
}
Comment thread
devshgraphicsprogramming marked this conversation as resolved.

SShaderSpecInfo* getSpecInfo(const hlsl::ShaderStage stage)
{
if (!isMutable()) return nullptr;
switch (stage) {
case hlsl::ShaderStage::ESS_TASK: return &m_specInfos[0];
case hlsl::ShaderStage::ESS_MESH: return &m_specInfos[1];
case hlsl::ShaderStage::ESS_FRAGMENT: return &m_specInfos[2];
}
return nullptr;
}

const SShaderSpecInfo* getSpecInfo(const hlsl::ShaderStage stage) const
{
const auto stageIndex = stageToIndex(stage);
if (stageIndex != -1)
return &m_specInfos[stageIndex];
return nullptr;
}
Comment thread
GDBobby marked this conversation as resolved.
Outdated

inline bool valid() const override
{
if (!m_layout) return false;
if (!m_layout->valid())return false;

// https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkGraphicsPipelineCreateInfo.html#VUID-VkGraphicsPipelineCreateInfo-dynamicRendering-06576
if (!m_renderpass || m_params.subpassIx >= m_renderpass->getSubpassCount()) return false;

core::bitflag<hlsl::ShaderStage> stagePresence = {};
for (auto shader_i = 0u; shader_i < m_specInfos.size(); shader_i++)
{
const auto& info = m_specInfos[shader_i];
if (info.shader)
stagePresence |= indexToStage(shader_i);
}
return hasRequiredStages(stagePresence);
}

protected:
using base_t::base_t;
virtual ~ICPUMeshPipeline() override = default;

std::array<SShaderSpecInfo, MESH_SHADER_STAGE_COUNT> m_specInfos;

private:
explicit ICPUMeshPipeline(ICPUPipelineLayout* layout, ICPURenderpass* renderpass)
: base_t(layout, {}, renderpass)
{}

static inline int8_t stageToIndex(const hlsl::ShaderStage stage)
{
const auto stageIx = hlsl::findLSB(stage);
if (stageIx < 0 || stageIx >= MESH_SHADER_STAGE_COUNT || hlsl::bitCount(stage)!=1)
return -1;
return stageIx;
}
Comment thread
devshgraphicsprogramming marked this conversation as resolved.

static inline hlsl::ShaderStage indexToStage(const int8_t index)
{
switch (index) {
case 0: return hlsl::ShaderStage::ESS_TASK;
case 1: return hlsl::ShaderStage::ESS_MESH;
case 2: return hlsl::ShaderStage::ESS_FRAGMENT;
}
return hlsl::ShaderStage::ESS_UNKNOWN;
}

inline core::smart_refctd_ptr<base_t> clone_impl(core::smart_refctd_ptr<ICPUPipelineLayout>&& layout, uint32_t depth) const override final
{
auto* newPipeline = new ICPUMeshPipeline(layout.get(), m_renderpass.get());
newPipeline->m_params = m_params;

for (auto specInfo_i = 0u; specInfo_i < m_specInfos.size(); specInfo_i++)
{
newPipeline->m_specInfos[specInfo_i] = m_specInfos[specInfo_i].clone(depth);
}

return core::smart_refctd_ptr<base_t>(newPipeline, core::dont_grab);
}

inline void visitDependents_impl(std::function<bool(const IAsset*)> visit) const override
{
if (!visit(m_layout.get())) return;
if (!visit(m_renderpass.get())) return;
for (const auto& info : m_specInfos)
if (!visit(info.shader.get())) return;
}
};

}

#endif
3 changes: 3 additions & 0 deletions include/nbl/builtin/hlsl/indirect_commands.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ struct DispatchIndirectCommand_t
uint32_t num_groups_z;
};

// in vulkan this struct is distinct from DispatchIndirect, but has the same data - https://docs.vulkan.org/refpages/latest/refpages/source/VkDrawMeshTasksIndirectCommandEXT.html
using DrawMeshTasksIndirectCommand_t = DispatchIndirectCommand_t;

struct TraceRaysIndirectCommand_t
{
uint64_t raygenShaderRecordAddress;
Expand Down
24 changes: 18 additions & 6 deletions include/nbl/video/IGPUCommandBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,10 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject
bool copyAccelerationStructureFromMemory(const AccelerationStructure::DeviceCopyFromMemoryInfo& copyInfo);

//! state setup
bool bindComputePipeline(const IGPUComputePipeline* const pipeline);
bool bindGraphicsPipeline(const IGPUGraphicsPipeline* const pipeline);
bool bindComputePipeline(const IGPUComputePipeline* const pipeline);
bool bindMeshPipeline(const IGPUMeshPipeline* const pipeline);

bool bindRayTracingPipeline(const IGPURayTracingPipeline* const pipeline);
bool bindDescriptorSets(
const asset::E_PIPELINE_BIND_POINT pipelineBindPoint, const IGPUPipelineLayout* const layout,
Expand Down Expand Up @@ -442,6 +444,12 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject
}
bool dispatchIndirect(const asset::SBufferBinding<const IGPUBuffer>& binding);

bool drawMeshTasks(const uint32_t groupCountX, const uint32_t groupCountY = 1, const uint32_t groupCountZ = 1);
inline bool drawMeshTasks(const hlsl::vector<uint16_t, 3> groupCount) {
return drawMeshTasks(groupCount.x, groupCount.y, groupCount.z);
}
Comment thread
GDBobby marked this conversation as resolved.
Outdated
bool drawMeshTasksIndirect(const asset::SBufferBinding<const IGPUBuffer>& binding, const uint32_t drawCount, uint32_t stride);

//! Begin/End RenderPasses
struct SRenderpassBeginInfo
{
Expand Down Expand Up @@ -585,7 +593,7 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject
virtual const void* getNativeHandle() const = 0;

inline const core::unordered_map<const IGPUDescriptorSet*, uint64_t>& getBoundDescriptorSetsRecord() const { return m_boundDescriptorSetsRecord; }
const IGPUGraphicsPipeline* getBoundGraphicsPipeline() const { return m_boundGraphicsPipeline; }
const IGPUPipelineBase* getBoundGraphicsPipeline() const { return m_boundRasterizationPipeline; }
Comment on lines -588 to +597

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we have a RasterizationPipelineBase, cause PipelineBase is also inherited from by RT and compute

const IGPUComputePipeline* getBoundComputePipeline() const { return m_boundComputePipeline; }
const IGPURayTracingPipeline* getBoundRayTracingPipeline() const { return m_boundRayTracingPipeline; }

Expand Down Expand Up @@ -670,8 +678,9 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject
virtual bool copyAccelerationStructureToMemory_impl(const IGPUAccelerationStructure* src, const asset::SBufferBinding<IGPUBuffer>& dst) = 0;
virtual bool copyAccelerationStructureFromMemory_impl(const asset::SBufferBinding<const IGPUBuffer>& src, IGPUAccelerationStructure* dst) = 0;

virtual bool bindComputePipeline_impl(const IGPUComputePipeline* const pipeline) = 0;
virtual bool bindGraphicsPipeline_impl(const IGPUGraphicsPipeline* const pipeline) = 0;
virtual bool bindComputePipeline_impl(const IGPUComputePipeline* const pipeline) = 0;
virtual bool bindMeshPipeline_impl(const IGPUMeshPipeline* const pipeline) = 0;
virtual bool bindRayTracingPipeline_impl(const IGPURayTracingPipeline* const pipeline) = 0;
virtual bool bindDescriptorSets_impl(
const asset::E_PIPELINE_BIND_POINT pipelineBindPoint, const IGPUPipelineLayout* const layout,
Expand Down Expand Up @@ -715,6 +724,9 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject
virtual bool drawIndirectCount_impl(const asset::SBufferBinding<const IGPUBuffer>& indirectBinding, const asset::SBufferBinding<const IGPUBuffer>& countBinding, const uint32_t maxDrawCount, const uint32_t stride) = 0;
virtual bool drawIndexedIndirectCount_impl(const asset::SBufferBinding<const IGPUBuffer>& indirectBinding, const asset::SBufferBinding<const IGPUBuffer>& countBinding, const uint32_t maxDrawCount, const uint32_t stride) = 0;

virtual bool drawMeshTasks_impl(const uint32_t groupCountX, const uint32_t groupCountY, const uint32_t groupCountZ) = 0;
virtual bool drawMeshTasksIndirect_impl(const asset::SBufferBinding<const IGPUBuffer>& binding, const uint32_t drawCount, const uint32_t stride) = 0;

virtual bool blitImage_impl(const IGPUImage* const srcImage, const IGPUImage::LAYOUT srcImageLayout, IGPUImage* const dstImage, const IGPUImage::LAYOUT dstImageLayout, const std::span<const SImageBlit> regions, const IGPUSampler::E_TEXTURE_FILTER filter) = 0;
virtual bool resolveImage_impl(const IGPUImage* const srcImage, const IGPUImage::LAYOUT srcImageLayout, IGPUImage* const dstImage, const IGPUImage::LAYOUT dstImageLayout, const uint32_t regionCount, const SImageResolve* pRegions) = 0;

Expand Down Expand Up @@ -750,7 +762,7 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject

m_boundDescriptorSetsRecord.clear();
m_TLASTrackingOps.clear();
m_boundGraphicsPipeline= nullptr;
m_boundRasterizationPipeline= nullptr;
m_boundComputePipeline= nullptr;
m_boundRayTracingPipeline= nullptr;
m_haveRtPipelineStackSize = false;
Expand All @@ -768,7 +780,7 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject
deleteCommandList();
m_boundDescriptorSetsRecord.clear();
m_TLASTrackingOps.clear();
m_boundGraphicsPipeline= nullptr;
m_boundRasterizationPipeline= nullptr;
m_boundComputePipeline= nullptr;
m_boundRayTracingPipeline= nullptr;
m_haveRtPipelineStackSize = false;
Expand Down Expand Up @@ -929,7 +941,7 @@ class NBL_API2 IGPUCommandBuffer : public IBackendObject
// operations as they'll be performed in order
core::vector<std::variant<TLASTrackingWrite,TLASTrackingCopy,TLASTrackingRead>> m_TLASTrackingOps;

const IGPUGraphicsPipeline* m_boundGraphicsPipeline;
const IGPUPipelineBase* m_boundRasterizationPipeline;
const IGPUComputePipeline* m_boundComputePipeline;
const IGPURayTracingPipeline* m_boundRayTracingPipeline;

Expand Down
52 changes: 32 additions & 20 deletions include/nbl/video/IGPUCommandPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

#include "nbl/video/IEvent.h"
#include "nbl/video/IGPUDescriptorSet.h"
#include "nbl/video/IGPUComputePipeline.h"
#include "nbl/video/IGPUGraphicsPipeline.h"
#include "nbl/video/IGPUComputePipeline.h"
#include "nbl/video/IGPUMeshPipeline.h"
#include "nbl/video/IGPURayTracingPipeline.h"
#include "nbl/video/IGPUFramebuffer.h"
#include "nbl/video/IQueryPool.h"
Expand Down Expand Up @@ -125,14 +126,16 @@ class IGPUCommandPool : public IBackendObject
class CBeginRenderPassCmd;
class CPipelineBarrierCmd;
class CBindDescriptorSetsCmd;
class CBindComputePipelineCmd;
class CUpdateBufferCmd;
class CResetQueryPoolCmd;
class CWriteTimestampCmd;
class CBeginQueryCmd;
class CEndQueryCmd;
class CCopyQueryPoolResultsCmd;
class CBindGraphicsPipelineCmd;
class CBindComputePipelineCmd;
class CBindMeshPipelineCmd;
class CBindRayTracingPipelineCmd;
class CPushConstantsCmd;
class CBindVertexBuffersCmd;
class CCopyBufferCmd;
Expand All @@ -155,7 +158,6 @@ class IGPUCommandPool : public IBackendObject
class CCopyAccelerationStructureToOrFromMemoryCmd; // for both vkCmdCopyAccelerationStructureToMemoryKHR and vkCmdCopyMemoryToAccelerationStructureKHR
class CTraceRaysCmd;
class CTraceRaysIndirectCmd;
class CBindRayTracingPipelineCmd;

protected:
IGPUCommandPool(core::smart_refctd_ptr<const ILogicalDevice>&& dev, const core::bitflag<CREATE_FLAGS> _flags, const uint8_t _familyIx)
Expand Down Expand Up @@ -529,15 +531,6 @@ class IGPUCommandPool::CBindDescriptorSetsCmd final : public IFixedSizeCommand<C
core::smart_refctd_ptr<const IGPUDescriptorSet> m_sets[IGPUPipelineLayout::DESCRIPTOR_SET_COUNT];
};

class IGPUCommandPool::CBindComputePipelineCmd final : public IFixedSizeCommand<CBindComputePipelineCmd>
{
public:
CBindComputePipelineCmd(core::smart_refctd_ptr<const IGPUComputePipeline>&& pipeline) : m_pipeline(std::move(pipeline)) {}

private:
core::smart_refctd_ptr<const IGPUComputePipeline> m_pipeline;
};

class IGPUCommandPool::CUpdateBufferCmd final : public IFixedSizeCommand<CUpdateBufferCmd>
{
public:
Expand Down Expand Up @@ -604,6 +597,33 @@ class IGPUCommandPool::CBindGraphicsPipelineCmd final : public IFixedSizeCommand
core::smart_refctd_ptr<const IGPUGraphicsPipeline> m_pipeline;
};

class IGPUCommandPool::CBindComputePipelineCmd final : public IFixedSizeCommand<CBindComputePipelineCmd>
{
public:
CBindComputePipelineCmd(core::smart_refctd_ptr<const IGPUComputePipeline>&& pipeline) : m_pipeline(std::move(pipeline)) {}

private:
core::smart_refctd_ptr<const IGPUComputePipeline> m_pipeline;
};

class IGPUCommandPool::CBindMeshPipelineCmd final : public IFixedSizeCommand<CBindMeshPipelineCmd>
{
public:
CBindMeshPipelineCmd(core::smart_refctd_ptr<const IGPUMeshPipeline>&& pipeline) : m_pipeline(std::move(pipeline)) {}

private:
core::smart_refctd_ptr<const IGPUMeshPipeline> m_pipeline;
};

class IGPUCommandPool::CBindRayTracingPipelineCmd final : public IFixedSizeCommand<CBindRayTracingPipelineCmd>
{
public:
CBindRayTracingPipelineCmd(core::smart_refctd_ptr<const IGPURayTracingPipeline>&& pipeline) : m_pipeline(std::move(pipeline)) {}

private:
core::smart_refctd_ptr<const IGPURayTracingPipeline> m_pipeline;
};

Comment thread
devshgraphicsprogramming marked this conversation as resolved.
class IGPUCommandPool::CPushConstantsCmd final : public IFixedSizeCommand<CPushConstantsCmd>
{
public:
Expand Down Expand Up @@ -870,14 +890,6 @@ class IGPUCommandPool::CTraceRaysIndirectCmd final : public IFixedSizeCommand<CT
core::smart_refctd_ptr<const IGPUBuffer> m_bindingBuffer;
};

class IGPUCommandPool::CBindRayTracingPipelineCmd final : public IFixedSizeCommand<CBindRayTracingPipelineCmd>
{
public:
CBindRayTracingPipelineCmd(core::smart_refctd_ptr<const IGPURayTracingPipeline>&& pipeline) : m_pipeline(std::move(pipeline)) {}

private:
core::smart_refctd_ptr<const IGPURayTracingPipeline> m_pipeline;
};
NBL_ENUM_ADD_BITWISE_OPERATORS(IGPUCommandPool::CREATE_FLAGS)

}
Expand Down
Loading