search

ConfettiFX / The-Forge

The Forge Cross-Platform Rendering Framework PC Windows, Steamdeck (native), Ray Tracing, macOS / iOS, Android, XBOX, PS4, PS5, Switch, Quest 2
Apache License 2.0
4.65k stars 491 forks source link

Vulkan.cpp: Put most frequently changed params first seems incorrect #230

Closed yaoyao-cn closed 2 years ago

yaoyao-cn commented 2 years ago

here, the comment says Put most frequently changed params first, but it seems that the code just do a reverse iteration

https://github.com/ConfettiFX/The-Forge/blob/e882fbc000b2915b52c98fe3a8c791930490dd3c/Common_3/Renderer/Vulkan/Vulkan.cpp#L5833

maybe i am wrong

here is the dx12 implement:

https://github.com/ConfettiFX/The-Forge/blob/e882fbc000b2915b52c98fe3a8c791930490dd3c/Common_3/Renderer/Direct3D12/Direct3D12.cpp?_pjax=%23js-repo-pjax-container%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%2C%20%5Bdata-pjax-container%5D#L4154

manas-kulkarni commented 2 years ago

DESCRIPTOR_UPDATE_FREQ_PER_DRAW is at index 3. It will move from PER_DRAW->PER_BATCH->PER_FRAME->STATIC

yaoyao-cn commented 2 years ago

If I understand this correctly, the PER_DRAW (set = 3) shoud at index 0 of VkPipelineLayoutCreateInfo.pSetLayouts and PER_BATCH-->VkPipelineLayoutCreateInfo.pSetLayouts[1] PER_BATCH->VkPipelineLayoutCreateInfo.pSetLayouts[2] STATIC->VkPipelineLayoutCreateInfo.pSetLayouts[3] here https://github.com/ConfettiFX/The-Forge/blob/e882fbc000b2915b52c98fe3a8c791930490dd3c/Common_3/Renderer/Vulkan/Vulkan.cpp#L5904 the ith descriptorSetLayouts is put into VkPipelineLayoutCreateInfo.pSetLayouts[i]

Michael-Lfx commented 2 years ago

Thank you for the issue. I have the same question.

manas-kulkarni commented 2 years ago

Yes. I remember now. It is the opposite in VK. On DX12, it is recommended to place the most updated descriptors in front, and on VK, it is recommended to place the least updated descriptors in front. Good catch. We will fix that comment in VK

manas-kulkarni commented 2 years ago

DX12 - https://gpuopen.com/wp-content/uploads/2016/03/GDC_2016_D3D12_Right_On_Queue_final.pdf VK - https://renderdoc.org/vkspec_chunked/chap14.html