From d3d21c65f422d49ad5689d42f28ce5b99ee69933 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 21 Jul 2021 17:50:29 +0200 Subject: [PATCH] Change editor window destruction order The wrapper window can only be removed after we reparented the Wine window back to the root. --- src/wine-host/editor.cpp | 54 +++++++++++++------------- src/wine-host/editor.h | 84 ++++++++++++++++++++-------------------- 2 files changed, 69 insertions(+), 69 deletions(-) diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index a3f339d3..9459d8dc 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -242,6 +242,31 @@ Editor::Editor(MainContext& main_context, logger(logger), x11_connection(xcb_connect(nullptr, nullptr), xcb_disconnect), dnd_proxy_handle(WineXdndProxy::get_handle()), + xcb_wm_state_property( + get_atom_by_name(*x11_connection, wm_state_property_name)), + parent_window(parent_window_handle), + host_window(find_host_window(*x11_connection, + parent_window, + xcb_wm_state_property) + .value_or(parent_window)), + wrapper_window( + x11_connection, + [parent_window = parent_window]( + std::shared_ptr x11_connection, + xcb_window_t window) { + xcb_generic_error_t* error = nullptr; + const xcb_query_tree_cookie_t query_cookie = + xcb_query_tree(x11_connection.get(), parent_window); + const std::unique_ptr query_reply( + xcb_query_tree_reply(x11_connection.get(), query_cookie, + &error)); + THROW_X11_ERROR(error); + + xcb_create_window(x11_connection.get(), XCB_COPY_FROM_PARENT, + window, query_reply->root, 0, 0, 128, 128, 0, + XCB_WINDOW_CLASS_INPUT_OUTPUT, + XCB_COPY_FROM_PARENT, 0, nullptr); + }), client_area(get_maximum_screen_dimensions(*x11_connection)), // Create a window without any decoratiosn for easy embedding. The // combination of `WS_EX_TOOLWINDOW` and `WS_POPUP` causes the window to @@ -275,6 +300,7 @@ Editor::Editor(MainContext& main_context, nullptr, GetModuleHandle(nullptr), this)), + wine_window(get_x11_handle(win32_window.handle)), // If `config.editor_double_embed` is set, then we'll also create a child // window in `win32_child_window`. If we do this before calling // `ShowWindow()` on `win32_window` we'll run into X11 errors. @@ -288,33 +314,7 @@ Editor::Editor(MainContext& main_context, config.event_loop_interval()) .count()) : Win32Timer()), - idle_timer_proc(std::move(timer_proc)), - xcb_wm_state_property( - get_atom_by_name(*x11_connection, wm_state_property_name)), - parent_window(parent_window_handle), - wrapper_window( - x11_connection, - [parent_window = parent_window]( - std::shared_ptr x11_connection, - xcb_window_t window) { - xcb_generic_error_t* error = nullptr; - const xcb_query_tree_cookie_t query_cookie = - xcb_query_tree(x11_connection.get(), parent_window); - const std::unique_ptr query_reply( - xcb_query_tree_reply(x11_connection.get(), query_cookie, - &error)); - THROW_X11_ERROR(error); - - xcb_create_window(x11_connection.get(), XCB_COPY_FROM_PARENT, - window, query_reply->root, 0, 0, 128, 128, 0, - XCB_WINDOW_CLASS_INPUT_OUTPUT, - XCB_COPY_FROM_PARENT, 0, nullptr); - }), - wine_window(get_x11_handle(win32_window.handle)), - host_window(find_host_window(*x11_connection, - parent_window, - xcb_wm_state_property) - .value_or(parent_window)) { + idle_timer_proc(std::move(timer_proc)) { logger.log_editor_trace( [&]() { return "DEBUG: host_window: " + std::to_string(host_window); }); logger.log_editor_trace([&]() { diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 58abd01a..d73b2b88 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -306,6 +306,44 @@ class Editor { */ WineXdndProxy::Handle dnd_proxy_handle; + /** + * The atom corresponding to `WM_STATE`. + */ + xcb_atom_t xcb_wm_state_property; + /** + * The window handle of the editor window created by the DAW. + */ + const xcb_window_t parent_window; + /** + * The toplevel X11 window `parent_window` is contained in, or + * `parent_window` if the host doesn't do any fancy window embedding. We'll + * find this by looking for the topmost ancestor window of `parent_window` + * that has `WM_STATE` set. This is similar to how `xprop` and `xwininfo` + * select windows. In most cases this is going to be the same as + * `parent_window`, but some DAWs (such as REAPER) embed `parent_window` + * into another window. We have to listen for configuration changes on this + * topmost window to know when the window is being dragged around, and when + * returning keyboard focus to the host we'll focus this window. + * + * NOTE: When reopening a REAPER FX window that has previously been closed, + * REAPER will initialize the first plugin's editor first before + * opening the window. This means that the topmost FX window doesn't + * actually exist yet at that point, so we need to redetect this + * later. + * NOTE: Taking the very topmost window is not an option, because for some + * reason REAPER will only process keyboard input for that window when + * the mouse is within the window. + */ + xcb_window_t host_window; + + /** + * A window that sits between `parent_window` and `wine_window`. The entire + * purpose of this is to prevent the host from responding to the + * `ConfigureNotify` events we send to `wine_window` when the host + * subscribes to `SubStructureNotify` events on `parent_window`. + */ + X11Window wrapper_window; + /** * The Wine window's client area, or the maximum size of that window. This * will be set to a size that's large enough to be able to enter full screen @@ -322,6 +360,10 @@ class Editor { * embed itself in. */ DeferredWin32Window win32_window; + /** + * The X11 window handle of the window belonging to `win32_window`. + */ + const xcb_window_t wine_window; /** * A child window embedded inside of `win32_window`. This is only used if @@ -350,48 +392,6 @@ class Editor { */ std::optional> idle_timer_proc; - /** - * The atom corresponding to `WM_STATE`. - */ - xcb_atom_t xcb_wm_state_property; - - /** - * The window handle of the editor window created by the DAW. - */ - const xcb_window_t parent_window; - /** - * A window that sits between `parent_window` and `wine_window`. The entire - * purpose of this is to prevent the host from responding to the - * `ConfigureNotify` events we send to `wine_window` when the host - * subscribes to `SubStructureNotify` events on `parent_window`. - */ - X11Window wrapper_window; - /** - * The X11 window handle of the window belonging to `win32_window`. - */ - const xcb_window_t wine_window; - /** - * The toplevel X11 window `parent_window` is contained in, or - * `parent_window` if the host doesn't do any fancy window embedding. We'll - * find this by looking for the topmost ancestor window of `parent_window` - * that has `WM_STATE` set. This is similar to how `xprop` and `xwininfo` - * select windows. In most cases this is going to be the same as - * `parent_window`, but some DAWs (such as REAPER) embed `parent_window` - * into another window. We have to listen for configuration changes on this - * topmost window to know when the window is being dragged around, and when - * returning keyboard focus to the host we'll focus this window. - * - * NOTE: When reopening a REAPER FX window that has previously been closed, - * REAPER will initialize the first plugin's editor first before - * opening the window. This means that the topmost FX window doesn't - * actually exist yet at that point, so we need to redetect this - * later. - * NOTE: Taking the very topmost window is not an option, because for some - * reason REAPER will only process keyboard input for that window when - * the mouse is within the window. - */ - xcb_window_t host_window; - /** * The atom corresponding to `_NET_ACTIVE_WINDOW`. */