Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

Window destruction #50

Open
dhardy opened this issue Dec 15, 2022 · 1 comment
Open

Window destruction #50

dhardy opened this issue Dec 15, 2022 · 1 comment
Labels
backend:x11 bug Something isn't working

Comments

@dhardy
Copy link
Contributor

dhardy commented Dec 15, 2022

Symptom: the shello example runs fine, but hangs on exit.

Apparent cause: the X11 backend destroys a window before user state (handler) gets destroyed. This is potentially a problem because that user state may include stuff which depends on the window. At least, I appear to be hitting this issue:

So to fix this, naively, we need to destroy the RenderSurface before the window, like this:

impl WindowState {
    // ...
    fn destroy(&mut self) {
        drop(self.surface.take());

        Application::global().quit()
    }
}

... but there are two problems here:

  1. We rely on user applications to handle destruction in the correct order, despite this being a cross-platform API and the requirement only being present on one platform (X11)
  2. WGPU doesn't actually destroy the swapchain here (I'm not even sure if it is supposed to, though the Surface is the only part which is window-specific)

The alternative would be to keep the X11 window alive until after user state (handler) has been destroyed. This can't simply be done from backend::x11::window::Window::destroy since self.handler is borrowed when this method is called so further redesign would be needed (e.g. ensure the user state is destroyed immediately after handle_destroy_notify is called).

There is a further complication in that user state may hold a copy of a WindowHandle, which contains a Weak reference to the window object. I wonder whether further design changes are sensible:

  • Pass handle: &WindowHandle into all methods of WinHandler, remove Clone impl
  • Application::clipboard returns a temporary (non-Clone) object Clipboard<'_>
@lord lord added backend:x11 bug Something isn't working labels Apr 2, 2023
@jneem
Copy link
Collaborator

jneem commented Apr 9, 2023

Looks like this will be fixed in new xservers: https://gitlab.freedesktop.org/mesa/mesa/-/commit/819cbf329a56f1e72a4192b727c7a6d44ad2c2d7

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend:x11 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants