BeRo1985 / pasvulkan

Vulkan header generator, OOP-style API wrapper, framework and prospective Vulkan-based game engine for Object Pascal
zlib License
185 stars 28 forks source link

Some issues with Framework TpvVulkanRenderPass Object Procedure #24

Closed johnhutch111 closed 2 years ago

johnhutch111 commented 2 years ago

The Procedure TpvVulkanRenderPass.AddSubpassDescription has some inconsistencies..

There is a mix of TpvInt32 and TpvUInt32 in the procedures data.. see below.

                                               const aInputAttachments:array of TpvInt32;  
                                               const aColorAttachments:array of TpvInt32;
                                               const aResolveAttachments:array of TpvInt32;
                                               const aDepthStencilAttachment:TpvInt32;
                                               const aPreserveAttachments:array of TpvUInt32):TpvUInt32;

Shouldn't they all be TpvUInt32 or at least all the same??

Also..

You can only define a single ResolveAttachment in the Vulkan SubPassDescription record. So shouldn't this be a Single TpvInt32/TpvUInt32 not an Array??

BeRo1985 commented 2 years ago

Nope, the ResolveAttachment part is right in the current way, see: https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VkSubpassDescription.html

pResolveAttachments is NULL or a pointer to an array of colorAttachmentCount VkAttachmentReference structures defining the resolve attachments for this subpass and their layouts.

And the frame graph stuff in PasVulkan.FrameGraph.pas is also able to use it. Vulkan is able to render in multiple render targets at the same time, when the HW does support it, including resolving.

And Int32 instead UInt32 is here because of array indexing to avoid compiler warnings on some situations, as far as I remember.

johnhutch111 commented 2 years ago

BeRo1985, Thanks for the clarification. The reference I was using implied only NULL or one Resolve attachment as there was no count.

I still think all parameters should be UInt32 but I can work with the current definition.