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.

Sign up for GitHub

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

Fix Wine window wrapping and remove coordinate hack. #405

Closed
wants to merge 2 commits into from
+102 −170
Closed

Fix Wine window wrapping and remove coordinate hack. #405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
660549d
Fix window wrapping by implementing a minimal ICCCM window manager.
rbernon Feb 27, 2025
21dc7ae
Remove now unnecessary fix_local_coordinates workaround.
rbernon Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 95 additions & 133 deletions src/wine-host/editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ constexpr uint32_t parent_event_mask =
* slightly when the mouse is already inside of the editor window when
* opening it.
*/
constexpr uint32_t wrapper_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY |
constexpr uint32_t wrapper_event_mask = XCB_EVENT_MASK_SUBSTRUCTURE_REDIRECT |
XCB_EVENT_MASK_STRUCTURE_NOTIFY |
XCB_EVENT_MASK_KEY_PRESS |
XCB_EVENT_MASK_KEY_RELEASE;

Expand Down Expand Up @@ -266,8 +267,9 @@ Editor::Editor(MainContext& main_context,
logger_(logger),
x11_connection_(xcb_connect(nullptr, nullptr), xcb_disconnect),
dnd_proxy_handle_(WineXdndProxy::get_handle()),
client_area_(get_maximum_screen_dimensions(*x11_connection_)),
wrapper_window_size_({128, 128}),
host_window_config_({}),
parent_window_config_({}),
// Create a window without any decoratiosn for easy embedding. The
// combination of `WS_EX_TOOLWINDOW` and `WS_POPUP` causes the window to
// be drawn without any decorations (making resizes behave as you'd
Expand All @@ -279,23 +281,10 @@ Editor::Editor(MainContext& main_context,
reinterpret_cast<LPCSTR>(get_window_class()),
"yabridge plugin",
WS_POPUP,
// NOTE: With certain DEs/WMs (notably,
// Cinnamon), Wine does not render the
// window at all when using a primary
// display that's positioned to the
// right of another display. Presumably
// it tries to manually clip the client
// rendered client area to the physical
// display. During the reparenting and
// `fix_local_coordinates()` the window
// will be moved to `(0, 0)` anyways,
// but setting its initial position
// according to the primary display
// fixes these rendering issues.
GetSystemMetrics(SM_XVIRTUALSCREEN),
GetSystemMetrics(SM_YVIRTUALSCREEN),
client_area_.width,
client_area_.height,
0,
0,
wrapper_window_size_.width,
wrapper_window_size_.height,
nullptr,
nullptr,
GetModuleHandle(nullptr),
Expand Down Expand Up @@ -427,6 +416,8 @@ void Editor::resize(uint16_t width, uint16_t height) {
const std::array<uint32_t, 2> values{width, height};
xcb_configure_window(x11_connection_.get(), wrapper_window_.window_,
value_mask, values.data());
xcb_configure_window(x11_connection_.get(), wine_window_,
value_mask, values.data());
xcb_flush(x11_connection_.get());

// NOTE: This lets us skip resize requests in CLAP plugins when the plugin
Expand All @@ -449,13 +440,6 @@ void Editor::resize(uint16_t width, uint16_t height) {
SetWindowPos(win32_window_.handle_, nullptr, 0, 0, 0, 0,
SWP_NOSIZE | SWP_NOREDRAW | SWP_NOACTIVATE |
SWP_NOOWNERZORDER | SWP_DEFERERASE | SWP_NOCOPYBITS);

// Make sure that after the resize the screen coordinates always match
// up properly. Without this Soundtoys Crystallizer might appear choppy
// or skip a frame during their resize animation (which somehow calls
// `audioMasterSizeWindow()` with the same size a bunch of times in a
// row).
fix_local_coordinates();
}
}

Expand All @@ -469,17 +453,6 @@ 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<xcb_generic_event_t> generic_event;
while (generic_event.reset(xcb_poll_for_event(x11_connection_.get())),
generic_event != nullptr) {
Expand Down Expand Up @@ -558,28 +531,94 @@ void Editor::handle_x11_events() noexcept {
std::to_string(event->window);
});

// If the host window is different from the parent window then
// the Wine window is at a non-zero offset from the top-left
// corner. The host window will always receive absolute position
// information in its events, sent from the window manager, while
// the parent window might receive position changes relative to
// the host window when it is a child window.
if (event->window == host_window_ &&
is_synthetic_event) {
host_window_config_ = *event;
}
if (event->window == parent_window_ &&
host_window_ != parent_window_ &&
!is_synthetic_event) {
parent_window_config_ = *event;
}

// Window managers are expected to send ConfigureNotify to
// their managed windows whenever the window is being moved
// or resized by the user, so that application don't have to
// do all the relative positioning computation themselves.
// Wine also expects this and ignores position changes on its
// window parents, and its window position would get out of
// sync without this event.
if (event->window == host_window_ ||
event->window == parent_window_ ||
event->window == wrapper_window_.window_) {
if (!use_xembed_) {
// 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();
}
}
event->window == parent_window_) {
xcb_configure_notify_event_t translated_event{};
translated_event.response_type = XCB_CONFIGURE_NOTIFY;
translated_event.event = wine_window_;
translated_event.window = wine_window_;
translated_event.width = event->width;
translated_event.height = event->height;
translated_event.x = host_window_config_.x + parent_window_config_.x;
translated_event.y = host_window_config_.y + parent_window_config_.y;

xcb_send_event(
x11_connection_.get(), false, wine_window_,
XCB_EVENT_MASK_STRUCTURE_NOTIFY | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY,
reinterpret_cast<char*>(&translated_event));
xcb_flush(x11_connection_.get());
}
} break;
// We're listening for `ConfigureRequest` events on the
// wrapper window. This is received whenever Wine wants
// to configure its window, and we need to adjust the
// configuration so that it stays within our wrapper.
// Here, wwe could translate window position changes by
// moving the wrapper window itself but this isn't really
// necessary. Instead, we prevent Wine from actually moving
// its window.
case XCB_CONFIGURE_REQUEST: {
const auto event =
reinterpret_cast<xcb_configure_request_event_t*>(
generic_event.get());
logger_.log_editor_trace([&]() {
return "DEBUG: ConfigureRequest for window " +
std::to_string(event->window);
});
const uint16_t value_mask = XCB_CONFIG_WINDOW_X |
XCB_CONFIG_WINDOW_Y |
XCB_CONFIG_WINDOW_WIDTH |
XCB_CONFIG_WINDOW_HEIGHT;
const std::array<uint32_t, 4> values{0, 0, event->width, event->height};
xcb_configure_window(x11_connection_.get(), wine_window_,
value_mask, values.data());
xcb_flush(x11_connection_.get());
} break;
// We're listening for `MapRequest` events on the wrapper
// window. This is received whenever Wine wants to map its
// window, and we need to forward the request to the X server.
// Wine also expects the window manager to change the WM_STATE
// property whenever it has finished mapping the window. We
// effectively implement a sub window manager here, so update
// the property as we should.
case XCB_MAP_REQUEST: {
const auto event =
reinterpret_cast<xcb_map_request_event_t*>(
generic_event.get());
logger_.log_editor_trace([&]() {
return "DEBUG: MapRequest for window " +
std::to_string(event->window);
});
xcb_map_window(x11_connection_.get(), wine_window_);

const std::array<uint32_t, 2> values{XCB_ICCCM_WM_STATE_NORMAL, 0};
xcb_change_property(x11_connection_.get(), XCB_PROP_MODE_REPLACE, wine_window_, xcb_wm_state_property_,
xcb_wm_state_property_, 32, 2, values.data());
xcb_flush(x11_connection_.get());
} break;
// Start the XEmbed procedure when the window becomes visible,
// since most hosts will only show the window after the plugin
// has embedded itself into it.
Expand Down Expand Up @@ -630,10 +669,6 @@ void Editor::handle_x11_events() noexcept {

if (window == parent_window_ ||
window == wrapper_window_.window_) {
if (!use_xembed_) {
fix_local_coordinates();
}

// In case the WM somehow does not support
// `_NET_ACTIVE_WINDOW`, a more naive focus grabbing
// method implemented in the `WM_PARENTNOTIFY` handler
Expand Down Expand Up @@ -782,67 +817,6 @@ HWND Editor::win32_handle() const noexcept {
return win32_window_.handle_;
}

void Editor::fix_local_coordinates() const {
if (use_xembed_) {
return;
}

// We're purposely not using XEmbed here. This has the consequence that wine
// still thinks that any X and Y coordinates are relative to the x11 window
// root instead of the parent window provided by the DAW, causing all sorts
// of GUI interactions to break. To alleviate this we'll just lie to Wine
// and tell it that it's located at the parent window's location on the root
// window. We also will keep the child window at its largest possible size
// to allow for smooth resizing. This works because the embedding hierarchy
// is DAW window -> Win32 window (created in this class) -> VST plugin
// window created by the plugin itself. In this case it doesn't matter that
// the Win32 window is larger than the part of the client area the plugin
// draws to since any excess will be clipped off by the parent window.
const xcb_window_t root = get_root_window(*x11_connection_, parent_window_);

// We can't directly use the `event.x` and `event.y` coordinates because the
// parent window may also be embedded inside another window.
// NOTE: Tracktion Waveform uses client side decorations, and for VST2
// plugins they forgot to add a separate parent window that's already
// offset correctly. Instead, they'll have the plugin embed itself
// inside directly inside of the dialog, and Waveform then moves the
// window 27 pixels down. That's why we cannot use `parent_window_`
// here.
xcb_generic_error_t* error = nullptr;
const xcb_translate_coordinates_cookie_t translate_cookie =
xcb_translate_coordinates(x11_connection_.get(),
wrapper_window_.window_, root, 0, 0);
const std::unique_ptr<xcb_translate_coordinates_reply_t>
translated_coordinates(xcb_translate_coordinates_reply(
x11_connection_.get(), translate_cookie, &error));
THROW_X11_ERROR(error);

xcb_configure_notify_event_t translated_event{};
translated_event.response_type = XCB_CONFIGURE_NOTIFY;
translated_event.event = wine_window_;
translated_event.window = wine_window_;
// This should be set to the same sizes the window was created on. Since
// we're not using `SetWindowPos` to resize the Window, Wine can get a bit
// confused when we suddenly report a different client area size. Without
// this certain plugins (such as those by Valhalla DSP) would break.
translated_event.width = client_area_.width;
translated_event.height = client_area_.height;
translated_event.x = translated_coordinates->dst_x;
translated_event.y = translated_coordinates->dst_y;

logger_.log_editor_trace([&]() {
return "DEBUG: Spoofing local coordinates to (" +
std::to_string(translated_event.x) + ", " +
std::to_string(translated_event.y) + ")";
});

xcb_send_event(
x11_connection_.get(), false, wine_window_,
XCB_EVENT_MASK_STRUCTURE_NOTIFY | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY,
reinterpret_cast<char*>(&translated_event));
xcb_flush(x11_connection_.get());
}

void Editor::set_input_focus(bool grab) const {
// NOTE: When grabbing focus, you can hold down the shift key to focus the
// Wine window directly. This allows you to use the space key in
Expand Down Expand Up @@ -984,18 +958,6 @@ std::optional<POINT> 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<xcb_query_pointer_reply_t> 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;
Expand Down
44 changes: 7 additions & 37 deletions src/wine-host/editor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#pragma push_macro("_WIN32")
#undef _WIN32
#include <xcb/xcb.h>
#include <xcb/xcb_icccm.h>
#pragma pop_macro("_WIN32")

#include "../common/configuration.h"
Expand Down Expand Up @@ -230,18 +231,6 @@ class Editor {
*/
bool supports_ewmh_active_window() const;

/**
* 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;

/**
* Steal or release keyboard focus. This is done whenever the user clicks on
* the window since we don't have a way to detect whether the client window
Expand Down Expand Up @@ -317,12 +306,6 @@ class Editor {
*/
std::optional<POINT> 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
Expand Down Expand Up @@ -378,22 +361,18 @@ class Editor {
*/
WineXdndProxy::Handle dnd_proxy_handle_;

/**
* 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
* on a single display. This is more of a theoretical maximum size, as the
* plugin will only use a portion of this window to draw to. Because we're
* not changing the size of the Wine window and only resize the wrapper
* window it's been embedded in, resizing will feel smooth and native.
*/
const Size client_area_;

/**
* The size of the wrapper window. We'll prevent CLAP resize requests when
* the wrapper window is already at the correct size.
*/
Size wrapper_window_size_;

/**
* Last received configurations for the host and parent windows.
*/
xcb_configure_notify_event_t host_window_config_;
xcb_configure_notify_event_t parent_window_config_;

/**
* The handle for the window created through Wine that the plugin uses to
* embed itself in.
Expand Down Expand Up @@ -462,15 +441,6 @@ 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`.
*/
Expand Down
Loading