Skip to content

Conversation

@Happypig375
Copy link
Collaborator

@Happypig375 Happypig375 commented Dec 24, 2025

image

Added vector path rendering.

Also added -

  • CircleDispatcher
  • RectangleDispatcher
  • SpiralDispatcher
    with adjustable stroke and fill parameters.

Assets not propagated yet, awaiting review.

@bryanedds
Copy link
Owner

bryanedds commented Dec 25, 2025

Super cool stuff! Dean will be taking a look at this!

Perhaps this also frees me up on the OpenGL side to look at supporting vector graphics without having to worry about the Vulkan side needing additional work to match.

@deanjl
Copy link
Collaborator

deanjl commented Dec 27, 2025

Very exciting seeing someone else use this for the first time! The Vulkan code on the whole is spot on except for 2 issues with the buffer use.

The first issue is here:
let uniformBufferSize = sizeof<single> * 16 * Constants.Render.SpriteBatchSize
The SpriteBatchSize is of no relevance here. What you're specifying is the size of an actual VkBuffer for a given drawing instance, which is just the uniform (4 * 16). All the SpriteBatchSize is doing is making that buffer unnecessarily huge. The multiple instances represented by drawIndex use completely separate VkBuffers which are created as needed when you call Buffer.upload with a given index. You don't need to do anything for that. Think of these buffer layers as an implementation detail and just treat it as a normal buffer that needs an index. So just put sizeof<single> * 16.

The second issue is that you CAN'T destroy the vertex/index buffers at the end of the frame. This is completely invalid because the commands using the buffers haven't been executed yet. They're not submitted until the end of the frame, and the execution itself will take much longer because all the 3d shit is executed first! The only reason it's working at all is that the implementation is taking the liberty to defer the actual data destruction. This should also be triggering a validation error (though not with certainty), so make sure you've installed the Vulkan SDK.

Fortunately, you don't need to worry about buffer disposal either. The Staged buffers you're using take more time to create so are only intended for long-term data like the text-quad. Creating new vertex data on the fly works the same way as uniforms. Just do this (I just tweaked it so be sure to pull):

// create the buffers at init; size doesn't particularly matter here (see below) just guess the likely maximum
let vertexBuffer = Buffer.Buffer.create size (Buffer.Vertex true) vkc
let indexBuffer = Buffer.Buffer.create size (Buffer.Index true) vkc

// upload the data in frame; as with the buffer count, this will create a bigger VkBuffer if necessary
Buffer.Buffer.uploadArray drawIndex 0 vertices vertexBuffer vkc
Buffer.Buffer.uploadArray drawIndex 0 indices indexBuffer vkc

// here's the really cool part: you then bind the buffers like this
let mutable vertexBuf = vertexBuffer.[drawIndex]
...
Vulkan.vkCmdBindIndexBuffer (cb, indexBuffer.[drawIndex]...

// destroy at shutdown as usual
Buffer.Buffer.destroy vertexBuffer vkc
Buffer.Buffer.destroy indexBuffer vkc

Other than that, she's ready to merge as far as the Vulkan goes, great work!

@Happypig375
Copy link
Collaborator Author

The reviewed changes have been implemented.

@Happypig375
Copy link
Collaborator Author

Updated.

@Happypig375 Happypig375 requested review from bryanedds and deanjl and removed request for bryanedds February 9, 2026 15:13
@deanjl
Copy link
Collaborator

deanjl commented Feb 9, 2026

Good work once again. Just three minor changes needed:

  1. The vertex shader still uses the old and very bad uniform pattern with the alphabet naming. Just copy the relevant code from current SpriteBatch.vert and make sure all the bindings and other code blocks are in the correct order.
  2. We group uniform data together with pipelines but not vertex data, so the vertex and index buffers should be stored separately below TextQuad, and of course passed separately to drawVectorPath.
  3. Block comments need to be lower case, entirely except for type names etc.. This is obviously a general issue but affects the vulkan code.

@Happypig375
Copy link
Collaborator Author

@deanjl I implemented the suggested changes.

@deanjl
Copy link
Collaborator

deanjl commented Feb 10, 2026

The vert shader still doesn't conform to the right pattern. Just paste this in:

struct VectorPath
{
    mat4 modelViewProjection;
};

layout(push_constant) uniform PushConstant
{
    int drawId;
};

layout(binding = 0) uniform VectorPathBlock
{
    VectorPath vectorPath;
} vectorPath[];

...

mat4 modelViewProjection = vectorPath[drawId].vectorPath.modelViewProjection;

Vulkan uniforms suck.

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.

3 participants