From 670df31cc6891def67f92fff667d01d56690690a Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Sat, 31 Oct 2026 08:28:48 +0100 Subject: [PATCH] fix(wayland): open windows on compositors without xdg-decoration, viewporter or fractional scale Window - The constructor made a fractional scale, a toplevel decoration and a viewport from Device::fractionalScaleManager, Device::manager and Device::wpViewporter without looking at them. Device::handle_global binds those globals only when the compositor advertises them, and Initialize requires only wl_shm, wl_compositor and xdg_wm_base, so on a compositor without one of the three (GNOME's Mutter has no xdg-decoration) the constructor handed libwayland a null proxy and crashed before the window existed (#162). Each object is now made only when its global is there, which Resize and the destructor already assumed. - The fractional scale needs the viewporter too: the preferred scale is applied through the viewport, which shows the buffer's physical pixels at width/scale x height/scale. A surface without a viewport is as large as its buffer, so there the scale stays 1, which keeps pointer and touch positions, configure sizes and the text-input caret, all converted by it, in physical pixels; the compositor scales the window up on a HiDPI output. A scale that changes after the constructor is still applied only by the next resize, which is #165. - Without xdg-decoration the window has no title bar, since the library draws no client-side decorations. Device - The optional globals say they are optional, and that a request on a null proxy crashes. Tests - WindowOptionalGlobals: hides xdg-decoration, wp_viewporter and wp_fractional_scale_v1 from the Window one at a time and all at once, by nulling the Device statics after Initialize, and renders, resizes and closes a window each time without a protocol error. Before the fix it crashed on the window without xdg-decoration. A global the compositor does not offer is reported as a SKIP line, since the path that uses it goes untested there. README - Which Wayland protocols the window needs, and what it does without the optional ones. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 +- implementations/Crafter.Graphics-Window.cpp | 30 +++- interfaces/Crafter.Graphics-Device.cppm | 3 + project.cpp | 7 + tests/WindowOptionalGlobals/main.cpp | 143 ++++++++++++++++++++ 5 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 tests/WindowOptionalGlobals/main.cpp diff --git a/README.md b/README.md index c4cafac..820bcba 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,13 @@ shaded through an intersection shader with an any-hit cut-out. framing, input events. Pick a backend at build time via the target triple. The DOM backend routes every dynamic symbol through [additional/dom-env.js](additional/dom-env.js) and - [additional/dom-webgpu.js](additional/dom-webgpu.js). + [additional/dom-webgpu.js](additional/dom-webgpu.js). On Wayland the + window needs `wl_compositor`, `wl_shm` and `xdg_wm_base`, and uses + `xdg-decoration`, `wp_viewporter` and `wp_fractional_scale_v1` where the + compositor has them. Without xdg-decoration (GNOME) the window has no + title bar, as the library draws no client-side decorations; when either + of the other two is missing it draws at scale 1, and the compositor + scales it up on a HiDPI output. - **Device** *(Vulkan only)* — single-instance bring-up targeting `VK_EXT_descriptor_heap`; pipelines are created with `VK_PIPELINE_CREATE_2_DESCRIPTOR_HEAP_BIT_EXT` so there are no diff --git a/implementations/Crafter.Graphics-Window.cpp b/implementations/Crafter.Graphics-Window.cpp index 0cf2ed6..3339e5f 100644 --- a/implementations/Crafter.Graphics-Window.cpp +++ b/implementations/Crafter.Graphics-Window.cpp @@ -364,18 +364,36 @@ Window::Window(std::uint32_t width, std::uint32_t height) : width(width), height xdg_toplevel_add_listener(xdgToplevel, &xdg_toplevel_listener, this); wl_surface_commit(surface); - wp_scale = wp_fractional_scale_manager_v1_get_fractional_scale(Device::fractionalScaleManager, surface); - wp_fractional_scale_v1_add_listener(wp_scale, &wp_fractional_scale_v1_listener, this); + // The fractional scale, the decoration and the viewport come from + // optional protocols: Device::handle_global binds a global only when the + // compositor advertises it, and a request on the null proxy it leaves + // otherwise crashes in libwayland (#162). The preferred scale is applied + // through the viewport, which shows the width x height physical pixels of + // the buffer at width/scale x height/scale, so without a viewporter there + // is no fractional scale either: scale stays 1, the surface is as large as + // its buffer, and the compositor scales it up on a HiDPI output. A scale + // that changes after this is applied only by the next resize (#165). + if (Device::fractionalScaleManager != nullptr && Device::wpViewporter != nullptr) { + wp_scale = wp_fractional_scale_manager_v1_get_fractional_scale(Device::fractionalScaleManager, surface); + wp_fractional_scale_v1_add_listener(wp_scale, &wp_fractional_scale_v1_listener, this); + } while (wl_display_dispatch(Device::display) != -1 && !configured) {} wl_surface_commit(surface); - decoration = zxdg_decoration_manager_v1_get_toplevel_decoration(Device::manager, xdgToplevel); - zxdg_toplevel_decoration_v1_set_mode(decoration, ZXDG_TOPLEVEL_DECORATION_V1_MODE_SERVER_SIDE); + // GNOME's Mutter has no xdg-decoration: it leaves decorations to the + // client, and the library draws none, so there the window has no title + // bar. + if (Device::manager != nullptr) { + decoration = zxdg_decoration_manager_v1_get_toplevel_decoration(Device::manager, xdgToplevel); + zxdg_toplevel_decoration_v1_set_mode(decoration, ZXDG_TOPLEVEL_DECORATION_V1_MODE_SERVER_SIDE); + } - wpViewport = wp_viewporter_get_viewport(Device::wpViewporter, surface); - wp_viewport_set_destination(wpViewport, std::ceil(width/scale), std::ceil(height/scale)); + if (Device::wpViewporter != nullptr) { + wpViewport = wp_viewporter_get_viewport(Device::wpViewporter, surface); + wp_viewport_set_destination(wpViewport, std::ceil(width/scale), std::ceil(height/scale)); + } wl_surface_commit(surface); #endif diff --git a/interfaces/Crafter.Graphics-Device.cppm b/interfaces/Crafter.Graphics-Device.cppm index 3fef58c..bf89c20 100644 --- a/interfaces/Crafter.Graphics-Device.cppm +++ b/interfaces/Crafter.Graphics-Device.cppm @@ -89,6 +89,9 @@ export namespace Crafter { inline static wl_display* display = nullptr; inline static wl_seat* seat = nullptr; inline static xdg_wm_base* xdgWmBase = nullptr; + // Optional protocols, like wpViewporter below: null on a compositor + // that does not advertise them (GNOME has no xdg-decoration), and a + // request on a null proxy crashes in libwayland, so check first. inline static wp_fractional_scale_manager_v1* fractionalScaleManager = nullptr; inline static zxdg_decoration_manager_v1* manager = nullptr; inline static xkb_keymap* xkb_keymap; diff --git a/project.cpp b/project.cpp index 59a8f56..d7e14c5 100644 --- a/project.cpp +++ b/project.cpp @@ -427,6 +427,13 @@ extern "C" Configuration CrafterBuildProject(std::span a // the Wayland backend and a real compositor. cfg.tests.push_back(LibraryTest("WindowUpdateClock")); + // Issue #162: xdg-decoration, wp_viewporter and + // wp_fractional_scale_v1 are optional (GNOME has no + // xdg-decoration). Hides each from the Window in turn, and all + // three, and renders, resizes and closes a window each time. + // Needs the Wayland backend and a real compositor. + cfg.tests.push_back(LibraryTest("WindowOptionalGlobals")); + // RTPass pushes the application's push data and traces at an // explicit size: clears the frame, traces a quadrant with a hit // colour that changes every frame, and checks the read-back. diff --git a/tests/WindowOptionalGlobals/main.cpp b/tests/WindowOptionalGlobals/main.cpp new file mode 100644 index 0000000..e873c17 --- /dev/null +++ b/tests/WindowOptionalGlobals/main.cpp @@ -0,0 +1,143 @@ +//SPDX-License-Identifier: LGPL-3.0-only +//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +// Issue #162: xdg-decoration, wp_viewporter and wp_fractional_scale_v1 are +// optional, and a compositor may offer any of them or none (GNOME's Mutter +// has no xdg-decoration). Device::handle_global binds only what is +// advertised, but the Window constructor used all three unconditionally, so +// without one it handed libwayland a null proxy and crashed. This makes a +// window without each of the three, and without all of them, and checks for +// each: +// +// - the constructor returns, with the decoration, the viewport and the +// fractional scale made exactly when their globals are there; the +// fractional scale also needs the viewporter, since the preferred scale +// is applied through the viewport; +// - without a fractional scale the scale stays 1: the surface is then as +// large as its buffer, so surface-local pointer positions and configure +// sizes are already physical pixels; +// - the window renders, resizes, renders again and closes, and the +// compositor raises no protocol error over any of it. Resize sets the +// viewport's destination only when there is a viewport; the new size +// itself is not checked, since Resize stores it before it does anything +// else. +// +// The test hides the globals the compositor offers: it nulls the Device +// statics between Initialize and the Window, which is the state a compositor +// without them leaves. The proxies stay bound, unused. A window with every +// global the compositor offers is the control. A global it does not offer +// leaves the path that uses it untested, which the test reports as a SKIP +// line rather than a failure: the paths without it, which are what #162 is +// about, still run. +// +// Needs a live Wayland compositor and a Vulkan device. The validation check +// only means something with CRAFTER_GRAPHICS_VALIDATION set. + +#include +#include +#include "vulkan/vulkan.h" + +import Crafter.Graphics; +import std; +using namespace Crafter; + +namespace { + +int failures = 0; + +void Check(bool ok, std::string_view what) { + std::println("{} {}", ok ? "PASS" : "FAIL", what); + if (!ok) ++failures; +} + +// Which optional globals a window is made with. +struct Case { + std::string_view name; + bool decoration; + bool viewporter; + bool fractionalScale; +}; + +constexpr std::array kCases = { + Case{ "with every global", true, true, true }, + Case{ "without xdg-decoration", false, true, true }, + Case{ "without wp_viewporter", true, false, true }, + Case{ "without wp_fractional_scale_v1", true, true, false }, + Case{ "without any of the three", false, false, false }, +}; + +// A roundtrip fails, and the display keeps the error, once the compositor +// has raised a protocol error over anything sent before it. +bool NoProtocolError() { + return wl_display_roundtrip(Device::display) != -1 && wl_display_get_error(Device::display) == 0; +} + +} // namespace + +int main() { + Device::Initialize(); + Device::validationErrorCount = 0; + + const auto decorationManager = Device::manager; + const auto viewporter = Device::wpViewporter; + const auto fractionalScaleManager = Device::fractionalScaleManager; + std::println("the compositor offers xdg-decoration: {}, wp_viewporter: {}, wp_fractional_scale_v1: {}", + decorationManager != nullptr, viewporter != nullptr, fractionalScaleManager != nullptr); + const auto skipWithout = [](bool offered, std::string_view global) { + if (!offered) std::println("SKIP the compositor offers no {}, so no window here uses one", global); + }; + skipWithout(decorationManager != nullptr, "xdg-decoration"); + skipWithout(viewporter != nullptr, "wp_viewporter"); + skipWithout(fractionalScaleManager != nullptr && viewporter != nullptr, "wp_fractional_scale_v1 with wp_viewporter"); + + for (const Case& c : kCases) { + Device::manager = c.decoration ? decorationManager : nullptr; + Device::wpViewporter = c.viewporter ? viewporter : nullptr; + Device::fractionalScaleManager = c.fractionalScale ? fractionalScaleManager : nullptr; + const bool decorated = Device::manager != nullptr; + const bool viewport = Device::wpViewporter != nullptr; + const bool fractional = viewport && Device::fractionalScaleManager != nullptr; + + bool connected; + { + Window window(320, 240, "WindowOptionalGlobals test"); + Check((window.decoration != nullptr) == decorated, + std::format("{}: a decoration exactly when xdg-decoration is there", c.name)); + Check((window.wpViewport != nullptr) == viewport, + std::format("{}: a viewport exactly when wp_viewporter is there", c.name)); + Check((window.wp_scale != nullptr) == fractional, + std::format("{}: a fractional scale exactly when it and wp_viewporter are there", c.name)); + if (!fractional) { + Check(window.scale == 1.0f, std::format("{}: the scale stays 1 without a fractional scale", c.name)); + } + + for (int i = 0; i < Window::numFrames * 2; ++i) window.Render(); + window.Resize(400, 300); + for (int i = 0; i < Window::numFrames * 2; ++i) window.Render(); + Device::CheckVkResult(vkQueueWaitIdle(Device::queue)); + connected = NoProtocolError(); + Check(connected, std::format("{}: no protocol error while rendering and resizing", c.name)); + } + if (connected) { + connected = NoProtocolError(); + Check(connected, std::format("{}: no protocol error closing the window", c.name)); + } + // A display that raised an error is gone, and so is every later case. + if (!connected) break; + } + + Device::manager = decorationManager; + Device::wpViewporter = viewporter; + Device::fractionalScaleManager = fractionalScaleManager; + + if (std::getenv("CRAFTER_GRAPHICS_VALIDATION") != nullptr) { + Check(Device::validationErrorCount == 0, + std::format("no Vulkan validation errors ({} seen)", Device::validationErrorCount)); + } + if (failures != 0) { + std::println("{} check(s) failed", failures); + return EXIT_FAILURE; + } + std::println("all checks passed"); + return EXIT_SUCCESS; +}