WindowServer: Add API to set/get screen layouts

This sets the stage so that DisplaySettings can configure the screen
layout and set various screen resolutions in one go. It also allows
for an easy "atomic" revert of the previous settings.

Userland/Applications/DisplaySettings/MonitorSettingsWidget.cpp

@@ -80,23 +80,19 @@ void MonitorSettingsWidget::create_frame()
 
 void MonitorSettingsWidget::load_current_settings()
 {
-    auto ws_config = Core::ConfigFile::open("/etc/WindowServer.ini");
-
-    int scale_factor = ws_config->read_num_entry("Screen", "ScaleFactor", 1);
-    if (scale_factor != 1 && scale_factor != 2) {
-        dbgln("unexpected ScaleFactor {}, setting to 1", scale_factor);
-        scale_factor = 1;
+    m_screen_layout = GUI::WindowServerConnection::the().get_screen_layout();
+    auto& screen = m_screen_layout.screens[m_screen_layout.main_screen_index];
+    if (screen.scale_factor != 1 && screen.scale_factor != 2) {
+        dbgln("unexpected ScaleFactor {}, setting to 1", screen.scale_factor);
+        screen.scale_factor = 1;
     }
-    (scale_factor == 1 ? m_display_scale_radio_1x : m_display_scale_radio_2x)->set_checked(true);
-    m_monitor_widget->set_desktop_scale_factor(scale_factor);
+    (screen.scale_factor == 1 ? m_display_scale_radio_1x : m_display_scale_radio_2x)->set_checked(true);
+    m_monitor_widget->set_desktop_scale_factor(screen.scale_factor);
 
     // Let's attempt to find the current resolution and select it!
-    Gfx::IntSize find_size;
-    find_size.set_width(ws_config->read_num_entry("Screen", "Width", 1024));
-    find_size.set_height(ws_config->read_num_entry("Screen", "Height", 768));
-    auto index = m_resolutions.find_first_index(find_size).value_or(0);
-    Gfx::IntSize m_current_resolution = m_resolutions.at(index);
-    m_monitor_widget->set_desktop_resolution(m_current_resolution);
+    auto index = m_resolutions.find_first_index(screen.resolution).value_or(0);
+    Gfx::IntSize current_resolution = m_resolutions.at(index);
+    m_monitor_widget->set_desktop_resolution(current_resolution);
     m_resolution_combo->set_selected_index(index);
 
     m_monitor_widget->update();
@@ -104,28 +100,19 @@ void MonitorSettingsWidget::load_current_settings()
 
 void MonitorSettingsWidget::apply_settings()
 {
-    // Store the current screen resolution and scale factor in case the user wants to revert to it.
-    auto ws_config(Core::ConfigFile::open("/etc/WindowServer.ini"));
-    Gfx::IntSize current_resolution;
-    current_resolution.set_width(ws_config->read_num_entry("Screen", "Width", 1024));
-    current_resolution.set_height(ws_config->read_num_entry("Screen", "Height", 768));
-    int current_scale_factor = ws_config->read_num_entry("Screen", "ScaleFactor", 1);
-    if (current_scale_factor != 1 && current_scale_factor != 2) {
-        dbgln("unexpected ScaleFactor {}, setting to 1", current_scale_factor);
-        current_scale_factor = 1;
-    }
+    // TODO: implement multi-screen support
+    auto& main_screen = m_screen_layout.screens[m_screen_layout.main_screen_index];
+    main_screen.resolution = m_monitor_widget->desktop_resolution();
+    main_screen.scale_factor = (m_display_scale_radio_2x->is_checked() ? 2 : 1);
 
-    if (current_resolution != m_monitor_widget->desktop_resolution() || current_scale_factor != m_monitor_widget->desktop_scale_factor()) {
-        u32 display_index = 0; // TODO: implement multiple display support
-        auto result = GUI::WindowServerConnection::the().set_resolution(display_index, m_monitor_widget->desktop_resolution(), m_monitor_widget->desktop_scale_factor());
-        if (!result.success()) {
-            GUI::MessageBox::show(nullptr, String::formatted("Reverting to resolution {}x{} @ {}x", result.resolution().width(), result.resolution().height(), result.scale_factor()),
-                "Unable to set resolution", GUI::MessageBox::Type::Error);
-        } else {
+    // Fetch the latest configuration again, in case it has been changed by someone else.
+    // This isn't technically race free, but if the user automates changing settings we can't help...
+    auto current_layout = GUI::WindowServerConnection::the().get_screen_layout();
+    if (m_screen_layout != current_layout) {
+        auto result = GUI::WindowServerConnection::the().set_screen_layout(m_screen_layout, false);
+        if (result.success()) {
             auto box = GUI::MessageBox::construct(window(), String::formatted("Do you want to keep the new settings? They will be reverted after 10 seconds."),
-                String::formatted("New screen resolution: {}x{} @ {}x", m_monitor_widget->desktop_resolution().width(), m_monitor_widget->desktop_resolution().height(),
-                    m_monitor_widget->desktop_scale_factor()),
-                GUI::MessageBox::Type::Question, GUI::MessageBox::InputType::YesNo);
+                "Apply new screen layout", GUI::MessageBox::Type::Question, GUI::MessageBox::InputType::YesNo);
             box->set_icon(window()->icon());
 
             // If after 10 seconds the user doesn't close the message box, just close it.
@@ -135,12 +122,15 @@ void MonitorSettingsWidget::apply_settings()
 
             // If the user selects "No", closes the window or the window gets closed by the 10 seconds timer, revert the changes.
             if (box->exec() != GUI::MessageBox::ExecYes) {
-                result = GUI::WindowServerConnection::the().set_resolution(display_index, current_resolution, current_scale_factor);
-                if (!result.success()) {
-                    GUI::MessageBox::show(nullptr, String::formatted("Reverting to resolution {}x{} @ {}x", result.resolution().width(), result.resolution().height(), result.scale_factor()),
-                        "Unable to set resolution", GUI::MessageBox::Type::Error);
+                auto save_result = GUI::WindowServerConnection::the().save_screen_layout();
+                if (!save_result.success()) {
+                    GUI::MessageBox::show(window(), String::formatted("Error saving settings: {}", save_result.error_msg()),
+                        "Unable to save setting", GUI::MessageBox::Type::Error);
                 }
             }
+        } else {
+            GUI::MessageBox::show(window(), String::formatted("Error setting screen layout: {}", result.error_msg()),
+                "Unable to apply changes", GUI::MessageBox::Type::Error);
         }
     }
 }

Userland/Applications/DisplaySettings/MonitorSettingsWidget.h

@@ -11,6 +11,7 @@
 #include <LibGUI/ColorInput.h>
 #include <LibGUI/ComboBox.h>
 #include <LibGUI/RadioButton.h>
+#include <WindowServer/ScreenLayout.h>
 
 namespace DisplaySettings {
 
@@ -27,6 +28,7 @@ private:
     void create_resolution_list();
     void load_current_settings();
 
+    WindowServer::ScreenLayout m_screen_layout;
     Vector<Gfx::IntSize> m_resolutions;
 
     RefPtr<DisplaySettings::MonitorWidget> m_monitor_widget;

Userland/Libraries/LibGUI/CMakeLists.txt

@@ -74,6 +74,7 @@ set(SOURCES
     RegularEditingEngine.cpp
     ResizeCorner.cpp
     RunningProcessesModel.cpp
+    ScreenLayout.cpp
     ScrollableContainerWidget.cpp
     Scrollbar.cpp
     SeparatorWidget.cpp

Userland/Libraries/LibGUI/Desktop.h

@@ -11,11 +11,17 @@
 #include <LibGUI/Forward.h>
 #include <LibGfx/Rect.h>
 #include <Services/Taskbar/TaskbarWindow.h>
+#include <Services/WindowServer/ScreenLayout.h>
 
 namespace GUI {
 
+using ScreenLayout = WindowServer::ScreenLayout;
+
 class Desktop {
 public:
+    // Most people will probably have 4 screens or less
+    static constexpr size_t default_screen_rect_count = 4;
+
     static Desktop& the();
     Desktop();
 
@@ -35,7 +41,7 @@ public:
     void did_receive_screen_rects(Badge<WindowServerConnection>, const Vector<Gfx::IntRect, 4>&, size_t);
 
 private:
-    Vector<Gfx::IntRect, 4> m_rects;
+    Vector<Gfx::IntRect, default_screen_rect_count> m_rects;
     size_t m_main_screen_index { 0 };
     Gfx::IntRect m_bounding_rect;
 };

Userland/Libraries/LibGUI/Forward.h

@@ -85,3 +85,7 @@ enum class ModelRole;
 enum class SortOrder;
 
 }
+
+namespace WindowServer {
+class ScreenLayout;
+}

Userland/Libraries/LibGUI/ScreenLayout.cpp

@@ -0,0 +1,7 @@
+/*
+ * Copyright (c) 2021, the SerenityOS developers.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <Services/WindowServer/ScreenLayout.ipp>

Userland/Libraries/LibGUI/WindowManagerServerConnection.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <LibIPC/ServerConnection.h>
+#include <WindowServer/ScreenLayout.h>
 #include <WindowServer/WindowManagerClientEndpoint.h>
 #include <WindowServer/WindowManagerServerEndpoint.h>
 

Userland/Libraries/LibGUI/WindowServerConnection.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <LibIPC/ServerConnection.h>
+#include <WindowServer/ScreenLayout.h>
 #include <WindowServer/WindowClientEndpoint.h>
 #include <WindowServer/WindowServerEndpoint.h>
 

Userland/Services/NotificationServer/ClientConnection.h

@@ -7,6 +7,9 @@
 #pragma once
 
 #include <LibIPC/ClientConnection.h>
+#include <WindowServer/ScreenLayout.h>
+
+// Must be included after WindowServer/ScreenLayout.h
 #include <NotificationServer/NotificationClientEndpoint.h>
 #include <NotificationServer/NotificationServerEndpoint.h>
 

Userland/Services/WindowServer/CMakeLists.txt

@@ -22,6 +22,7 @@ set(SOURCES
     MenuItem.cpp
     MenuManager.cpp
     Screen.cpp
+    ScreenLayout.cpp
     Window.cpp
     WindowFrame.cpp
     WindowManager.cpp

Userland/Services/WindowServer/ClientConnection.cpp

@@ -297,14 +297,23 @@ Messages::WindowServer::GetWallpaperResponse ClientConnection::get_wallpaper()
     return Compositor::the().wallpaper_path();
 }
 
-Messages::WindowServer::SetResolutionResponse ClientConnection::set_resolution(u32 screen_index, Gfx::IntSize const& resolution, int scale_factor)
+Messages::WindowServer::SetScreenLayoutResponse ClientConnection::set_screen_layout(ScreenLayout const& screen_layout, bool save)
 {
-    if (auto* screen = Screen::find_by_index(screen_index)) {
-        bool success = WindowManager::the().set_resolution(*screen, resolution.width(), resolution.height(), scale_factor);
-        return { success, screen->size(), screen->scale_factor() };
-    }
-    dbgln("Setting resolution: Invalid screen index {}", screen_index);
-    return { false, {}, 0 };
+    String error_msg;
+    bool success = WindowManager::the().set_screen_layout(ScreenLayout(screen_layout), save, error_msg);
+    return { success, move(error_msg) };
+}
+
+Messages::WindowServer::GetScreenLayoutResponse ClientConnection::get_screen_layout()
+{
+    return { WindowManager::the().get_screen_layout() };
+}
+
+Messages::WindowServer::SaveScreenLayoutResponse ClientConnection::save_screen_layout()
+{
+    String error_msg;
+    bool success = WindowManager::the().save_screen_layout(error_msg);
+    return { success, move(error_msg) };
 }
 
 void ClientConnection::set_window_title(i32 window_id, String const& title)

Userland/Services/WindowServer/ClientConnection.h

@@ -18,6 +18,7 @@
 #include <LibIPC/ClientConnection.h>
 #include <WindowServer/Event.h>
 #include <WindowServer/Menu.h>
+#include <WindowServer/ScreenLayout.h>
 #include <WindowServer/WindowClientEndpoint.h>
 #include <WindowServer/WindowServerEndpoint.h>
 
@@ -27,6 +28,7 @@ class Compositor;
 class Window;
 class Menu;
 class Menubar;
+class ScreenLayout;
 class WMClientConnection;
 
 class ClientConnection final
@@ -125,7 +127,9 @@ private:
     virtual void set_background_color(String const&) override;
     virtual void set_wallpaper_mode(String const&) override;
     virtual Messages::WindowServer::GetWallpaperResponse get_wallpaper() override;
-    virtual Messages::WindowServer::SetResolutionResponse set_resolution(u32, Gfx::IntSize const&, int) override;
+    virtual Messages::WindowServer::SetScreenLayoutResponse set_screen_layout(ScreenLayout const&, bool) override;
+    virtual Messages::WindowServer::GetScreenLayoutResponse get_screen_layout() override;
+    virtual Messages::WindowServer::SaveScreenLayoutResponse save_screen_layout() override;
     virtual void set_window_cursor(i32, i32) override;
     virtual void set_window_custom_cursor(i32, Gfx::ShareableBitmap const&) override;
     virtual void popup_menu(i32, Gfx::IntPoint const&) override;

Userland/Services/WindowServer/Compositor.cpp

@@ -815,6 +815,9 @@ void Compositor::step_animations()
 
 void Compositor::screen_resolution_changed()
 {
+    // Screens may be gone now, invalidate any references to them
+    m_current_cursor_screen = nullptr;
+
     init_bitmaps();
     invalidate_occlusions();
     compose();

Userland/Services/WindowServer/Screen.cpp

@@ -22,6 +22,7 @@ namespace WindowServer {
 NonnullOwnPtrVector<Screen, default_screen_count> Screen::s_screens;
 Screen* Screen::s_main_screen { nullptr };
 Gfx::IntRect Screen::s_bounding_screens_rect {};
+ScreenLayout Screen::s_layout;
 
 ScreenInput& ScreenInput::the()
 {
@@ -43,19 +44,53 @@ const Screen& ScreenInput::cursor_location_screen() const
     return *screen;
 }
 
-Screen::Screen(const String& device, const Gfx::IntRect& virtual_rect, int scale_factor)
-    : m_virtual_rect(virtual_rect)
-    , m_scale_factor(scale_factor)
+bool Screen::apply_layout(ScreenLayout&& screen_layout, String& error_msg)
 {
-    m_framebuffer_fd = open(device.characters(), O_RDWR | O_CLOEXEC);
-    if (m_framebuffer_fd < 0) {
-        perror(String::formatted("failed to open {}", device).characters());
-        VERIFY_NOT_REACHED();
+    if (!screen_layout.is_valid(&error_msg))
+        return false;
+
+    auto screens_backup = move(s_screens);
+    auto layout_backup = move(s_layout);
+    for (auto& old_screen : screens_backup)
+        old_screen.close_device();
+
+    AK::ArmedScopeGuard rollback([&] {
+        for (auto& screen : s_screens)
+            screen.close_device();
+        s_screens = move(screens_backup);
+        s_layout = move(layout_backup);
+        for (auto& old_screen : screens_backup) {
+            if (!old_screen.open_device()) {
+                // Don't set error_msg here, it should already be set
+                dbgln("Rolling back screen layout failed: could not open device");
+            }
+        }
+        update_bounding_rect();
+    });
+    s_layout = move(screen_layout);
+    for (size_t index = 0; index < s_layout.screens.size(); index++) {
+        auto* screen = WindowServer::Screen::create(s_layout.screens[index]);
+        if (!screen) {
+            error_msg = String::formatted("Error creating screen #{}", index);
+            return false;
+        }
+
+        if (s_layout.main_screen_index == index)
+            screen->make_main_screen();
+
+        screen->open_device();
     }
 
-    if (fb_set_buffer(m_framebuffer_fd, 0) == 0) {
-        m_can_set_buffer = true;
-    }
+    rollback.disarm();
+    update_bounding_rect();
+    return true;
+}
+
+Screen::Screen(ScreenLayout::Screen& screen_info)
+    : m_virtual_rect(screen_info.location, { screen_info.resolution.width() / screen_info.scale_factor, screen_info.resolution.height() / screen_info.scale_factor })
+    , m_info(screen_info)
+{
+    open_device();
 
     // If the cursor is not in a valid screen (yet), force it into one
     dbgln("Screen() current physical cursor location: {} rect: {}", ScreenInput::the().cursor_location(), rect());
@@ -65,12 +100,41 @@ Screen::Screen(const String& device, const Gfx::IntRect& virtual_rect, int scale
 
 Screen::~Screen()
 {
-    close(m_framebuffer_fd);
+    close_device();
+}
+
+bool Screen::open_device()
+{
+    close_device();
+    m_framebuffer_fd = open(m_info.device.characters(), O_RDWR | O_CLOEXEC);
+    if (m_framebuffer_fd < 0) {
+        perror(String::formatted("failed to open {}", m_info.device).characters());
+        return false;
+    }
+
+    m_can_set_buffer = (fb_set_buffer(m_framebuffer_fd, 0) == 0);
+    set_resolution(true);
+    return true;
+}
+
+void Screen::close_device()
+{
+    if (m_framebuffer_fd >= 0) {
+        close(m_framebuffer_fd);
+        m_framebuffer_fd = -1;
+    }
+    if (m_framebuffer) {
+        int rc = munmap(m_framebuffer, m_size_in_bytes);
+        VERIFY(rc == 0);
+
+        m_framebuffer = nullptr;
+        m_size_in_bytes = 0;
+    }
 }
 
 void Screen::init()
 {
-    do_set_resolution(true, m_virtual_rect.width(), m_virtual_rect.height(), m_scale_factor);
+    set_resolution(true);
 }
 
 Screen& Screen::closest_to_rect(const Gfx::IntRect& rect)
@@ -113,64 +177,53 @@ void Screen::update_bounding_rect()
     }
 }
 
-bool Screen::do_set_resolution(bool initial, int width, int height, int new_scale_factor)
+bool Screen::set_resolution(bool initial)
 {
     // Remember the screen that the cursor is on. Make sure it stays on the same screen if we change its resolution...
-    auto& screen_with_cursor = ScreenInput::the().cursor_location_screen();
+    Screen* screen_with_cursor = nullptr;
+    if (!initial)
+        screen_with_cursor = &ScreenInput::the().cursor_location_screen();
 
-    int new_physical_width = width * new_scale_factor;
-    int new_physical_height = height * new_scale_factor;
-    if (!initial && physical_width() == new_physical_width && physical_height() == new_physical_height) {
-        VERIFY(initial || scale_factor() != new_scale_factor);
-        on_change_resolution(initial, m_pitch, physical_width(), physical_height(), new_scale_factor, screen_with_cursor);
-        return true;
-    }
-
-    FBResolution physical_resolution { 0, (unsigned)new_physical_width, (unsigned)new_physical_height };
+    FBResolution physical_resolution { 0, (unsigned)m_info.resolution.width(), (unsigned)m_info.resolution.height() };
     int rc = fb_set_resolution(m_framebuffer_fd, &physical_resolution);
     dbgln_if(WSSCREEN_DEBUG, "Screen #{}: fb_set_resolution() - return code {}", index(), rc);
 
+    auto on_change_resolution = [&]() {
+        if (initial || physical_resolution.width != (unsigned)m_info.resolution.width() || physical_resolution.height != (unsigned)m_info.resolution.height()) {
+            if (m_framebuffer) {
+                size_t previous_size_in_bytes = m_size_in_bytes;
+                int rc = munmap(m_framebuffer, previous_size_in_bytes);
+                VERIFY(rc == 0);
+            }
+
+            int rc = fb_get_size_in_bytes(m_framebuffer_fd, &m_size_in_bytes);
+            VERIFY(rc == 0);
+
+            m_framebuffer = (Gfx::RGBA32*)mmap(nullptr, m_size_in_bytes, PROT_READ | PROT_WRITE, MAP_SHARED, m_framebuffer_fd, 0);
+            VERIFY(m_framebuffer && m_framebuffer != (void*)-1);
+        }
+
+        m_info.resolution = { physical_resolution.width, physical_resolution.height };
+        m_pitch = physical_resolution.pitch;
+        if (this == screen_with_cursor) {
+            auto& screen_input = ScreenInput::the();
+            screen_input.set_cursor_location(screen_input.cursor_location().constrained(rect()));
+        }
+    };
+
     if (rc == 0) {
-        on_change_resolution(initial, physical_resolution.pitch, physical_resolution.width, physical_resolution.height, new_scale_factor, screen_with_cursor);
+        on_change_resolution();
         return true;
     }
     if (rc == -1) {
         int err = errno;
-        dbgln("Screen #{}: Failed to set resolution {}x{}: {}", index(), width, height, strerror(err));
-        on_change_resolution(initial, physical_resolution.pitch, physical_resolution.width, physical_resolution.height, new_scale_factor, screen_with_cursor);
+        dbgln("Screen #{}: Failed to set resolution {}: {}", index(), m_info.resolution, strerror(err));
+        on_change_resolution();
         return false;
     }
     VERIFY_NOT_REACHED();
 }
 
-void Screen::on_change_resolution(bool initial, int pitch, int new_physical_width, int new_physical_height, int new_scale_factor, Screen& screen_with_cursor)
-{
-    if (initial || physical_width() != new_physical_width || physical_height() != new_physical_height) {
-        if (m_framebuffer) {
-            size_t previous_size_in_bytes = m_size_in_bytes;
-            int rc = munmap(m_framebuffer, previous_size_in_bytes);
-            VERIFY(rc == 0);
-        }
-
-        int rc = fb_get_size_in_bytes(m_framebuffer_fd, &m_size_in_bytes);
-        VERIFY(rc == 0);
-
-        m_framebuffer = (Gfx::RGBA32*)mmap(nullptr, m_size_in_bytes, PROT_READ | PROT_WRITE, MAP_SHARED, m_framebuffer_fd, 0);
-        VERIFY(m_framebuffer && m_framebuffer != (void*)-1);
-    }
-
-    m_pitch = pitch;
-    m_virtual_rect.set_width(new_physical_width / new_scale_factor);
-    m_virtual_rect.set_height(new_physical_height / new_scale_factor);
-    m_scale_factor = new_scale_factor;
-    update_bounding_rect();
-
-    if (this == &screen_with_cursor) {
-        auto& screen_input = ScreenInput::the();
-        screen_input.set_cursor_location(screen_input.cursor_location().constrained(rect()));
-    }
-}
-
 void Screen::set_buffer(int index)
 {
     VERIFY(m_can_set_buffer);

Userland/Services/WindowServer/Screen.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include "ScreenLayout.h"
 #include <AK/NonnullOwnPtrVector.h>
 #include <Kernel/API/KeyCode.h>
 #include <LibGfx/Color.h>
@@ -72,6 +73,9 @@ public:
     }
     ~Screen();
 
+    static bool apply_layout(ScreenLayout&&, String&);
+    static const ScreenLayout& layout() { return s_layout; }
+
     static Screen& main()
     {
         VERIFY(s_main_screen);
@@ -124,11 +128,6 @@ public:
     void make_main_screen() { s_main_screen = this; }
     bool is_main_screen() const { return s_main_screen == this; }
 
-    template<typename... Args>
-    bool set_resolution(Args&&... args)
-    {
-        return do_set_resolution(false, forward<Args>(args)...);
-    }
     bool can_set_buffer() { return m_can_set_buffer; }
     void set_buffer(int index);
 
@@ -138,7 +137,7 @@ public:
 
     int width() const { return m_virtual_rect.width(); }
     int height() const { return m_virtual_rect.height(); }
-    int scale_factor() const { return m_scale_factor; }
+    int scale_factor() const { return m_info.scale_factor; }
 
     Gfx::RGBA32* scanline(int y);
 
@@ -148,9 +147,11 @@ public:
     Gfx::IntRect rect() const { return m_virtual_rect; }
 
 private:
-    Screen(const String& device, const Gfx::IntRect& virtual_rect, int scale_factor);
+    Screen(ScreenLayout::Screen&);
+    bool open_device();
+    void close_device();
     void init();
-    bool do_set_resolution(bool initial, int width, int height, int scale_factor);
+    bool set_resolution(bool initial);
     static void update_indices()
     {
         for (size_t i = 0; i < s_screens.size(); i++)
@@ -160,11 +161,10 @@ private:
 
     bool is_opened() const { return m_framebuffer_fd >= 0; }
 
-    void on_change_resolution(bool initial, int pitch, int physical_width, int physical_height, int scale_factor, Screen& screen_with_cursor);
-
     static NonnullOwnPtrVector<Screen, default_screen_count> s_screens;
     static Screen* s_main_screen;
     static Gfx::IntRect s_bounding_screens_rect;
+    static ScreenLayout s_layout;
     size_t m_index { 0 };
 
     size_t m_size_in_bytes;
@@ -175,7 +175,8 @@ private:
     int m_pitch { 0 };
     Gfx::IntRect m_virtual_rect;
     int m_framebuffer_fd { -1 };
-    int m_scale_factor { 1 };
+
+    ScreenLayout::Screen& m_info;
 };
 
 inline Gfx::RGBA32* Screen::scanline(int y)

Userland/Services/WindowServer/ScreenLayout.cpp

@@ -0,0 +1,7 @@
+/*
+ * Copyright (c) 2020, the SerenityOS developers.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include "ScreenLayout.ipp"

Userland/Services/WindowServer/ScreenLayout.h

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2020, the SerenityOS developers.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/String.h>
+#include <AK/Vector.h>
+#include <LibCore/ConfigFile.h>
+#include <LibGfx/Rect.h>
+#include <LibGfx/Size.h>
+#include <LibIPC/Forward.h>
+
+namespace WindowServer {
+
+class ScreenLayout {
+public:
+    struct Screen {
+        String device;
+        Gfx::IntPoint location;
+        Gfx::IntSize resolution;
+        int scale_factor;
+
+        Gfx::IntRect virtual_rect() const
+        {
+            return { location, { resolution.width() / scale_factor, resolution.height() / scale_factor } };
+        }
+
+        auto operator<=>(const Screen&) const = default;
+    };
+
+    Vector<Screen> screens;
+    unsigned main_screen_index { 0 };
+
+    bool is_valid(String* error_msg = nullptr) const;
+    void normalize();
+    bool load_config(const Core::ConfigFile& config_file, String* error_msg = nullptr);
+    bool save_config(Core::ConfigFile& config_file, bool sync = true) const;
+
+    // TODO: spaceship operator
+    bool operator!=(const ScreenLayout& other) const;
+    bool operator==(const ScreenLayout& other) const
+    {
+        return !(*this != other);
+    }
+};
+
+}
+
+namespace IPC {
+
+bool encode(Encoder&, const WindowServer::ScreenLayout::Screen&);
+bool decode(Decoder&, WindowServer::ScreenLayout::Screen&);
+bool encode(Encoder&, const WindowServer::ScreenLayout&);
+bool decode(Decoder&, WindowServer::ScreenLayout&);
+
+}

Userland/Services/WindowServer/ScreenLayout.ipp

@@ -0,0 +1,232 @@
+/*
+ * Copyright (c) 2020, the SerenityOS developers.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <Services/WindowServer/ScreenLayout.h>
+
+// Must be included after LibIPC/Forward.h
+#include <LibIPC/Decoder.h>
+#include <LibIPC/Encoder.h>
+
+namespace WindowServer {
+
+bool ScreenLayout::is_valid(String* error_msg) const
+{
+    if (screens.is_empty()) {
+        if (error_msg)
+            *error_msg = "Must have at least one screen";
+        return false;
+    }
+    if (main_screen_index >= screens.size()) {
+        if (error_msg)
+            *error_msg = String::formatted("Invalid main screen index: {}", main_screen_index);
+        return false;
+    }
+    int smallest_x = 0;
+    int smallest_y = 0;
+    for (size_t i = 0; i < screens.size(); i++) {
+        auto& screen = screens[i];
+        if (screen.device.is_null() || screen.device.is_empty()) {
+            if (error_msg)
+                *error_msg = String::formatted("Screen #{} has no path", i);
+            return false;
+        }
+        for (size_t j = 0; j < screens.size(); j++) {
+            auto& other_screen = screens[j];
+            if (&other_screen == &screen)
+                continue;
+            if (screen.device == other_screen.device) {
+                if (error_msg)
+                    *error_msg = String::formatted("Screen #{} is using same device as screen #{}", i, j);
+                return false;
+            }
+            if (screen.virtual_rect().intersects(other_screen.virtual_rect())) {
+                if (error_msg)
+                    *error_msg = String::formatted("Screen #{} overlaps with screen #{}", i, j);
+                return false;
+            }
+        }
+        if (screen.location.x() < 0 || screen.location.y() < 0) {
+            if (error_msg)
+                *error_msg = String::formatted("Screen #{} has invalid location: {}", i, screen.location);
+            return false;
+        }
+        if (screen.resolution.width() <= 0 || screen.resolution.height() <= 0) {
+            if (error_msg)
+                *error_msg = String::formatted("Screen #{} has invalid resolution: {}", i, screen.resolution);
+            return false;
+        }
+        if (screen.scale_factor < 1) {
+            if (error_msg)
+                *error_msg = String::formatted("Screen #{} has invalid scale factor: {}", i, screen.scale_factor);
+            return false;
+        }
+        if (i == 0 || screen.location.x() < smallest_x)
+            smallest_x = screen.location.x();
+        if (i == 0 || screen.location.y() < smallest_y)
+            smallest_y = screen.location.y();
+    }
+    if (smallest_x != 0 || smallest_y != 0) {
+        if (error_msg)
+            *error_msg = "Screen layout has not been normalized";
+        return false;
+    }
+    Vector<const Screen*, 16> reachable_screens { &screens[main_screen_index] };
+    bool did_reach_another_screen;
+    do {
+        did_reach_another_screen = false;
+        auto* latest_reachable_screen = reachable_screens[reachable_screens.size() - 1];
+        for (auto& screen : screens) {
+            if (&screen == latest_reachable_screen || reachable_screens.contains_slow(&screen))
+                continue;
+            if (screen.virtual_rect().is_adjacent(latest_reachable_screen->virtual_rect())) {
+                reachable_screens.append(&screen);
+                did_reach_another_screen = true;
+                break;
+            }
+        }
+    } while (did_reach_another_screen);
+    if (reachable_screens.size() != screens.size()) {
+        for (size_t i = 0; i < screens.size(); i++) {
+            auto& screen = screens[i];
+            if (!reachable_screens.contains_slow(&screen)) {
+                if (error_msg)
+                    *error_msg = String::formatted("Screen #{} {} cannot be reached from main screen #{} {}", i, screen.virtual_rect(), main_screen_index, screens[main_screen_index].virtual_rect());
+                break;
+            }
+        }
+        return false;
+    }
+    return true;
+}
+
+void ScreenLayout::normalize()
+{
+    int smallest_x = 0;
+    int smallest_y = 0;
+    for (size_t i = 0; i < screens.size(); i++) {
+        auto& screen = screens[i];
+        if (i == 0 || screen.location.x() < smallest_x)
+            smallest_x = screen.location.x();
+        if (i == 0 || screen.location.y() < smallest_y)
+            smallest_y = screen.location.y();
+    }
+    if (smallest_x != 0 || smallest_y != 0) {
+        for (auto& screen : screens)
+            screen.location.translate_by(-smallest_x, -smallest_y);
+    }
+}
+
+bool ScreenLayout::load_config(const Core::ConfigFile& config_file, String* error_msg)
+{
+    screens.clear_with_capacity();
+    main_screen_index = config_file.read_num_entry("Screens", "DefaultScreen", 0);
+    for (size_t index = 0;; index++) {
+        auto group_name = String::formatted("Screen{}", index);
+        if (!config_file.has_group(group_name))
+            break;
+        screens.append({ config_file.read_entry(group_name, "Device"),
+            { config_file.read_num_entry(group_name, "Left"), config_file.read_num_entry(group_name, "Top") },
+            { config_file.read_num_entry(group_name, "Width"), config_file.read_num_entry(group_name, "Height") },
+            config_file.read_num_entry(group_name, "ScaleFactor", 1) });
+    }
+    if (!is_valid(error_msg)) {
+        *this = {};
+        return false;
+    }
+    return true;
+}
+
+bool ScreenLayout::save_config(Core::ConfigFile& config_file, bool sync) const
+{
+    config_file.write_num_entry("Screens", "DefaultScreen", main_screen_index);
+
+    size_t index = 0;
+    while (index < screens.size()) {
+        auto& screen = screens[index];
+        auto group_name = String::formatted("Screen{}", index);
+        config_file.write_entry(group_name, "Device", screen.device);
+        config_file.write_num_entry(group_name, "Left", screen.location.x());
+        config_file.write_num_entry(group_name, "Top", screen.location.y());
+        config_file.write_num_entry(group_name, "Width", screen.resolution.width());
+        config_file.write_num_entry(group_name, "Height", screen.resolution.height());
+        config_file.write_num_entry(group_name, "ScaleFactor", screen.scale_factor);
+        index++;
+    }
+    // Prune screens no longer in the layout
+    for (;;) {
+        auto group_name = String::formatted("Screen{}", index++);
+        if (!config_file.has_group(group_name))
+            break;
+        config_file.remove_group(group_name);
+    }
+
+    if (sync && !config_file.sync())
+        return false;
+    return true;
+}
+
+bool ScreenLayout::operator!=(const ScreenLayout& other) const
+{
+    if (this == &other)
+        return false;
+    if (main_screen_index != other.main_screen_index)
+        return true;
+    if (screens.size() != other.screens.size())
+        return true;
+    for (size_t i = 0; i < screens.size(); i++) {
+        if (screens[i] != other.screens[i])
+            return true;
+    }
+    return false;
+}
+
+}
+
+namespace IPC {
+
+bool encode(Encoder& encoder, const WindowServer::ScreenLayout::Screen& screen)
+{
+    encoder << screen.device << screen.location << screen.resolution << screen.scale_factor;
+    return true;
+}
+
+bool decode(Decoder& decoder, WindowServer::ScreenLayout::Screen& screen)
+{
+    String device;
+    if (!decoder.decode(device))
+        return false;
+    Gfx::IntPoint location;
+    if (!decoder.decode(location))
+        return false;
+    Gfx::IntSize resolution;
+    if (!decoder.decode(resolution))
+        return false;
+    int scale_factor = 0;
+    if (!decoder.decode(scale_factor))
+        return false;
+    screen = { device, location, resolution, scale_factor };
+    return true;
+}
+
+bool encode(Encoder& encoder, const WindowServer::ScreenLayout& screen_layout)
+{
+    encoder << screen_layout.screens << screen_layout.main_screen_index;
+    return true;
+}
+
+bool decode(Decoder& decoder, WindowServer::ScreenLayout& screen_layout)
+{
+    Vector<WindowServer::ScreenLayout::Screen> screens;
+    if (!decoder.decode(screens))
+        return false;
+    unsigned main_screen_index = 0;
+    if (!decoder.decode(main_screen_index))
+        return false;
+    screen_layout = { move(screens), main_screen_index };
+    return true;
+}
+
+}

Userland/Services/WindowServer/WindowManager.cpp

@@ -93,50 +93,42 @@ Gfx::Font const& WindowManager::window_title_font() const
     return Gfx::FontDatabase::default_font().bold_variant();
 }
 
-bool WindowManager::set_resolution(Screen& screen, int width, int height, int scale)
+bool WindowManager::set_screen_layout(ScreenLayout&& screen_layout, bool save, String& error_msg)
 {
-    auto screen_rect = screen.rect();
-    if (screen_rect.width() == width && screen_rect.height() == height && screen.scale_factor() == scale)
-        return true;
-
-    // Make sure it's impossible to set an invalid resolution
-    if (!(width >= 640 && height >= 480 && scale >= 1)) {
-        dbgln("Compositor: Tried to set invalid resolution: {}x{}", width, height);
+    if (!Screen::apply_layout(move(screen_layout), error_msg))
         return false;
-    }
 
-    auto old_scale_factor = screen.scale_factor();
-    bool success = screen.set_resolution(width, height, scale);
-    if (success && old_scale_factor != scale)
-        reload_icon_bitmaps_after_scale_change();
+    reload_icon_bitmaps_after_scale_change();
 
     Compositor::the().screen_resolution_changed();
 
     ClientConnection::for_each_client([&](ClientConnection& client) {
         client.notify_about_new_screen_rects(Screen::rects(), Screen::main().index());
     });
-    if (success) {
-        m_window_stack.for_each_window([](Window& window) {
-            window.recalculate_rect();
-            return IterationDecision::Continue;
-        });
+
+    m_window_stack.for_each_window([](Window& window) {
+        window.screens().clear_with_capacity();
+        window.recalculate_rect();
+        return IterationDecision::Continue;
+    });
+
+    if (save)
+        Screen::layout().save_config(*m_config);
+    return true;
+}
+
+ScreenLayout WindowManager::get_screen_layout() const
+{
+    return Screen::layout();
+}
+
+bool WindowManager::save_screen_layout(String& error_msg)
+{
+    if (!Screen::layout().save_config(*m_config)) {
+        error_msg = "Could not save";
+        return false;
     }
-    if (m_config) {
-        if (success) {
-            dbgln("Saving resolution: {} @ {}x to config file at {}", Gfx::IntSize(width, height), scale, m_config->filename());
-            m_config->write_num_entry("Screen", "Width", width);
-            m_config->write_num_entry("Screen", "Height", height);
-            m_config->write_num_entry("Screen", "ScaleFactor", scale);
-            m_config->sync();
-        } else {
-            dbgln("Saving fallback resolution: {} @1x to config file at {}", screen.size(), m_config->filename());
-            m_config->write_num_entry("Screen", "Width", screen.size().width());
-            m_config->write_num_entry("Screen", "Height", screen.size().height());
-            m_config->write_num_entry("Screen", "ScaleFactor", 1);
-            m_config->sync();
-        }
-    }
-    return success;
+    return true;
 }
 
 void WindowManager::set_acceleration_factor(double factor)

Userland/Services/WindowServer/WindowManager.h

@@ -21,6 +21,7 @@
 #include <WindowServer/Event.h>
 #include <WindowServer/MenuManager.h>
 #include <WindowServer/Menubar.h>
+#include <WindowServer/ScreenLayout.h>
 #include <WindowServer/WMClientConnection.h>
 #include <WindowServer/WindowSwitcher.h>
 #include <WindowServer/WindowType.h>
@@ -129,7 +130,9 @@ public:
     Gfx::Font const& font() const;
     Gfx::Font const& window_title_font() const;
 
-    bool set_resolution(Screen&, int width, int height, int scale);
+    bool set_screen_layout(ScreenLayout&&, bool, String&);
+    ScreenLayout get_screen_layout() const;
+    bool save_screen_layout(String&);
 
     void set_acceleration_factor(double);
     void set_scroll_step_size(unsigned);

Userland/Services/WindowServer/WindowServer.ipc

@@ -93,7 +93,10 @@ endpoint WindowServer
     set_background_color(String background_color) =|
     set_wallpaper_mode(String mode) =|
 
-    set_resolution(u32 screen_index, Gfx::IntSize resolution, int scale_factor) => (bool success, Gfx::IntSize resolution, int scale_factor)
+    set_screen_layout(::WindowServer::ScreenLayout screen_layout, bool save) => (bool success, String error_msg)
+    get_screen_layout() => (::WindowServer::ScreenLayout screen_layout)
+    save_screen_layout() => (bool success, String error_msg)
+
     set_window_icon_bitmap(i32 window_id, Gfx::ShareableBitmap icon) =|
 
     get_wallpaper() => (String path)

Userland/Services/WindowServer/main.cpp

@@ -76,84 +76,26 @@ int main(int, char**)
     }
 
     // First check which screens are explicitly configured
-    AK::HashTable<String> fb_devices_configured;
-    int main_screen_index = wm_config->read_num_entry("Screens", "MainScreen", 0);
-    for (int screen_index = 0;; screen_index++) {
-        auto group_name = String::formatted("Screen{}", screen_index);
-        if (!wm_config->has_group(group_name))
-            break;
-
-        int scale = wm_config->read_num_entry(group_name, "ScaleFactor", 1);
-        auto device_path = wm_config->read_entry(group_name, "Device", {});
-        if (device_path.is_null() || device_path.is_empty()) {
-            dbgln("Screen {} misses Device setting", screen_index);
-            break;
-        }
-
-        Gfx::IntRect virtual_rect {
-            wm_config->read_num_entry(group_name, "Left", 0 / scale),
-            wm_config->read_num_entry(group_name, "Top", 0 / scale),
-            wm_config->read_num_entry(group_name, "Width", 1024 / scale),
-            wm_config->read_num_entry("Screen", "Height", 768 / scale)
-        };
-
-        // Check if the screen would be overlapping with another one
-        if (WindowServer::Screen::for_each([&](auto& screen) {
-                if (screen.rect().intersects(virtual_rect)) {
-                    dbgln("Screen {} rect {} overlaps with screen {} rect {}: Ignoring configuration", screen.index(), screen.rect(), screen_index, virtual_rect);
-                    return IterationDecision::Break;
-                }
-                return IterationDecision::Continue;
-            })
-            == IterationDecision::Break) {
-            // Ignore the configuration
-            break;
-        }
-
-        auto* screen = WindowServer::Screen::create(device_path, virtual_rect, scale);
-        if (!screen) {
-            dbgln("Screen {} failed to be created", screen_index);
-            break;
-        }
-
-        if (main_screen_index == screen_index)
-            screen->make_main_screen();
-
-        // Remember that we used this device for a screen already
-        fb_devices_configured.set(device_path);
-    }
-
-    // Check that all screens are contiguous and can be "reached" from the main screen
     {
-        Vector<WindowServer::Screen*, 16> reachable_screens { &WindowServer::Screen::main() };
-        bool did_reach_another_screen;
-        do {
-            did_reach_another_screen = false;
-            auto* latest_reachable_screen = reachable_screens[reachable_screens.size() - 1];
-            WindowServer::Screen::for_each([&](auto& screen) {
-                if (&screen == latest_reachable_screen || reachable_screens.contains_slow(&screen))
-                    return IterationDecision::Continue;
-                if (screen.rect().is_adjacent(latest_reachable_screen->rect())) {
-                    reachable_screens.append(&screen);
-                    did_reach_another_screen = true;
-                    return IterationDecision::Break;
-                }
-                return IterationDecision::Continue;
-            });
-        } while (did_reach_another_screen);
-        if (reachable_screens.size() != WindowServer::Screen::count()) {
-            WindowServer::Screen::for_each([&](auto& screen) {
-                if (!reachable_screens.contains_slow(&screen))
-                    dbgln("Screen {} cannot be reached from main screen {}: Bad configuration!", screen.index(), WindowServer::Screen::main().index());
-                return IterationDecision::Continue;
-            });
-            dbgln("At least one screen is not adjacent to another screen, exiting!");
+        AK::HashTable<String> fb_devices_configured;
+        WindowServer::ScreenLayout screen_layout;
+        String error_msg;
+        if (!screen_layout.load_config(*wm_config, &error_msg)) {
+            dbgln("Error loading screen configuration: {}", error_msg);
+            return 1;
+        }
+
+        for (auto& screen_info : screen_layout.screens)
+            fb_devices_configured.set(screen_info.device);
+
+        // TODO: Enumerate the /dev/fbX devices and set up any ones we find that we haven't already used
+
+        if (!WindowServer::Screen::apply_layout(move(screen_layout), error_msg)) {
+            dbgln("Error applying screen layout: {}", error_msg);
             return 1;
         }
     }
 
-    // TODO: Enumerate the /dev/fbX devices and set up any ones we find that we haven't already used
-
     auto& screen_input = WindowServer::ScreenInput::the();
     screen_input.set_cursor_location(WindowServer::Screen::main().rect().center());
     screen_input.set_acceleration_factor(atof(wm_config->read_entry("Mouse", "AccelerationFactor", "1.0").characters()));
@@ -169,10 +111,9 @@ int main(int, char**)
         return 1;
     }
 
-    if (unveil("/dev", "") < 0) {
-        perror("unveil /dev");
-        return 1;
-    }
+    // NOTE: Because we dynamically need to be able to open new /dev/fb*
+    // devices we can't really unveil all of /dev unless we have some
+    // other mechanism that can hand us file descriptors for these.
 
     if (unveil(nullptr, nullptr) < 0) {
         perror("unveil");

Userland/Utilities/chres.cpp

@@ -26,9 +26,16 @@ int main(int argc, char** argv)
     // A Core::EventLoop is all we need, but WindowServerConnection needs a full Application object.
     char* dummy_argv[] = { argv[0] };
     auto app = GUI::Application::construct(1, dummy_argv);
-    auto result = GUI::WindowServerConnection::the().set_resolution(screen, Gfx::IntSize { width, height }, scale);
-    if (!result.success()) {
-        warnln("failed to set resolution");
+    auto screen_layout = GUI::WindowServerConnection::the().get_screen_layout();
+    if (screen < 0 || (size_t)screen >= screen_layout.screens.size()) {
+        warnln("invalid screen index: {}", screen);
+        return 1;
+    }
+    auto& main_screen = screen_layout.screens[screen];
+    main_screen.resolution = { width, height };
+    auto set_result = GUI::WindowServerConnection::the().set_screen_layout(screen_layout, true);
+    if (!set_result.success()) {
+        warnln("failed to set resolution: {}", set_result.error_msg());
         return 1;
     }
 }