Skip to content

Conversation

@GDBobby
Copy link

@GDBobby GDBobby commented Dec 24, 2025

Mesh Pipelines

Description of changes that aren't implicit.

  1. Reordered every mention of Graphics, Compute, Mesh, and Raytracing pipelines to follow that order. For example, in line 331 of IGPUCommandBuffer, Compute was moved below Graphics, and mesh was put below compute, which is above raytracing.
  2. In ILogicalDevice, I abstracted the commonalities between Graphics and Mesh pipeline creation. The end result for Graphics doesn't change at all.
  3. I'll probably need help with the features and limits. I included the bare minimum.

Testing

https://github.com/GDBobby/Nabla-Examples-and-Tests/tree/master/MeshShader

TODO list:

I need to fix IGPUCommandBuffer::drawMeshTasksIndirect. I haven't done anything with Indirect commands yet so I'll have to figure it out as I go.

@devshgraphicsprogrammingjenkins
Copy link
Contributor

[CI]: Can one of the admins verify this patch?

@devshgraphicsprogramming
Copy link
Member

What a wonderful Christmas gift!

@devshgraphicsprogramming
Copy link
Member

@GDBobby the last person to add @kevyuu did the RT Pipeline PR, he can review after me

.gitmodules Outdated
[submodule "3rdparty/boost/superproject"]
path = 3rdparty/boost/superproject
url = ../boost.git
url = git@github.com:Devsh-Graphics-Programming/boost.git

Choose a reason for hiding this comment

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

as @AnastaZIuk will tell you, this needs to stay as it was, you need to for the submodules yourself as well.

Its important the submodules are relative.

Comment on lines 76 to 78
params.featuresToEnable = getRequiredDeviceFeatures().unionWith(supportedPreferredFormats);
params.featuresToEnable.meshShader = true;
params.featuresToEnable.taskShader = true;

Choose a reason for hiding this comment

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

you can't add this to all examples, override the function like the raytracing examples in the mesh shader example

Comment on lines 40 to 46
// distinct struct, new name with the same data - https://docs.vulkan.org/refpages/latest/refpages/source/VkDrawMeshTasksIndirectCommandEXT.html
struct DrawMeshTasksIndirectCommand_t
{
uint32_t num_groups_x;
uint32_t num_groups_y;
uint32_t num_groups_z;
};

Choose a reason for hiding this comment

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

just use DispatchIndirectCommand_t instead

Comment on lines 446 to 448
bool drawMeshTasks(const uint32_t groupCountX, const uint32_t groupCountY = 1, const uint32_t groupCountZ = 1);
template<typename T> requires std::is_integral_v<T>
bool drawMeshTasks(const hlsl::vector<T, 3> groupCount)

Choose a reason for hiding this comment

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

provide only one uint16_t3 overload

Comment on lines 588 to 599
const IGPUGraphicsPipeline* getBoundGraphicsPipeline() const { return m_boundGraphicsPipeline; }
const IGPUComputePipeline* getBoundComputePipeline() const { return m_boundComputePipeline; }
const IGPUMeshPipeline* getBoundMeshPipeline() const { return m_boundMeshPipeline; }

Choose a reason for hiding this comment

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

graphics and mesh bind to same bindpoint (unlike RT which has a separate bindpoint from Compute)

you may need to return a common base class (Ssee how its done for acceleration structures)

Comment on lines 932 to 950
const IGPUGraphicsPipeline* m_boundGraphicsPipeline;
const IGPUComputePipeline* m_boundComputePipeline;
const IGPUMeshPipeline* m_boundMeshPipeline;

Choose a reason for hiding this comment

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

graphics and mesh bind to same bindpoint (unlike RT which has a separate bindpoint from Compute)

you may need to store a common base class (Ssee how its done for acceleration structures - BLAS and TLAS the GPU types)

Copy link
Author

Choose a reason for hiding this comment

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

graphics and mesh bind to same bindpoint (unlike RT which has a separate bindpoint from Compute)

you may need to store a common base class (Ssee how its done for acceleration structures - BLAS and TLAS the GPU types)

Can you explain this further so that I know what kind of change should be made? In what situation are we expecting dynamic dispatch to help?

// we reference a pipeline layout and a renderpass
constexpr static inline bool HasChildren = true;
// the video type
using video_t = IGPUGraphicsPipeline;

Choose a reason for hiding this comment

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

typo ? should be IGPUMeshPipeline probably ?

Przemog1 and others added 18 commits December 26, 2025 13:36
…sent yet in the master branch

Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
keptsecret and others added 23 commits December 26, 2025 13:43
Signed-off-by: Corey <corey.w108@gmail.com>
…spv for extension

Signed-off-by: Corey <corey.w108@gmail.com>
…range

Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
… half each time

Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
…ight now. graphics is stable tho

Signed-off-by: Corey <corey.w108@gmail.com>
i PROBABLY messed up something in mesh. committing so I can keep track of changes while I test in the example

Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
…ight now. graphics is stable tho

Signed-off-by: Corey <corey.w108@gmail.com>
i PROBABLY messed up something in mesh. committing so I can keep track of changes while I test in the example

Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
Signed-off-by: Corey <corey.w108@gmail.com>
@GDBobby
Copy link
Author

GDBobby commented Dec 26, 2025

Sorry, I tried to do the signoff rebase, I can undo that.

The most bulky change will be the abstraction between mesh and the traditional graphics bindpoint which I'm awaiting further direction on, it should altogether still take less than an hour or so.

I guess I'm pushing the mesh pipeline change so I'll put my thoughts out.
The easiest change would be to return PipelineBase

 //former
 //const IGPUMeshPipeline* getBoundMeshPipeline() const { return m_boundMeshPipeline; }
 
 //replacement
const IPipelineBase* getBoundRasterizationPipeline() const{return m_boundRasterizationPipeline;}

the alternative would be to make a IRasterizationPipeline, like so

class IMeshPipeline : IRasterizationPipeline : IPipeline;
class IGraphicsPipeline : IRasterizationPipeline : IPipeline;

Graphics Pipeline Libraries are on my todo list within the month, so I'm not very familiar with them. But I've found zero mentions of mesh shaders and libraries within the same text. I'm not sure how the planned pipeline library refactor will change the hypothetical IRasterizationPipeline, which makes me a bit hesitant.

@devshgraphicsprogramming
Copy link
Member

Sorry, I tried to do the signoff rebase, I can undo that.

The most bulky change will be the abstraction between mesh and the traditional graphics bindpoint which I'm awaiting further direction on, it should altogether still take less than an hour or so.

I guess I'm pushing the mesh pipeline change so I'll put my thoughts out. The easiest change would be to return PipelineBase

 //former
 //const IGPUMeshPipeline* getBoundMeshPipeline() const { return m_boundMeshPipeline; }
 
 //replacement
const IPipelineBase* getBoundRasterizationPipeline() const{return m_boundRasterizationPipeline;}

the alternative would be to make a IRasterizationPipeline, like so

class IMeshPipeline : IRasterizationPipeline : IPipeline;
class IGraphicsPipeline : IRasterizationPipeline : IPipeline;

Graphics Pipeline Libraries are on my todo list within the month, so I'm not very familiar with them. But I've found zero mentions of mesh shaders and libraries within the same text. I'm not sure how the planned pipeline library refactor will change the hypothetical IRasterizationPipeline, which makes me a bit hesitant.

  1. don't force push
  2. if in doubt then merge

Now we have commits with two authors

As for your question, make a IRasterizationPipeline please

@GDBobby
Copy link
Author

GDBobby commented Jan 1, 2026

Okay, I had to do a fresh clean clone from the source because I deleted my fork locally for some reason I forgot, and cloning the fork doesn't work. (something about examples/media/ditt). But anyways, if you'd like I can reopen the PR on that fresh clone without the rebase, I have to copy over my changes from that anyways now.

Moving towards IRasterizationPipeline, Now that I've gotten a bit more familiar with the system, I believe what you wanted and asked for was IGPUPipelineBase
which would look like

const IGPUPipelineBase* getBoundGraphicsPipeline() const { return m_boundRasterizationPipeline; }

That's most similar to the abstraction from BLAS and TLAS.

I didn't like how IRasterizationPipeline turned out, but I figured I would share before deleting it. I tried moving around PipelineLayoutType and RenderpassType a bit but it didn't help.

I'll wrap this up tomorrow. Happy New Year!

@devshgraphicsprogramming
Copy link
Member

Okay, I had to do a fresh clean clone from the source because I deleted my fork locally for some reason I forgot, and cloning the fork doesn't work. (something about examples/media/ditt). But anyways, if you'd like I can reopen the PR on that fresh clone without the rebase, I have to copy over my changes from that anyways now.

Moving towards IRasterizationPipeline, Now that I've gotten a bit more familiar with the system, I believe what you wanted and asked for was IGPUPipelineBase which would look like

const IGPUPipelineBase* getBoundGraphicsPipeline() const { return m_boundRasterizationPipeline; }

That's most similar to the abstraction from BLAS and TLAS.

I didn't like how IRasterizationPipeline turned out, but I figured I would share before deleting it. I tried moving around PipelineLayoutType and RenderpassType a bit but it didn't help.

I'll wrap this up tomorrow. Happy New Year!

Happy New Year

ok do a diff, do it as one new commit from lastest master

We'll continue reviewing on that PR

One word of note, is that we have some important business things to tend to so review time is a bit limited (at least from me and Arek)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants