From 86f042d6a9cefb4415a453e697c2eeb854c55bbd Mon Sep 17 00:00:00 2001 From: Aiden J Erickson Date: Tue, 16 Jul 2024 23:18:38 -0400 Subject: [PATCH] [x11] fix: get desktop bounds correctly (aseprite #3118) Use Xrandr's XRRGetMonitors function to find find the position, bounds, and "main screen" status of all monitors. On X11, there is no supported method to find the workarea of a monitor. This includes the _NET_WORKAREA atom, which breaks multi-monitor setups. This may not be an issue as most window managers aggressively and correctly enforce their, albeit private, workarea for windows. Signed-off-by: Aiden J Erickson --- os/CMakeLists.txt | 9 +++++-- os/x11/monitor.cpp | 24 ++++++++++++++++++ os/x11/monitor.h | 29 ++++++++++++++++++++++ os/x11/screen.h | 61 +++++++++++++++++----------------------------- os/x11/system.h | 22 +++++++++++++---- os/x11/window.cpp | 2 +- os/x11/x11.cpp | 11 +++++++++ os/x11/x11.h | 5 +++- 8 files changed, 115 insertions(+), 48 deletions(-) create mode 100644 os/x11/monitor.cpp create mode 100644 os/x11/monitor.h diff --git a/os/CMakeLists.txt b/os/CMakeLists.txt index 4e8cf9eaa..cbd72fc02 100644 --- a/os/CMakeLists.txt +++ b/os/CMakeLists.txt @@ -11,7 +11,9 @@ set(LAF_OS_SOURCES common/system.cpp dnd.cpp system.cpp - window.cpp) + window.cpp + x11/monitor.cpp + x11/monitor.h) if(WIN32) list(APPEND LAF_OS_SOURCES win/color_space.cpp @@ -146,7 +148,10 @@ else() if(NOT X11_Xinput_FOUND) message(FATAL_ERROR "Xinput library not found") endif() - list(APPEND LAF_OS_PLATFORM_LIBS ${X11_Xcursor_LIB}) + if(NOT X11_Xrandr_FOUND) + message(FATAL_ERROR "Xrandr library not found") + endif() + list(APPEND LAF_OS_PLATFORM_LIBS ${X11_Xcursor_LIB} ${X11_Xrandr_LIB}) check_library_exists(X11 XOpenIM "${X11_LIB_SEARCH_PATH}" XIM_FOUND) diff --git a/os/x11/monitor.cpp b/os/x11/monitor.cpp new file mode 100644 index 000000000..2b18bb95a --- /dev/null +++ b/os/x11/monitor.cpp @@ -0,0 +1,24 @@ +#include "monitor.h" +#include "os/x11/x11.h" + +#include + +namespace os { + +MonitorsX11::MonitorsX11() +{ + auto x11display = X11::instance()->display(); + + int numMonitors; + XRRMonitorInfo* monitors = XRRGetMonitors(x11display, XDefaultRootWindow(x11display), false, &numMonitors); + + m_numMonitors = numMonitors; + m_monitors = unique_monitors_ptr(monitors); +} + +int MonitorsX11::numMonitors() const { return m_numMonitors; } + +const XRRMonitorInfo& MonitorsX11::monitor(int monitorNum) const { return m_monitors.get()[monitorNum]; } + +} // namespace os + diff --git a/os/x11/monitor.h b/os/x11/monitor.h new file mode 100644 index 000000000..19d2a7d84 --- /dev/null +++ b/os/x11/monitor.h @@ -0,0 +1,29 @@ +#ifndef MONITOR_H +#define MONITOR_H +#pragma once + +#include +#include + +namespace os { + +struct MonitorCloser { + void operator()(XRRMonitorInfo* monitors) const noexcept { XRRFreeMonitors(monitors); } +}; + +typedef std::unique_ptr unique_monitors_ptr; + +class MonitorsX11 { +public: + MonitorsX11(); + int numMonitors() const; + const XRRMonitorInfo& monitor(int monitorNum) const; + +private: + int m_numMonitors; + unique_monitors_ptr m_monitors; +}; + +} // namespace os + +#endif //MONITOR_H diff --git a/os/x11/screen.h b/os/x11/screen.h index 6db6dfb7d..515b176af 100644 --- a/os/x11/screen.h +++ b/os/x11/screen.h @@ -9,51 +9,33 @@ #pragma once #include "os/screen.h" +#include "os/x11/monitor.h" #include "os/x11/x11.h" -#include +#include namespace os { class ScreenX11 : public Screen { public: - ScreenX11(int screen) { - auto x11 = X11::instance(); - auto x11display = x11->display(); - - m_bounds.w = XDisplayWidth(x11display, screen); - m_bounds.h = XDisplayHeight(x11display, screen); - - ::Window root = XDefaultRootWindow(x11display); - Atom _NET_WORKAREA = XInternAtom(x11display, "_NET_WORKAREA", False); - - Atom actual_type; - int actual_format; - unsigned long nitems; - unsigned long bytes_after; - unsigned long* prop; - - int res = XGetWindowProperty(x11display, root, - _NET_WORKAREA, - 0, 4, - False, XA_CARDINAL, - &actual_type, &actual_format, - &nitems, &bytes_after, - (unsigned char**)&prop); - if (res == Success && nitems == 4) { - m_workarea.x = prop[0]; - m_workarea.y = prop[1]; - m_workarea.w = prop[2]; - m_workarea.h = prop[3]; - XFree(prop); - } - else { - m_workarea = m_bounds; - } - } - bool isMainScreen() const override { - return (m_screen == XDefaultScreen(X11::instance()->display())); + ScreenX11(int monitorNum) : m_monitorNum(monitorNum) { + MonitorsX11* monitors = X11::instance()->monitors(); + const XRRMonitorInfo& monitor = monitors->monitor(monitorNum); + + m_bounds.x = monitor.x; + m_bounds.y = monitor.y; + m_bounds.w = monitor.width; + m_bounds.h = monitor.height; + // Xorg doesn't really provide a way to find workarea per monitor :/ + m_workarea.x = monitor.x; + m_workarea.y = monitor.y; + m_workarea.w = monitor.width; + m_workarea.h = monitor.height; + + m_isPrimary = monitor.primary; } + + bool isMainScreen() const override { return m_isPrimary; } gfx::Rect bounds() const override { return m_bounds; } gfx::Rect workarea() const override { return m_workarea; } os::ColorSpaceRef colorSpace() const override { @@ -61,10 +43,11 @@ class ScreenX11 : public Screen { return os::instance()->makeColorSpace(gfx::ColorSpace::MakeSRGB()); } void* nativeHandle() const override { - return reinterpret_cast(m_screen); + return reinterpret_cast(m_monitorNum); } private: - int m_screen; + int m_monitorNum; + bool m_isPrimary; gfx::Rect m_bounds; gfx::Rect m_workarea; }; diff --git a/os/x11/system.h b/os/x11/system.h index 360ca3916..c974dfeca 100644 --- a/os/x11/system.h +++ b/os/x11/system.h @@ -14,7 +14,9 @@ #include "os/x11/screen.h" #include "os/x11/x11.h" +#include #include +#include namespace os { @@ -25,6 +27,7 @@ class SystemX11 : public CommonSystem { void setTabletOptions(const TabletOptions& options) override { m_tabletOptions = options; } + TabletOptions tabletOptions() const override { return m_tabletOptions; } @@ -81,14 +84,23 @@ class SystemX11 : public CommonSystem { } ScreenRef mainScreen() override { - return make_ref( - XDefaultScreen(X11::instance()->display())); + MonitorsX11* monitors = X11::instance()->monitors(); + const int numMonitors = monitors->numMonitors(); + + // we have to search for the primary monitor + for (int monitor=0; monitormonitor(monitor).primary) { + return make_ref(monitor); + } + } + + return nullptr; } void listScreens(ScreenList& list) override { - const int nscreen = XScreenCount(X11::instance()->display()); - for (int screen=0; screen(screen)); + const int numMonitors = X11::instance()->monitors()->numMonitors(); + for (int monitor=0; monitor(monitor)); } private: diff --git a/os/x11/window.cpp b/os/x11/window.cpp index 788405009..dd9f68676 100644 --- a/os/x11/window.cpp +++ b/os/x11/window.cpp @@ -447,7 +447,7 @@ WindowX11::~WindowX11() os::ScreenRef WindowX11::screen() const { - return os::make_ref(DefaultScreen(m_display)); + return os::instance()->mainScreen(); } os::ColorSpaceRef WindowX11::colorSpace() const diff --git a/os/x11/x11.cpp b/os/x11/x11.cpp index 886010a6c..4021b48a6 100644 --- a/os/x11/x11.cpp +++ b/os/x11/x11.cpp @@ -73,4 +73,15 @@ XInput* X11::xinput() return m_xinput.get(); } +MonitorsX11* X11::monitors() +{ + if (!m_monitors) { + // TODO: Use `XRRScreenChangeNotifyEvent` in the man page of Xrandr(3) to update this variable? + m_monitors = std::make_unique(); + } + + return m_monitors.get(); +} + + } // namespace os diff --git a/os/x11/x11.h b/os/x11/x11.h index 0e8fe7e15..60eb87aae 100644 --- a/os/x11/x11.h +++ b/os/x11/x11.h @@ -10,7 +10,8 @@ #pragma once #include "base/debug.h" -#include "gfx/color_space.h" // Include here avoid error with None +#include "gfx/color_space.h" // Include here avoid error with None +#include "monitor.h" #include "os/event_queue.h" #include @@ -32,11 +33,13 @@ class X11 { ::Display* display() const { return m_display; } ::XIM xim() const { return m_xim; } XInput* xinput(); + MonitorsX11* monitors(); private: ::Display* m_display; ::XIM m_xim; std::unique_ptr m_xinput; + std::unique_ptr m_monitors; }; } // namespace os