SecondHalfGames / yakui

yakui is a declarative Rust UI library for games
Apache License 2.0
230 stars 21 forks source link

Fix Vulkan crash with TextureChange::Modified #155

Closed patowen closed 5 months ago

patowen commented 5 months ago

The TextureChange::Removed and TextureChange::Modified match arms in YakuiVulkan::update_textures both have logic to remove a texture, but the logic is inconsistent between the two arms. This PR changes the TextureChange::Modified branch to be consistent with TextureChange::Removed.

This change also fixes a crash I experienced when trying to change UI text based on in-game information on a game that has a graphics pipeline depth (simultaneous frames in flight) of 2. The following Vulkan validation error appeared during this crash: Validation Error: [ VUID-vkDestroyImage-image-01000 ] | MessageID = 0xf2d29b5a | vkDestroyImage(): can't be called on VkImage 0xe3343f000000028f[] that is currently in use by VkCommandBuffer 0x20919128700[frame]. The Vulkan spec states: All submitted commands that refer to image, either directly or via a VkImageView, must have completed execution (https://vulkan.lunarg.com/doc/view/1.3.280.0/windows/1.3-extensions/vkspec.html#VUID-vkDestroyImage-image-01000) id=VUID-vkDestroyImage-image-01000 number=-221078694 queue_labels= cmd_labels= objects= Setting a breakpoint on the validation error did point me to the TextureChange::Modified match arm.

I am still relatively inexperienced in Vulkan, so I'm not certain this fix is correct nor would I be confident in setting up some kind of test case to be able to reproduce the issue in this repo itself. For extra context, I encountered this issue in https://github.com/Ralith/hypermine/pull/391.

kanerogers commented 5 months ago

Thanks for another fantastic PR - well documented and great explanations.

@Ralith I'll defer sync stuff to you as it's not my strong suit, but happy to take a look myself if you're time poor.

patowen commented 5 months ago

Thanks for the kind words!