From 9005474dedcae64b65d5ce3d208ab57f5ebc383a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 28 Apr 2023 15:27:02 +0200 Subject: [PATCH] Spool fix_local_coordinates() call until release This will cause the function to only be called on a `ConfigureNotify` after all mouse buttons have been released. This prevents flickering when dragging windows around. --- CHANGELOG.md | 4 ++++ src/wine-host/editor.cpp | 39 ++++++++++++++++++++++++++++++++++++++- src/wine-host/editor.h | 20 ++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aef78763..16f1beed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Changed +- When dragging plugin windows around, yabridge now waits for the mouse buttons + to be released before informing Wine about the window's new screen + coordinates. This prevents constant flickering when dragging plugin windows + around with some plugin and window manager combinations. - Yabridge now preemptively unsets the `WAYLAND_DISPLAY` environment variable when launching Wine. Upstream Wine currently does not yet have a Wayland driver, but future versions may. When that happens yabridge's X11 window diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 2114544e..5120d8fc 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -469,6 +469,17 @@ void Editor::handle_x11_events() noexcept { // function calls involving it will fail. All functions called from // here should be able to handle that cleanly. try { + // HACK: See the docstrings on `should_fix_local_coordinates_` and + // `fix_local_coordinates()` + if (should_fix_local_coordinates_ && !is_mouse_button_held()) { + logger_.log_editor_trace([&]() { + return "DEBUG: Performing spooled local coordinate fix"; + }); + + fix_local_coordinates(); + should_fix_local_coordinates_ = false; + } + std::unique_ptr generic_event; while (generic_event.reset(xcb_poll_for_event(x11_connection_.get())), generic_event != nullptr) { @@ -551,7 +562,21 @@ void Editor::handle_x11_events() noexcept { event->window == parent_window_ || event->window == wrapper_window_.window_) { if (!use_xembed_) { - fix_local_coordinates(); + // NOTE: See the docstring on this field. This + // avoids flickering with some window manager + // and plugin combinations when dragging + // plugin windows around. + if (is_mouse_button_held()) { + logger_.log_editor_trace([&]() { + return "DEBUG: ConfigureNotify received " + "while mouse button is held down, " + "spooling the coordinate fix"; + }); + + should_fix_local_coordinates_ = true; + } else { + fix_local_coordinates(); + } } } } break; @@ -959,6 +984,18 @@ std::optional Editor::get_current_pointer_position() const noexcept { .y = query_pointer_reply->root_y + (win32_pos.top - x11_y_pos)}; } +bool Editor::is_mouse_button_held() const { + xcb_generic_error_t* error = nullptr; + const xcb_query_pointer_cookie_t pointer_query_cookie = + xcb_query_pointer(x11_connection_.get(), host_window_); + const std::unique_ptr pointer_query_reply( + xcb_query_pointer_reply(x11_connection_.get(), pointer_query_cookie, + &error)); + THROW_X11_ERROR(error); + + return pointer_query_reply->mask != 0; +} + bool Editor::is_wine_window_active() const { if (!supports_ewmh_active_window()) { return false; diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 54245558..63de9c96 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -234,6 +234,11 @@ class Editor { * Lie to the Wine window about its coordinates on the screen for * reparenting without using XEmbed. See the comment at the top of the * implementation on why this is needed. + * + * One of the events that trigger this is `ConfigureNotify` messages. Some + * WMs may continuously send this message while dragging a window around. To + * avoid flickering, the main `handle_x11_events()` function will wait to + * call this function until the all mouse buttons have been released. */ void fix_local_coordinates() const; @@ -312,6 +317,12 @@ class Editor { */ std::optional get_current_pointer_position() const noexcept; + /** + * Checks whether any mouse button is held. Used to defer calling + * `fix_local_coordinates()` when dragging windows around. + */ + bool is_mouse_button_held() const; + /** * Returns `true` if the currently active window (as per * `_NET_ACTIVE_WINDOW`) contains `wine_window_`. If the window manager does @@ -451,6 +462,15 @@ class Editor { */ xcb_window_t host_window_; + /** + * Used to delay calling `fix_local_coordinates()` when dragging windows + * around with the mouse. Some WMs will continuously send `ConfigureNotify` + * messages when dragging windows around, and the `fix_local_coordinates()` + * function may cause the window to blink. This becomes a but jarring if it + * happens 60 times per second while dragging windows around. + */ + bool should_fix_local_coordinates_ = false; + /** * The atom corresponding to `_NET_ACTIVE_WINDOW`. */