fix combined build: mark stub functions __attribute__((weak)) to resolve multiple-definition conflicts when linking both X11 and Wayland backends
This commit is contained in:
lkn
parent
709a8d76fc
commit
60c43b734a
|
|
@ -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 <flags>` 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 <string.h>
|
||||
#include <stdlib.h>
|
||||
#include <limits.h>
|
||||
#include <wayland-server-core.h>
|
||||
#include <wlr/types/wlr_output.h>
|
||||
#include <wlr/types/wlr_scene.h>
|
||||
#include <wlr/types/wlr_buffer.h>
|
||||
#include <wlr/interfaces/wlr_buffer.h>
|
||||
#include <libdrm/drm_fourcc.h>
|
||||
#include <pixman.h>
|
||||
#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 <flags> && 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 <wlr/types/wlr_scene.h>
|
||||
#include <wlr/types/wlr_buffer.h>
|
||||
#include <wlr/interfaces/wlr_buffer.h>
|
||||
#include <pixman.h>
|
||||
#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 `<WINGs/WMEvent.h>` 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.
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue