Created
April 4, 2026 10:16
-
-
Save jjolmo/f8a6e251380fd36a320eb357ad8b3cb9 to your computer and use it in GitHub Desktop.
Aseprite XWayland fixes: color picker crash + popup closing + capture release
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| diff --git a/src/app/ui/popup_window_pin.cpp b/src/app/ui/popup_window_pin.cpp | |
| index dc3f839a0..6ed26f082 100644 | |
| --- a/src/app/ui/popup_window_pin.cpp | |
| +++ b/src/app/ui/popup_window_pin.cpp | |
| @@ -62,6 +62,15 @@ bool PopupWindowPin::onProcessMessage(Message* msg) | |
| m_pinned = false; | |
| break; | |
| } | |
| + | |
| + // Don't close this popup when the mouse moves outside the hot | |
| + // region. This kind of popup (color picker, etc.) should only | |
| + // close on explicit actions: clicking outside, pressing | |
| + // Escape/Enter, or closing via the close button. | |
| + case kMouseMoveMessage: | |
| + if (!m_pinned) | |
| + return Window::onProcessMessage(msg); | |
| + break; | |
| } | |
| return PopupWindow::onProcessMessage(msg); | |
| diff --git a/src/ui/manager.cpp b/src/ui/manager.cpp | |
| index 8d2e6f962..605d60cd4 100644 | |
| --- a/src/ui/manager.cpp | |
| +++ b/src/ui/manager.cpp | |
| @@ -461,12 +461,19 @@ void Manager::generateMessagesFromOSEvents() | |
| break; | |
| } | |
| - case os::Event::WindowLeave: | |
| + case os::Event::AppLeave: | |
| + // Release capture only when focus leaves the application | |
| + // entirely (e.g. Alt+Tab to another app). We don't release | |
| + // on WindowLeave because on XWayland the compositor can | |
| + // send spurious FocusOut events that would prematurely | |
| + // release capture and close popup windows (e.g. the color | |
| + // picker popup). | |
| if (capture_widget) { | |
| - const gfx::Point mousePos = display->nativeWindow()->pointFromScreen( | |
| + Display* captureDisplay = capture_widget->display(); | |
| + const gfx::Point mousePos = captureDisplay->nativeWindow()->pointFromScreen( | |
| get_mouse_position()); | |
| auto* msg = newMouseMessage(kMouseUpMessage, | |
| - display, | |
| + captureDisplay, | |
| nullptr, | |
| mousePos, | |
| PointerType::Unknown, | |
| diff --git a/laf/os/x11/system.h b/laf/os/x11/system.h | |
| index c8c1ef5..2cb358e 100644 | |
| --- a/laf/os/x11/system.h | |
| +++ b/laf/os/x11/system.h | |
| @@ -14,6 +14,8 @@ | |
| #include "os/x11/screen.h" | |
| #include "os/x11/x11.h" | |
| +#include <algorithm> | |
| + | |
| #include <X11/Xlib.h> | |
| #include <X11/Xutil.h> | |
| #include <X11/extensions/Xrandr.h> | |
| @@ -65,19 +67,42 @@ public: | |
| int screen = XDefaultScreen(display); | |
| ::Window root = XRootWindow(display, screen); | |
| + // Clamp coordinates to the root window bounds to avoid | |
| + // X11 BadMatch error when picking colors outside the screen | |
| + // (e.g. when dragging the color picker outside the window on | |
| + // XWayland or multi-monitor setups). See aseprite/aseprite#5469. | |
| + XWindowAttributes rootAttrs; | |
| + if (!XGetWindowAttributes(display, root, &rootAttrs)) | |
| + return gfx::ColorNone; | |
| + | |
| + int x = std::clamp(screenPosition.x, 0, rootAttrs.width - 1); | |
| + int y = std::clamp(screenPosition.y, 0, rootAttrs.height - 1); | |
| + | |
| + // Install a temporary error handler so that if XGetImage still | |
| + // fails (e.g. on composited desktops) we don't crash. | |
| + static bool s_xerror = false; | |
| + s_xerror = false; | |
| + auto oldHandler = XSetErrorHandler([](::Display*, XErrorEvent*) -> int { | |
| + s_xerror = true; | |
| + return 0; | |
| + }); | |
| + | |
| XImage* image = | |
| - XGetImage(display, root, screenPosition.x, screenPosition.y, 1, 1, AllPlanes, ZPixmap); | |
| - if (image) { | |
| - XColor color; | |
| - color.pixel = XGetPixel(image, 0, 0); | |
| - XDestroyImage(image); | |
| + XGetImage(display, root, x, y, 1, 1, AllPlanes, ZPixmap); | |
| - XQueryColor(display, XDefaultColormap(display, screen), &color); | |
| + XSetErrorHandler(oldHandler); | |
| - // Each red/green/blue channel is 16-bit, so we have to convert to 8-bit. | |
| - return gfx::rgba(color.red >> 8, color.green >> 8, color.blue >> 8); | |
| - } | |
| - return gfx::ColorNone; | |
| + if (s_xerror || !image) | |
| + return gfx::ColorNone; | |
| + | |
| + XColor color; | |
| + color.pixel = XGetPixel(image, 0, 0); | |
| + XDestroyImage(image); | |
| + | |
| + XQueryColor(display, XDefaultColormap(display, screen), &color); | |
| + | |
| + // Each red/green/blue channel is 16-bit, so we have to convert to 8-bit. | |
| + return gfx::rgba(color.red >> 8, color.green >> 8, color.blue >> 8); | |
| } | |
| ScreenRef primaryScreen() override |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment