Overv / VulkanTutorial

Tutorial for the Vulkan graphics and compute API
https://vulkan-tutorial.com
Creative Commons Attribution Share Alike 4.0 International
3.06k stars 511 forks source link

Edit tutorial to reset & record command buffers each frame #255

Closed charles-lunarg closed 2 years ago

charles-lunarg commented 2 years ago

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:

Overv commented 2 years ago

Very nice! I'll be able to start reviewing this PR sometime this week.

charles-lunarg commented 2 years ago

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. https://github.com/vblanco20-1/vulkan-guide/pull/53

ralphtheninja commented 2 years ago

@charles-lunarg Thanks for taking your time with this. I'm a beginner myself and I was pondering these exact concepts the other day.

ralphtheninja commented 2 years ago

Nice review!

Overv commented 2 years ago

@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.

Krenodeno commented 2 years ago

Sure, I'll do it. Not sure how much time it will take though, I'll do my best :)

charles-lunarg commented 2 years ago

I addressed all the changes requested by Overv, cleaning up some paragraphs, and fixing resizing to not deadlock.

These include:

Krenodeno commented 2 years ago

@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.

Overv commented 2 years ago

@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.

Krenodeno commented 2 years ago

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 !

charles-lunarg commented 2 years ago

FYI There is nothing more to do in this PR as far as I can tell. I don't have any outstanding questions or concerns. Totally capable of doing further revisions if someone else wants to review.

SaschaWillems commented 2 years ago

Totally happy with these changes. They make the tutorial as a whole much better. Would love to se this merged and the website updated :)

Overv commented 2 years ago

Excellent work!

ralphtheninja commented 2 years ago

@Overv Is this automatically deployed to the live tutorial?

Overv commented 2 years ago

@ralphtheninja Yeah, the website is automatically redeployed once a day.

Overv commented 2 years ago

@Krenodeno Would you have time to work on the translation soon? :)

Krenodeno commented 2 years ago

I'll try to look at it this week. I think there are other changes made by other PRs that I'll have to update as well.