diff --git a/WL_DECOMPOSITION_PLAN.md b/WL_DECOMPOSITION_PLAN.md new file mode 100644 index 0000000..570d7ea --- /dev/null +++ b/WL_DECOMPOSITION_PLAN.md @@ -0,0 +1,354 @@ +# wl_backend.c Decomposition Plan + +## Overview + +`src/backend/wayland/wl_backend.c` is 2329 lines containing 6 unrelated functional +areas that have accumulated in a single file. This plan decomposes it into +focused modules, each of which can be carried out as an independent commit. + +The target is to reduce `wl_backend.c` to ~900 lines containing only the +compositor lifecycle (open/close/run/terminate) and the small protocol handlers +that are tightly coupled to it. + +--- + +## Pre-requisites + +- Build system: `src/Makefile.am` (lines 164-209) lists all `backend/wayland/*.c` + files under `wmaker_SOURCES`. Any new `.c` file must be added there. +- After editing `Makefile.am`, run `autoreconf -fi && ./configure ` to + regenerate `Makefile.in` and `Makefile`. +- All wayland backend files share state through the global `wl_state` struct + declared in `wl_types.h` and defined in `wl_backend.c`. +- Forward declarations use `extern` for functions defined in other `.c` files. + Functions that become file-local after extraction should be marked `static`. + +--- + +## Step 1 — Remove duplicate layer shell code (delete ~180 lines) + +**Problem:** `wl_backend.c` lines 1539-1720 contain a `static` copy of the +layer shell implementation. An identical non-static copy exists in +`wl_layer_shell.c` (196 lines, already in `Makefile.am`). The `static` +qualifier on the `wl_backend.c` copy means it shadows the `wl_layer_shell.c` +version within this translation unit — the `wl_layer_shell.c` version is +effectively dead code. + +**Functions to delete from `wl_backend.c`:** + +| Function | Lines | Status in wl_backend.c | +|---|---|---| +| `layer_surface_compute_geometry` | 1546-1568 | `static` — duplicate | +| `layer_surface_compute_position` | 1574-1608 | `static` — duplicate | +| `handle_layer_surface_map` | 1610-1618 | `static` — duplicate | +| `handle_layer_surface_unmap` | 1620-1630 | `static` — duplicate | +| `handle_layer_surface_destroy` | 1632-1646 | `static` — duplicate | +| `handle_layer_shell_new_surface` | 1653-1720 | `static` — duplicate | +| `LAYER_VIEW_FROM` macro | 1539-1540 | duplicate of wl_layer_shell.c:14 | + +**Changes:** + +1. In `wl_backend.c`, delete lines 1539-1720 (the `LAYER_VIEW_FROM` macro + through the end of `handle_layer_shell_new_surface`). + +2. In `wl_backend.c`, change the forward declaration at line 222 from: + ```c + static void handle_layer_shell_new_surface(struct wl_listener *listener, void *data); + ``` + to: + ```c + extern void handle_layer_shell_new_surface(struct wl_listener *listener, void *data); + ``` + +3. No `Makefile.am` change needed — `wl_layer_shell.c` is already listed. + +**Verification:** `make clean && make` — the linker resolves +`handle_layer_shell_new_surface` from `wl_layer_shell.o`. Run the compositor +and verify a layer-shell client (e.g. `waybar`, `wlr-randr`) still works. + +--- + +## Step 2 — Extract desktop background to `wl_background.c` (~300 lines) + +**Problem:** `wl_set_desktop_background()` (lines 2034-2287) plus its helper +types and functions (lines 1984-2032) form a self-contained 300-line module +for rendering wallpapers. It has no callers within `wl_backend.c` — it is +called from `defaults.c` via the backend vtable. + +**Functions/types to move:** + +| Symbol | Lines | Visibility | +|---|---|---| +| `struct wl_bg_buffer` | 1987-1990 | file-local struct | +| `bg_buffer_destroy` | 1992-1998 | `static` | +| `bg_buffer_begin_data_ptr_access` | 2000-2009 | `static` | +| `bg_buffer_end_data_ptr_access` | 2011-2014 | `static` | +| `bg_buffer_impl` | 2016-2020 | `static const` | +| `parse_color_to_rcolor` | 2022-2032 | `static` | +| `wl_set_desktop_background` | 2034-2287 | `extern` (vtable) | + +**Steps:** + +1. Create `src/backend/wayland/wl_background.c` with these includes: + ```c + #ifdef HAVE_CONFIG_H + #include "config.h" + #endif + #include + #include + #include + #include + #include + #include + #include + #include + #include + #include + #include "../../screen.h" + #include "../../resources.h" + #include "wl_types.h" + ``` + +2. Move all 7 symbols listed above into the new file. Keep `static` on the + helpers; `wl_set_desktop_background` stays non-static. + +3. Delete lines 1984-2287 from `wl_backend.c`. + +4. In `wl_backend.c`, the `#include` list does not need changes — the function + is resolved at link time. + +5. Add to `src/Makefile.am` inside the `if USE_WAYLAND_BACKEND` block: + ``` + backend/wayland/wl_background.c \ + ``` + +6. Run `autoreconf -fi && ./configure && make`. + +**Verification:** Set a gradient or pixmap background in +`~/GNUstep/Defaults/WindowMaker` and confirm it renders. + +--- + +## Step 3 — Move screen internals to `wl_screen.c` (~235 lines) + +**Problem:** `wl_screen.c` already contains `wl_screen_open`, +`wl_screen_close`, `wl_screen_init_display`, `wl_show_crash_dialog`, +`wl_get_color_for_colormap`, and `wl_parse_color` (244 lines). But the +screen setup functions `wl_screen_alloc_gcs`, `wl_screen_create_internals`, +`wl_screen_create_pixmaps`, `wl_screen_load_tech_font`, and the one-liner +stubs remain in `wl_backend.c`. + +**Functions to move:** + +| Function | Lines | Notes | +|---|---|---| +| `wl_screen_select_input` | 1732 | one-liner stub | +| `wl_screen_set_root_cursor` | 1733 | one-liner stub | +| `wl_screen_set_icon_sizes` | 1734 | one-liner stub | +| `wl_screen_setup_noticeboard` | 1735 | one-liner stub | +| `wl_screen_alloc_gcs` | 1737-1836 | ~100 lines, X11 GC creation | +| `wl_screen_create_internals` | 1838-1966 | ~130 lines, scene setup | +| `wl_screen_create_pixmaps` | 1967 | one-liner stub | +| `wl_screen_load_tech_font` | 1968-1978 | small | +| `wl_screen_show_mini_screenshot` | 1979 | one-liner stub | +| `wl_screen_capture_area` | 1980 | one-liner stub | +| `wl_screen_capture_window` | 1981 | one-liner stub | +| `wl_refresh_desktop` | 1982 | one-liner stub | +| `wl_monitors_query` | 2289-2323 | ~35 lines | +| `wl_monitors_select_events` | 2325 | one-liner stub | + +**Steps:** + +1. Add required includes to `wl_screen.c` (some may already be present): + ```c + #include + #include + #include + #include + #include "../../framewin.h" + #include "../../defaults.h" + #include "../../xmodifier.h" + #include "../../def_pixmaps.h" + ``` + +2. `wl_screen_create_internals` references these external symbols that must + be declared (via `extern` or header include) in `wl_screen.c`: + - `frame_buf_impl` — defined in `wl_framebuf.c`; add `extern const struct wlr_buffer_impl frame_buf_impl;` + - `fb_attach_tree_listener`, `fb_attach_scene_buf_listener` — defined in `wl_framebuf.c` + - `W_RegisterBacking`, `W_UnregisterBacking` — WINGs internals + - `OVL_GH` — defined in `wl_overlay.c` or `wl_types.h` + +3. `wl_screen_alloc_gcs` references `DEF_FRAME_THICKNESS` (from `framewin.h`) + and `WM_EVMASK_*` macros. Ensure these headers are included. + +4. Move all 14 functions listed above from `wl_backend.c` into `wl_screen.c`. + +5. Delete lines 1732-1982 and 2289-2329 from `wl_backend.c`. + +6. No `Makefile.am` change needed — `wl_screen.c` is already listed. + +**Verification:** `make` succeeds. Launch compositor, verify screen +initialization (GCs created, workspace badge visible, dock shadow works). + +--- + +## Step 4 — Extract X11 event bridge to `wl_xwayland.c` (~55 lines) + +**Problem:** `wl_handle_x11_events()` (lines 269-326) translates XEvents from +the XWayland connection into WMEvents. It belongs with the other XWayland +code in `wl_xwayland.c` (429 lines, already in `Makefile.am` under +`USE_XWAYLAND`). + +**Functions to move:** + +| Function | Lines | Notes | +|---|---|---| +| `wl_handle_x11_events` | 269-326 | `static` in wl_backend.c | + +**Steps:** + +1. Move `wl_handle_x11_events` to `wl_xwayland.c`. Remove `static` so it + can be called from `wl_backend.c`. + +2. In `wl_backend.c`, replace the function body with: + ```c + extern int wl_handle_x11_events(int fd, uint32_t mask, void *data); + ``` + (The non-XWAYLAND stub at lines 323-325 stays in `wl_backend.c`.) + +3. Ensure `wl_xwayland.c` includes `` and `"../../event.h"` + for `WMHandleEvent`. + +**Verification:** Launch with XWayland enabled, open an X11 app (e.g. +`xterm`), verify keyboard/mouse events work. + +--- + +## Step 5 — Extract protocol global init from `wl_display_open()` (~240 lines) + +**Problem:** `wl_display_open()` is 670 lines. Lines 578-818 are a linear +sequence of ~25 protocol global creation calls (`wlr_*_create` + +`wl_signal_add`). This block has no control flow dependencies on the +surrounding code beyond `wl_state.display` and `wl_state.seat` being set. + +**Approach:** Extract into a helper function `wl_protocols_init()` in +`wl_extensions.c` (which currently contains only stubs and error handlers — +172 lines). + +**Steps:** + +1. In `wl_extensions.c`, add a new function: + ```c + void wl_protocols_init(void) + ``` + that contains the protocol global creation block (lines 578-818 of + `wl_backend.c`). This includes creation of: + - `wlr_compositor`, `wlr_subcompositor` + - `wlr_seat`, `wlr_data_device_manager` + - seat event wiring (request_set_cursor, request_set_selection, + request_start_drag, request_set_primary_selection) + - `wlr_primary_selection_v1_device_manager` + - `wlr_xdg_output_manager_v1` + - `wlr_idle_inhibit_v1` + - `wlr_viewporter` + - `wlr_relative_pointer_manager_v1` + - `wlr_pointer_constraints_v1` + - `wlr_keyboard_shortcuts_inhibit_v1` + - `wlr_linux_dmabuf_v1` + - `wlr_screencopy_manager_v1` + - `wlr_export_dmabuf_manager_v1` + - `wlr_output_manager_v1` + - `wlr_output_power_manager_v1` + - `wlr_gamma_control_manager_v1` + - `wlr_presentation` + - `wlr_text_input_manager_v3` + - `wlr_input_method_manager_v2` + - `wlr_virtual_keyboard_manager_v1` + - `wlr_foreign_toplevel_manager_v1` + - `wlr_xdg_activation_v1` + - `wlr_layer_shell_v1` + - `wlr_data_control_manager_v1` + - `wlr_virtual_pointer_manager_v1` + - `wlr_xdg_foreign_registry` + `wlr_xdg_foreign_v2` + - `wlr_xdg_shell` + - `wmaker_popup_manager_v1` (custom protocol) + - `wlr_xdg_decoration_manager_v1` + - `wlr_renderer_init_wl_shm` + +2. The function needs these `extern` handler declarations (move from + `wl_backend.c` forward-declaration block): + - `handle_seat_request_cursor` + - `handle_seat_request_set_selection` — currently `static` in wl_backend.c; + must become non-static and move to `wl_extensions.c` or stay in + wl_backend.c with an `extern` declaration + - `handle_seat_request_start_drag` — already in `wl_dnd.c` + - `handle_seat_request_primary_selection` — currently `static`; same treatment + - `handle_idle_new_inhibitor` — currently `static`; same treatment + - `handle_new_pointer_constraint` — currently `static`; same treatment + - `handle_new_shortcuts_inhibitor` — currently `static`; same treatment + - All the `extern` handlers already in other files (output_mgr, text_input, etc.) + +3. Move the 5 small `static` handler functions that are only used by protocol + init into `wl_extensions.c` as well (remove `static`): + - `handle_seat_request_set_selection` (lines 1451-1459) + - `handle_seat_request_primary_selection` (lines 1467-1475) + - `handle_idle_new_inhibitor` (lines 1484-1491) + - `handle_new_pointer_constraint` (lines 1507-1517) + - `handle_new_shortcuts_inhibitor` (lines 1528-1535) + +4. In `wl_display_open()`, replace lines 578-821 with: + ```c + wl_protocols_init(); + ``` + +5. Add required `#include` directives to `wl_extensions.c` for the wlroots + protocol types used. The full list is long — copy the relevant subset + from `wl_backend.c`'s include block. + +6. No `Makefile.am` change needed — `wl_extensions.c` is already listed. + +**Verification:** `make` succeeds. Launch compositor, verify all protocol +globals are advertised (`wayland-info` or `wlr-randr` should list them). + +--- + +## Execution order + +Steps are ordered by risk (lowest first) and independence: + +| Step | Risk | Reason | +|---|---|---| +| 1 (layer shell dedup) | **Lowest** | Pure deletion of shadowed static code | +| 2 (background) | **Low** | Self-contained, no cross-references | +| 3 (screen internals) | **Medium** | Many extern references to resolve | +| 4 (X11 bridge) | **Low** | Small, single function | +| 5 (protocol init) | **Medium** | Touches the critical init path; many handler relocations | + +Each step should be a separate commit. Build and smoke-test after each. + +--- + +## Expected result + +| File | Before | After | +|---|---|---| +| `wl_backend.c` | 2329 lines | ~900 lines | +| `wl_layer_shell.c` | 196 lines (dead) | 196 lines (live) | +| `wl_background.c` | — (new) | ~310 lines | +| `wl_screen.c` | 244 lines | ~480 lines | +| `wl_xwayland.c` | 429 lines | ~485 lines | +| `wl_extensions.c` | 172 lines | ~450 lines | + +The remaining `wl_backend.c` contains: +- `wl_display_open()` (~430 lines) — backend/renderer/allocator/scene/cursor + creation, XKB keymap, XWayland setup, frame timer, shutdown pipe, pump thread +- `wl_display_close()` (~210 lines) — teardown and listener removal +- `wl_display_post_open()`, `wl_display_screen_count()`, + `wl_display_default_screen()` — trivial stubs +- `wl_event_loop_run()` (~190 lines) — signalfd, main dispatch loop +- `wl_event_loop_terminate()` — shutdown pipe write +- `wl_startup_pump_thread()` — startup helper +- `wl_shutdown_pipe_handler()`, `frame_timer_cb()` — event loop callbacks +- Global `wl_state` definition + +This is a coherent "compositor lifecycle" module at a manageable size. diff --git a/src/backend/wayland/wl_stubs.c b/src/backend/wayland/wl_stubs.c index 48d257f..6086e04 100644 --- a/src/backend/wayland/wl_stubs.c +++ b/src/backend/wayland/wl_stubs.c @@ -54,14 +54,14 @@ struct _XDisplay; typedef struct _XDisplay Display; /* XWayland wrapper stubs — real implementations in wl_xdisplay.c / wl_xwayland.c */ -void wl_xdisplay_flush(void) {} -void wl_xdisplay_sync(int d) { (void)d; } -unsigned long wl_xdisplay_intern_atom(const char *n) { (void)n; return 0; } -Display *wl_get_x_display(void) { return NULL; } -RImage *wl_xwayland_capture_snapshot(WScreen *s, struct wlr_xwayland_surface *xw) { (void)s; (void)xw; return NULL; } -float wl_xwayland_get_window_opacity(WNativeWindow cw) { (void)cw; return 1.0f; } -void wl_register_error_handlers(void) {} -RContext *wl_create_rcontext(int screen_number, RContextAttributes *attribs) +__attribute__((weak)) void wl_xdisplay_flush(void) {} +__attribute__((weak)) void wl_xdisplay_sync(int d) { (void)d; } +__attribute__((weak)) unsigned long wl_xdisplay_intern_atom(const char *n) { (void)n; return 0; } +__attribute__((weak)) Display *wl_get_x_display(void) { return NULL; } +__attribute__((weak)) RImage *wl_xwayland_capture_snapshot(WScreen *s, struct wlr_xwayland_surface *xw) { (void)s; (void)xw; return NULL; } +__attribute__((weak)) float wl_xwayland_get_window_opacity(WNativeWindow cw) { (void)cw; return 1.0f; } +__attribute__((weak)) void wl_register_error_handlers(void) {} +__attribute__((weak)) RContext *wl_create_rcontext(int screen_number, RContextAttributes *attribs) { (void)screen_number; return RCreateContextWayland(1920, 1080, attribs); } #if USE_XWAYLAND void wl_client_message_forward(WMEvent *event, WNativeWindow target_win); diff --git a/src/backend/wayland/wl_window.c b/src/backend/wayland/wl_window.c index 02a7983..8066d0b 100644 --- a/src/backend/wayland/wl_window.c +++ b/src/backend/wayland/wl_window.c @@ -26,7 +26,7 @@ struct wl_frame_buf *frame_buf_find(WNativeWindow id); extern void *W_RegisterBacking(unsigned long id, void *img); extern void W_UnregisterBacking(unsigned long id); struct wl_toplevel_view *wl_find_view_by_id(WNativeWindow id); -pixman_image_t *get_window_image_from_x11(Display *x_display, WNativeWindow win) +__attribute__((weak)) pixman_image_t *get_window_image_from_x11(Display *x_display, WNativeWindow win) { (void)x_display; (void)win; return NULL; }