Skip to content

Conversation

gpx1000
Copy link
Contributor

@gpx1000 gpx1000 commented Jul 5, 2025

This is specific to the issue #73

  • Catch vk::SystemError in mainLoop and presentation logic to handle vk::Result::eErrorOutOfDateKHR gracefully.
  • Log and ignore swapchain-related errors during shutdown or presentation to prevent crashes while rethrowing unexpected exceptions.
  • Updated multiple attachments for consistent error handling.

We likely will need to add better error detection everywhere; but in the spirit of just handling it when we get an out of date. It should be properly handled already in the draw function; but this makes it so we don't just throw an exception. I guess a better solution might be to handle the vk::Result::eErrorOutOfDateKHR by sending it to resize multiple times rather than the once. However, I think this is the right solution for now for this.

…tation functions

- Catch `vk::SystemError` in `mainLoop` and presentation logic to handle `vk::Result::eErrorOutOfDateKHR` gracefully.
- Log and ignore swapchain-related errors during shutdown or presentation to prevent crashes while rethrowing unexpected exceptions.
- Updated multiple attachments for consistent error handling.
@gpx1000 gpx1000 linked an issue Jul 5, 2025 that may be closed by this pull request
Copy link
Collaborator

@SaschaWillems SaschaWillems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change now results in an infinite loop for me when trying to resize the application:

image

That message is spammed endlessly, the application no longer reacts.

# Conflicts:
#	attachments/17_swap_chain_recreation.cpp
#	attachments/18_vertex_input.cpp
#	attachments/19_vertex_buffer.cpp
#	attachments/20_staging_buffer.cpp
#	attachments/26_texture_mapping.cpp
#	attachments/27_depth_buffering.cpp
#	attachments/30_multisampling.cpp
#	attachments/31_compute_shader.cpp
@gpx1000
Copy link
Contributor Author

gpx1000 commented Aug 21, 2025

I just tried to fix 17. @SaschaWillems, do me a favor and try that swapchain resize chapter code in windows and see if it fixes resize. I'll bring it to all of the other files if it solves it.

@SaschaWillems
Copy link
Collaborator

No change:

WindowsTerminal_cdnpxr5dCh.mp4

I don't see proper resize handling in that chapter (or any other chapter).

… `ErrorOutOfDateKHR` with swapchain recreation.
@gpx1000
Copy link
Contributor Author

gpx1000 commented Aug 22, 2025

I think there's a difference in our systems. Debugging in Linux, I'm seeing it correctly return the result of outofdate and does the recreate swapchain thing. However, you're getting spam, which means that it is throwing that error. Thus the "right" thing to do I think is to call the recreate swapchain in the catch block instead of just printing out to console. I mean that makes sense to me that it would rethrow each frame instead of sending out the error message which is admittedly what I'm used to.
Sorry about asking you to try again, but I think I might have this fixed again, we're only trying out chapter 17 until you say we're good.

@SaschaWillems
Copy link
Collaborator

The latest version of chapter 17 does work indeed. No more issues, so feel free to apply the change to all chapters 👍🏻

@gpx1000
Copy link
Contributor Author

gpx1000 commented Aug 23, 2025

Okay, I think Resizing is hopefully working for all.

Copy link
Collaborator

@SaschaWillems SaschaWillems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for all updated chapters. But chapters 33 and 34 are missing. both crash for me when trying to resize. Chapter 37 doesn't allow resizing (due to how the window is set up).

- Add `framebufferResizeCallback` to monitor window resize events.
- Implement `recreateSwapChain` to handle swapchain rebuild during resize scenarios.
- Adjust `drawFrame` logic to manage `ErrorOutOfDateKHR` and `SuboptimalKHR` results robustly.
@gpx1000
Copy link
Contributor Author

gpx1000 commented Aug 24, 2025

Okay, all of the chapters should handle resizing correctly.

Copy link

@cubanismo cubanismo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments on the WSI specifics inline.

glfwWaitEvents();
}
}
#endif
// Wait for device to finish operations
device.waitIdle();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note per recent spec updates, waiting for idle on the queue used for presentation is sufficient.

Also, it would be best if VK_EXT_swapchain_maintenance1's present fence were used here instead.

Is there a tutorial yet for smooth resize? I.e., using oldSwapchain to create the new swapchain and deferring cleanup until pending swaps have reached a point where it's safe to delete the old sawpchain(s)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we should open 3 issues you presented here:
1.) remove not needed device.waitIdle();
2.) use swapchain_maintenance1
3.) request to create a smooth resize tutorial.

The first two will require making changes to the tutorial itself as we don't want to just change the code without making sure the tutorial explains what needs to be done to be correct. The last will require a full tutorial which takes more time and planning; however, it should be done as we do see the request from time to time to give a sample on smooth resizing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to take into account that VK_EXT_swapchain_maintenance1 isn't universally supported, and the KHR variant even less so. So would prob. need to be optional for now.

@@ -1548,28 +1582,29 @@ class HelloTriangleApplication {
};
queue.submit(submitInfo, *inFlightFences[currentFrame]);

const vk::PresentInfoKHR presentInfoKHR{
.waitSemaphoreCount = 1,
.pWaitSemaphores = &*presentCompleteSemaphore[imageIndex],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this example still have renderFinishedSemaphores, but also have these presentCompleteSemaphore array that seems to server the same purpose, but with a slightly misleading name (It seems to be used more as a "rendering complete/present-ready" indicator, rather than a "present complete" indicator, the latter of which I'm not sure is possible to measure using semaphores in current Vulkan, which is what lead me to look closely at this code).

The Android file below seems to still use renderFinishedSemaphores in the manner that fits its name for what appears to be the same steps renderFinishedSemaphores is used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'opps' is the why :) I made a bug. Thanks for the report, I'll address it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resizing no longer working
3 participants