Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wayland: Window::hide doesn't hide the window #4225

Closed
flukejones opened this issue Dec 28, 2023 · 14 comments · Fixed by #5529
Closed

Wayland: Window::hide doesn't hide the window #4225

flukejones opened this issue Dec 28, 2023 · 14 comments · Fixed by #5529
Labels
a:backend-winit Winit backend (mS,mO) bug Something isn't working
Milestone

Comments

@flukejones
Copy link
Contributor

flukejones commented Dec 28, 2023

On Linux, using Wayland (Gnome). Issue seems universal across desktops. I have not tried xorg as I don't have any requirement for it for the last few years.

To illustrate the issue I created a minimal example here which is a heavily cut down version of what I have working using egui. Of note however is that if I update egui at all I then have the same issue where if I try to keep the app running as a background process but close the main window, the window is just frozen.

The issue may be in winit but I think maybe we can confirm first?

(click on the tray icon to open app window then try close the window).

@flukejones
Copy link
Contributor Author

There are now a few branches using different styles to try and achieve this:

  • slint-lsp-style
  • simplified
  • initial-try

slint-lsp-style is where I've tried to copy the way the lsp-preview does things but I've not been successful. I have found that I do require the Option<App> so I can take and drop the App window and make it actually close. Otherwise show()/hide() don't appear to remove the window at all (but it does stop responding on hide).

If I try to narrow things down I think I get this:

  1. window().hide() does not hide the window, but it does stop reacting (an Option<App> .take() is required)
  2. window().show() makes the window react again
  3. after dropping the window I can't recreate it as it isn't in the right thread...

If I could solve for 3, then I think everything I need follows on - for example creating multiple windows from different threads. I see there are several similar issues, but none address the threaded case.

@tronical
Copy link
Member

tronical commented Jan 2, 2024

window().hide() not hiding the window seems odd to me. We pretty quickly call winit_window.set_visible(false);, but the event loop is kept alive (in your example).

I've tried your example and I think that this is a winit/wayland bug. I'm running this against Gnome with Wayland, and out of the box the show/hide menu items in the tray menu don't do anything. If I unset WAYLAND_DISPLAY (and keep DISPLAY) and let the app run against xwayland, then it seems to work.

Do you observe the same?

@flukejones
Copy link
Contributor Author

I'm not actually able to get this running with xwayland: Other("Winit backend failed to find a suitable renderer. Last failure was: Error creating OpenGL display (Xlib(\n XlibDisplayHandle {\n display: 0x0000555d0c46d950,\n screen: 0,\n },\n)) with glutin: provided display handle is not supported")

@ogoffart ogoffart added bug Something isn't working a:backend-winit Winit backend (mS,mO) labels Jan 15, 2024
@ogoffart ogoffart changed the title Window doesn't close if trying to run in background Wayland: Window::hode doesn't hide the window Jan 17, 2024
@ogoffart ogoffart changed the title Wayland: Window::hode doesn't hide the window Wayland: Window::hide doesn't hide the window Jan 17, 2024
@flukejones
Copy link
Contributor Author

For what it's worth, this seems to also be a bug in winit, so likely the issue isn't slint at all. (I also had the same issue in egui).

I'm now using the same style of "app thread run, window app close" as the LSP and with the latest git pull it seems to work pretty well for my purposes... (this branch of example)

It should be noted that the issue seems to be quite specific, after trying a few things using winit multi-window. That is, if there is one last window and the main thread for it is still running it will not close, but all other windows will do so fine. Looks related to a drop change in winit actually.

@flukejones
Copy link
Contributor Author

flukejones commented Jan 22, 2024

Appears not to be an issue with winit v0.29.10 (from git) multiwindow now.

Update: Looks like the only real way to do what I want is to actually drop the windows. Same as is done in winit example tbh, that drops the windows also, even the last one.

If I have some time, maybe I can submit a slint example of this.

Clarification: actual hide() is broken. But I suspect that is no-longer a workable method of doing things anyway as-per winit docs it's not supported on most platforms https://docs.rs/winit/latest/winit/window/struct.Window.html#method.set_visible

@tronical
Copy link
Member

Ahh, that explains it! I was trying your example last week and I saw the same effect, that hide() didn't just do anything at all. Explains it, when it's not working under Wayland.

Looks like at the least we need to document this :(

@ogoffart
Copy link
Member

Found rust-windowing/winit#2388 (the reporter looks familiar :-) )

This means we'll have to keep the winit Window in a RefCell and destroy it and re-create it all the time the user wants it to have it hidden.

@tronical
Copy link
Member

Agreed

@flukejones
Copy link
Contributor Author

Found rust-windowing/winit#2388 (the reporter looks familiar :-) )

Ha.. I don't know who that is.

This means we'll have to keep the winit Window in a RefCell and destroy it and re-create it all the time the user wants it to have it hidden.

Will this not result in unexpected side-effects? I've not used .hide() on any other systems so I'm not sure how it reacts, but if it retains position/state then doing a drop erases that, right?

@ogoffart
Copy link
Member

Will this not result in unexpected side-effects? I've not used .hide() on any other systems so I'm not sure how it reacts, but if it retains position/state then doing a drop erases that, right?

This will be about destroying the winit::Window, not not the Slint component.
That said, this is a bit complicated because right now, the skia and femtovg renderer holds pointer to the opengl surface that would also vanish. So we'll have to find a solution for that.

@tronical
Copy link
Member

This means we'll have to keep the winit Window in a RefCell and destroy it and re-create it all the time the user wants it to have it hidden.

Will this not result in unexpected side-effects? I've not used .hide() on any other systems so I'm not sure how it reacts, but if it retains position/state then doing a drop erases that, right?

Yes, that's a problem. We'd have to save/restore that manually.

My feeling is that if we work around this, we should limit the workaround to wayland. The downside though is that that means the code path will receive a lot less testing, as hiding a window is probably not that common.

@flukejones
Copy link
Contributor Author

My feeling is that if we work around this, we should limit the workaround to wayland. The downside though is that that means the code path will receive a lot less testing, as hiding a window is probably not that common.

I can assure you that this is a critical feature for asusctl/rog-control-center so it will receive continuous testing.

@ryanabx
Copy link

ryanabx commented May 18, 2024

Ah, I wish I hadn't spent hours trying to figure out what I was doing wrong, haha 😅

I'll try using drop like fluke mentioned...

Also, hey @flukejones , nice to see you in yet another place!

@flukejones
Copy link
Contributor Author

@ryanabx my rog-control-center demonstrate it well.

@ogoffart ogoffart added this to the 1.7 Release milestone Jun 5, 2024
tronical added a commit that referenced this issue Jul 2, 2024
On Wayland hiding a window requires destroying the surface, which
means destroying the winit window as well as the underlying graphics
surface. The latter is tricky as we have to keep the renderer around,
as our WindowAdapter trait's `renderer()` function returns a `&dyn
Renderer` and that also has to work without a window (to obtain text
metrics).

Fixes #4225
tronical added a commit that referenced this issue Jul 2, 2024
On Wayland hiding a window requires destroying the surface, which
means destroying the winit window as well as the underlying graphics
surface. The latter is tricky as we have to keep the renderer around,
as our WindowAdapter trait's `renderer()` function returns a `&dyn
Renderer` and that also has to work without a window (to obtain text
metrics).

Fixes #4225
tronical added a commit that referenced this issue Jul 2, 2024
On Wayland hiding a window requires destroying the surface, which
means destroying the winit window as well as the underlying graphics
surface. The latter is tricky as we have to keep the renderer around,
as our WindowAdapter trait's `renderer()` function returns a `&dyn
Renderer` and that also has to work without a window (to obtain text
metrics).

Fixes #4225
tronical added a commit that referenced this issue Jul 2, 2024
On Wayland hiding a window requires destroying the surface, which
means destroying the winit window as well as the underlying graphics
surface. The latter is tricky as we have to keep the renderer around,
as our WindowAdapter trait's `renderer()` function returns a `&dyn
Renderer` and that also has to work without a window (to obtain text
metrics).

Fixes #4225
tronical added a commit that referenced this issue Jul 3, 2024
On Wayland hiding a window requires destroying the surface, which
means destroying the winit window as well as the underlying graphics
surface. The latter is tricky as we have to keep the renderer around,
as our WindowAdapter trait's `renderer()` function returns a `&dyn
Renderer` and that also has to work without a window (to obtain text
metrics).

Fixes #4225

Co-Authored-By: Olivier Goffart <[email protected]>
tronical added a commit that referenced this issue Jul 3, 2024
On Wayland hiding a window requires destroying the surface, which
means destroying the winit window as well as the underlying graphics
surface. The latter is tricky as we have to keep the renderer around,
as our WindowAdapter trait's `renderer()` function returns a `&dyn
Renderer` and that also has to work without a window (to obtain text
metrics).

Fixes #4225

Co-Authored-By: Olivier Goffart <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:backend-winit Winit backend (mS,mO) bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants