[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 <[email protected]>
aseprite · Jul 20, 2024 · 86f042d · 86f042d
1 parent 2a25a91
commit 86f042d
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 48 deletions.
diff --git 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
@@ -0,0 +1,24 @@
+#include "monitor.h"
+#include "os/x11/x11.h"
+
+#include <X11/extensions/Xrandr.h>
+
+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
@@ -0,0 +1,29 @@
+#ifndef MONITOR_H
+#define MONITOR_H
+#pragma once
+
+#include <memory>
+#include <X11/extensions/Xrandr.h>
+
+namespace os {
+
+struct MonitorCloser {
+  void operator()(XRRMonitorInfo* monitors) const noexcept { XRRFreeMonitors(monitors); }
+};
+
+typedef std::unique_ptr<XRRMonitorInfo, MonitorCloser> 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
@@ -9,62 +9,45 @@
 #pragma once
 
 #include "os/screen.h"
+#include "os/x11/monitor.h"
 #include "os/x11/x11.h"
 
-#include <X11/Xatom.h>
+#include <X11/extensions/Xrandr.h>
 
 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 {
     // TODO get screen color space
     return os::instance()->makeColorSpace(gfx::ColorSpace::MakeSRGB());
   }
   void* nativeHandle() const override {
-    return reinterpret_cast<void*>(m_screen);
+    return reinterpret_cast<void*>(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
@@ -14,7 +14,9 @@
 #include "os/x11/screen.h"
 #include "os/x11/x11.h"
 
+#include <X11/Xlib.h>
 #include <X11/Xutil.h>
+#include <X11/extensions/Xrandr.h>
 
 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<ScreenX11>(
-      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; monitor<numMonitors; monitor++) {
+      if (monitors->monitor(monitor).primary) {
+        return make_ref<ScreenX11>(monitor);
+      }
+    }
+
+    return nullptr;
   }
 
   void listScreens(ScreenList& list) override {
-    const int nscreen = XScreenCount(X11::instance()->display());
-    for (int screen=0; screen<nscreen; screen++)
-      list.push_back(make_ref<ScreenX11>(screen));
+    const int numMonitors = X11::instance()->monitors()->numMonitors();
+    for (int monitor=0; monitor<numMonitors; monitor++)
+      list.push_back(make_ref<ScreenX11>(monitor));
   }
 
 private:

diff --git 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<ScreenX11>(DefaultScreen(m_display));
+  return os::instance()->mainScreen();
 }
 
 os::ColorSpaceRef WindowX11::colorSpace() const

diff --git 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<MonitorsX11>();
+  }
+
+  return m_monitors.get();
+}
+
+
 } // namespace os
diff --git 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 <X11/Xlib.h>
@@ -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<XInput> m_xinput;
+  std::unique_ptr<MonitorsX11> m_monitors;
 };
 
 } // namespace os