Edit tutorial to reset & record command buffers each frame #255
Conversation
|
Very nice! I'll be able to start reviewing this PR sometime this week. |
|
It has come to my attention that there may be a sync bug W.R.T the depth buffer. Gist is that the same depth buffer could be read/written to by multiple frames in flight. A much more detailed description & solution is presented in the vk-guide.dev repo. |
|
@charles-lunarg Thanks for taking your time with this. I'm a beginner myself and I was pondering these exact concepts the other day. |
en/03_Drawing_a_triangle/03_Drawing/02_Rendering_and_presentation.md
Outdated
Show resolved
Hide resolved
|
Nice review! |
|
@Krenodeno Would you be interested in incorporating these changes in the French translation as well? Otherwise we'll have to add a disclaimer that some parts of the translation are out of date. |
|
Sure, I'll do it. Not sure how much time it will take though, I'll do my best :) |
|
I addressed all the changes requested by Overv, cleaning up some paragraphs, and fixing resizing to not deadlock. These include:
|
|
@Overv How can I edit the translation ? Forking the fork will not display change here (or does it ?), or Charles will have to merge to be displayed here. |
There are still going to be a few changes to this PR (possibly somewhat significant), so I'd wait a bit with the translation work. I think that once this PR is finished it would be good to get the changes in the English version out as quickly as possible and then you can work on the translation in your own pull request just like with earlier changes. |
Okay, that's good for me ! |
Means the code now calls recordCommandBuffer every frame instead of ahead of time Many changes to the structure of chapters 14 & 15, with small changes to subsequent chapters. Primary focus was shifting everything away from 'swapchain image count' to MAX_FRAMES_IN_FLIGHT. This has caused a lot of chapter 15 to need to be rewritten. Such as: Introducing fences alongside semaphores and not later; Waiting on a fence at the start of the frame before introducing acquireNextImage; consolidating the Frames In Flight concepts to apply to command buffers, fences, and semaphores at the same time. Chapter 14 saw command buffer allocation reduced to 1 at a time. This allows the concept of Frames in Flight to not need introduction before having a triangle drawing on the screen.
Greatly improve the descriptions of semaphores and fences before their introduction into the code. Provide examples with psuedo code. By using max_frames_in_flight command buffers & semaphores, we dont need to keep track of previously sumitted frames' fences and wait on them "just in case". This removes a lot of the confusion I had when I first was trying to understand the vulkan update loop.
Un-comment vkDeviceWaitIdle and remove resizing of imagesInFlight (which was removed)
* Add description of fences needing explicit resetting while semaphores are automatic * Elaborate on why 2 frames in flight are chosen
Because acquiring the swapchain image index may cause drawFrame to return early, it was possible to cause the next vkWaitForFences to deadlock. By delaying fence resetting till after acquiring, it prevents the deadlock.
charles-lunarg
force-pushed
the
rerecord_command_buffers
branch
from
2c85b77 to
4bd8019
Compare
|
FYI There is nothing more to do in this PR as far as I can tell. |
|
Totally happy with these changes. They make the tutorial as a whole much better. Would love to se this merged and the website updated :) |
|
Excellent work! |
|
@Overv Is this automatically deployed to the live tutorial? |
|
@ralphtheninja Yeah, the website is automatically redeployed once a day. |
|
@Krenodeno Would you have time to work on the translation soon? :) |
|
I'll try to look at it this week. |
* Update tutorial for command buffer re-recording Means the code now calls recordCommandBuffer every frame instead of ahead of time Many changes to the structure of chapters 14 & 15, with small changes to subsequent chapters. Primary focus was shifting everything away from 'swapchain image count' to MAX_FRAMES_IN_FLIGHT. This has caused a lot of chapter 15 to need to be rewritten. Such as: Introducing fences alongside semaphores and not later; Waiting on a fence at the start of the frame before introducing acquireNextImage; consolidating the Frames In Flight concepts to apply to command buffers, fences, and semaphores at the same time. Chapter 14 saw command buffer allocation reduced to 1 at a time. This allows the concept of Frames in Flight to not need introduction before having a triangle drawing on the screen. * Update introduction to semaphores and fences Greatly improve the descriptions of semaphores and fences before their introduction into the code. Provide examples with psuedo code. By using max_frames_in_flight command buffers & semaphores, we dont need to keep track of previously sumitted frames' fences and wait on them "just in case". This removes a lot of the confusion I had when I first was trying to understand the vulkan update loop. * Remove accidental code changes Un-comment vkDeviceWaitIdle and remove resizing of imagesInFlight (which was removed) * Use uint32_t instead of size_t for currentFrame * Address typos and fixup changes for re-recording command buffers PR * Add description of fences needing explicit resetting while semaphores are automatic * Elaborate on why 2 frames in flight are chosen * Fix deadlock in resizing from resetting the fence too early Because acquiring the swapchain image index may cause drawFrame to return early, it was possible to cause the next vkWaitForFences to deadlock. By delaying fence resetting till after acquiring, it prevents the deadlock.
This is a large change to the tutorial which makes the tutorial re-record command buffers. Originally described in #202 (and this PR fixes #202 )
Changes:
VkBufferrecreation from swapchain resizingimagesInFlight, since it is no longer necessary.