Emit destroy_surfaces() consistently on all platforms
#4419
+41
−11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarijnS95
changed the title
destroy_surfaces() event consistently on all platformsdestroy_surfaces() consistently on all platforms
MarijnS95
force-pushed
the
consistent-destroy-surfaces
branch
2 times, most recently
from
4948b84 to
b9475de
Compare
Surface creation and destruction events exist "specifically" for Android which has diverging lifetimes for its `Surface` structure compared to other platforms; specifically Vulkan `VkSurfaceKHR` or `EGLSurface` objects need to be destroyed and recreated. Commit 6cdb317 ("Consistently deliver a Resumed event on all platforms") made sure to consistently call `can_create_surfaces()` (`Event::Resumed` back then) on all platforms directly after startup so that users don't have to have platform-specific surface creation behaviour in two disjoint places, but we forgot about `destroy_surfaces()` (back then `Event::Suspended`) leading to applications still having to handle this destruction in two different places. Solve that by calling the callback on all those platforms, directly before returning `PumpStatus::Exit` from `fn single_iteration()`.
MarijnS95
force-pushed
the
consistent-destroy-surfaces
branch
from
b9475de to
1e4c538
Compare
MarijnS95
commented
Jan 21, 2026
Comment on lines
-290
to
+292
| /// Dispatch control flow events (`NewEvents`, `AboutToWait`, and | ||
| /// `LoopExiting`) as necessary to bring the internal `RunnerState` to the | ||
| /// new runner state. | ||
| /// Dispatch control flow events ([`ApplicationHandler::new_events()`] and | ||
| /// [`ApplicationHandler::about_to_wait()`]) as necessary to bring the internal [`RunnerState`] | ||
| /// to the new runner state. |
Member
Author
There was a problem hiding this comment.
Fwiw while looking through places to modify code, I found lots of these cases where no intradoc-links are used, in turn allowing completely stale references to inexistent identifiers to exist in the codebase (LoopExiting is gone in favour of dropping App, or is it not?).
A simple cargo doc test with disallowed warnings would have found them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3206
Surface creation and destruction events exist "specifically" for Android which has diverging lifetimes for its
Surfacestructure compared to other platforms; specifically VulkanVkSurfaceKHRorEGLSurfaceobjects need to be destroyed and recreated.Commit 6cdb317 ("Consistently deliver a Resumed event on all platforms") made sure to consistently call
can_create_surfaces()(Event::Resumedback then) on all platforms directly after startup so that users don't have to have platform-specific surface creation behaviour in two disjoint places, but we forgot aboutdestroy_surfaces()(back thenEvent::Suspended) leading to applications still having to handle this destruction in two different places.Solve that by calling the callback on all those platforms, directly before returning
PumpStatus::Exitfromfn single_iteration()orfn pump_app_events()(on Appkit).Tested on:
Androidthis was already supported before this PR, of course; but would be useful to check/assert that Android (the two backendsandroid-activityhosts currently) consistently emit this event before exiting the loop however.CFRunLoopActivity::Exit?Important
Draft because this is entirely untested and may not even compile on all platforms. The samples likely still need to be updated to match the new behaviour too.
In a distant future (before Winit 0.31 perhaps) these events should perhaps be moved to
WindowEvent, as outlined in #4420.