Skip to content

Properly end winit event loop#7565

Merged
emilk merged 3 commits intoemilk:mainfrom
tye-exe:bad_close
Sep 30, 2025
Merged

Properly end winit event loop#7565
emilk merged 3 commits intoemilk:mainfrom
tye-exe:bad_close

Conversation

@tye-exe
Copy link
Copy Markdown
Contributor

@tye-exe tye-exe commented Sep 26, 2025

This PR changes eframe to comply with the proper methodology of ending winit. This fixes the winit window remaining open on Wayland (Linux) by allowing winit to properly clean up the window.

Details

On a winit "CloseRequested" event eframe drops open windows, which then triggers a "Destroyed" event and allows for winit to clean up resources. In response to the "Destroyed" event, the event loop is exited and control flow returned to user code.
This follows the advice recomended in this winit issue in response to a similar problem.

I have tested the new change using:

  • glow on Wayland (Linux)
  • wgpu on Wayland (Linux)
  • glow on X11 (Linux)
  • wgpu on X11 (Linux)
  • glow on WASM
  • wpgu on WASM

Current implementation

output1

After changes

output

Misc Info

I attempted to run ./scripts/check.sh, but it fails on the I have branched from on some code that I have not modified.

This changes eframe to allow for proper clean of up open windows by
dropping open windows when the "CloseRequested" event is received.
And then running the event loop until the winit "Destroy" event is
received.

This aligns with recommendations from winit rust-windowing/winit#4135
This code is never reached as "GlowWinitRunning" is dropped earlier on
as part of the exiting process in response to the winit "CloseRequested"
event.
The winit "Destroyed" event only ever occurs after "CloseRequested".

The exception to this is if a window was not properly closed before the
event loop was excited previously.
@github-actions
Copy link
Copy Markdown

Preview available at https://egui-pr-preview.github.io/pr/7565-badclose
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

@emilk
Copy link
Copy Markdown
Owner

emilk commented Sep 27, 2025

Thanks for workin on this! This looks very promising.

Please run cargo run -p test_viewports and make sure that still works as expected 🙏

@emilk emilk added bug Something is broken eframe Relates to epi and eframe egui-winit porblems related to winit labels Sep 27, 2025
@tye-exe
Copy link
Copy Markdown
Contributor Author

tye-exe commented Sep 27, 2025

It works as best I can tell

viewports.mp4

The other example that I was able to run on my computer behaved the same as when they were run on the main branch (excluding "serial_windows", which works as expected on Wayland with this PR).

@tye-exe
Copy link
Copy Markdown
Contributor Author

tye-exe commented Sep 27, 2025

I researched into the why the viewport ID check was added and it to resolve this issue with this commit.

if viewport_id == Some(ViewportId::ROOT) {
    return EventResult::Exit;
} else {
    return EventResult::Wait;
}

However, I cannot verify that this is not a regression as the close button cannot be hidden on Wayland, and the example did not run at all on X11. Although if this the window is recreated (which it appears to be as my closing animation is played each time the button in the example is clicked) on Wayland a crash did not occur.

I believe that this is not a regression, as the viewport does close and reopen on Wayland without any issues. However, the example did not run at all on X11, either on the main branch or my branch.

@emilk
Copy link
Copy Markdown
Owner

emilk commented Sep 30, 2025

I wonder if this also solves

@tye-exe
Copy link
Copy Markdown
Contributor Author

tye-exe commented Sep 30, 2025

I believe that this issue will be solved, on Linux at least, as the behavior described in that issue matches exactly what I have fixed.
Though I am not able to verify if it is solved on MacOS, since I do not have access to any Apple devices. I may be able to test on a Windows device in about 10 or so hours, as I do not have any to hand.

@emilk
Copy link
Copy Markdown
Owner

emilk commented Sep 30, 2025

I tested it on macOS, and this works great:

Copy link
Copy Markdown
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Thanks!

@emilk emilk merged commit a450b1c into emilk:main Sep 30, 2025
28 of 29 checks passed
podusowski pushed a commit to podusowski/egui that referenced this pull request Oct 10, 2025
@nekename
Copy link
Copy Markdown

nekename commented Dec 2, 2025

Thank you so much, @tye-exe!
Because showing the window after it's been hidden is broken in egui on Windows and Wayland, I spent 25mins implementing opening a new window as a solution for those platforms in my project using a loop and a wait for a tokio::sync::Notify.
However when it just went black and unresponsive my heart sank and I was fully ready to abandon the whole effort, even relegating it to a separate branch and scrapping it on main, but I thankfully decided to just look up if there had been any reports of this or fixes.
Turns out I just needed to update from 0.32 to 0.33 :)

Masterchef365 pushed a commit to Masterchef365/egui that referenced this pull request Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is broken eframe Relates to epi and eframe egui-winit porblems related to winit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eframe application window does not close eframe: create multiple windows in sequence

3 participants