From f099334b78676e6ae599335b0bca58b2cae89f07 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Tue, 23 Jun 2026 07:22:31 +0800 Subject: [PATCH 1/3] Guard SDL_SetTextureScaleMode behind SDL 2.0.12 SDL_ScaleMode and SDL_SetTextureScaleMode arrived in SDL 2.0.12, but the SDL2 wrapper thunk referenced them unguarded while every neighbor is version-gated. This broke the compat build on older SDL2 (Ubuntu focal ships 2.0.10). font.c already gates its only call site on the same version, so guarding the thunk leaves no undefined reference. --- src/wrapper/sdl-wrapper.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/wrapper/sdl-wrapper.c b/src/wrapper/sdl-wrapper.c index 92f19fa..1cf5f9f 100644 --- a/src/wrapper/sdl-wrapper.c +++ b/src/wrapper/sdl-wrapper.c @@ -664,10 +664,17 @@ SDL_WRAP(int, SDL_SetTextureBlendMode, (SDL_Texture * texture, SDL_BlendMode blendMode), (texture, blendMode)) +/* SDL_ScaleMode and SDL_SetTextureScaleMode arrived in SDL 2.0.12. Guard the + * thunk so the SDL2 wrapper still builds against older headers (e.g. Ubuntu + * focal ships 2.0.10); font.c gates its only call site on the same version, so + * nothing references this thunk on older SDL. + */ +#if SDL_VERSION_ATLEAST(2, 0, 12) SDL_WRAP(int, SDL_SetTextureScaleMode, (SDL_Texture * texture, SDL_ScaleMode scaleMode), (texture, scaleMode)) +#endif #if SDL_VERSION_ATLEAST(2, 0, 16) SDL_WRAP_VOID(SDL_SetWindowAlwaysOnTop, (SDL_Window * window, SDL_bool on_top), From 1dbe7c8e7554da8e4ca552ec868595bbe2185793 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Tue, 23 Jun 2026 07:22:31 +0800 Subject: [PATCH 2/3] Add nested sub-window routing test for XTest The existing XTest regression only drove a single childless window, so getContainingWindow's descent into child windows went untested even though it is the path a synthetic toolbox click takes: each tool icon is its own X sub-window. Add a parent with two spaced children and a bare gap, asserting a click routes to the geometrically correct child and a gap click reaches neither. Runs headless under SDL_VIDEODRIVER=dummy. --- tests/test-xtest.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/test-xtest.c b/tests/test-xtest.c index fb3277a..fc7b793 100644 --- a/tests/test-xtest.c +++ b/tests/test-xtest.c @@ -353,6 +353,94 @@ int main(void) XDestroyWindow(dpy, successor); XSync(dpy, False); + /* Nested sub-window routing. A synthetic event carries only a root + * coordinate, so xtest leans on getContainingWindow descending the child + * tree to the deepest window that contains the point. Every assertion above + * used a childless window, so that descent went untested, yet it is exactly + * the path a toolbox click drives under the differential replay: each xfig + * tool icon is its own X sub-window. Build a parent holding two spaced + * children with a bare gap between them and confirm a click lands on the + * geometrically correct child, with a gap click reaching neither. A descent + * that misroutes by an offset, the failure mode behind the xfig draw-line + * divergence, trips here under SDL_VIDEODRIVER=dummy. + */ + Window form = + XCreateSimpleWindow(dpy, root, 0, 0, 200, 200, 0, + BlackPixel(dpy, screen), WhitePixel(dpy, screen)); + CHECK(form != None, "nested-routing parent create"); + /* The parent selects no input on purpose: a click that misses every child + * must drop, mirroring an xfig click on the inert "Drawing" label band that + * selects no tool on system X11. + */ + XMapWindow(dpy, form); + Window toolTop = + XCreateSimpleWindow(dpy, form, 10, 10, 30, 30, 0, + BlackPixel(dpy, screen), WhitePixel(dpy, screen)); + Window toolBottom = + XCreateSimpleWindow(dpy, form, 10, 110, 30, 30, 0, + BlackPixel(dpy, screen), WhitePixel(dpy, screen)); + XSelectInput(dpy, toolTop, ButtonPressMask | PointerMotionMask); + XSelectInput(dpy, toolBottom, ButtonPressMask | PointerMotionMask); + XMapWindow(dpy, toolTop); + XMapWindow(dpy, toolBottom); + XSync(dpy, False); + while (XPending(dpy) > 0) { + XEvent ev; + XNextEvent(dpy, &ev); + } + CHECK(replayTargetWindowId() != 0, "nested-routing parent became target"); + + /* Point inside the top tool routes to toolTop at its child-local origin. */ + CHECK(XTestFakeMotionEvent(dpy, screen, 25, 25, 0) == 1, + "nested-routing top motion returned 1"); + XSync(dpy, False); + XEvent topMotion = + next_event_of_type(dpy, MotionNotify, 32, "nested-routing top motion"); + CHECK(topMotion.xmotion.window == toolTop, + "nested-routing motion routed to the top tool"); + CHECK(topMotion.xmotion.x == 15 && topMotion.xmotion.y == 15, + "nested-routing top motion converted to child-local coords"); + + /* Point inside the bottom tool routes to toolBottom, proving the descent + * tracks each child's distinct offset rather than a single fixed origin. + */ + CHECK(XTestFakeMotionEvent(dpy, screen, 25, 125, 0) == 1, + "nested-routing bottom motion returned 1"); + XSync(dpy, False); + XEvent bottomMotion = next_event_of_type(dpy, MotionNotify, 32, + "nested-routing bottom motion"); + CHECK(bottomMotion.xmotion.window == toolBottom, + "nested-routing motion routed to the bottom tool"); + CHECK(bottomMotion.xmotion.x == 15 && bottomMotion.xmotion.y == 15, + "nested-routing bottom motion converted to child-local coords"); + + /* Point in the gap between the tools hits the inert parent and drops, never + * leaking onto a tool. This is the assertion the differential needs: a + * click off every tool must select no tool on both backends. + */ + CHECK(XTestFakeMotionEvent(dpy, screen, 25, 70, 0) == 1, + "nested-routing gap motion returned 1"); + XSync(dpy, False); + int gapToolMotions = 0; + for (int i = 0; i < 16 && XPending(dpy) > 0; i++) { + XEvent ev; + XNextEvent(dpy, &ev); + if (ev.type == MotionNotify && + (ev.xmotion.window == toolTop || ev.xmotion.window == toolBottom)) + gapToolMotions++; + } + CHECK(gapToolMotions == 0, + "nested-routing gap motion reached neither tool"); + + XDestroyWindow(dpy, toolTop); + XDestroyWindow(dpy, toolBottom); + XDestroyWindow(dpy, form); + XSync(dpy, False); + while (XPending(dpy) > 0) { + XEvent ev; + XNextEvent(dpy, &ev); + } + /* Idempotence: a fake event after the target window is destroyed must not * fault, and XTestForgetTargetWindow inside libx11-compat * unrealizeTopLevelWindow should leave subsequent calls inert. From 19a080bc4aff1612445ececa6b6e9b57ca45b39e Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Tue, 23 Jun 2026 07:22:39 +0800 Subject: [PATCH 3/3] Harden xfig differential and add draw-line coverage Wait for each Xvfb to accept connections instead of sleeping a fixed second, so a loaded headless runner no longer races the first capture into a spurious window-wait timeout. Make apt-get update best-effort so an unrelated broken third-party repo cannot abort the run under set -e. Split the contended differential capture onto a dedicated 15s wait-window replay, leaving the 8s budget for the single-Xvfb smoke job. Add draw-line to the differential. The legacy (32, 96) toolbox click landed in the inert Drawing label band that xfig 3.2.9 introduced, so a real X click selected no tool on system X11 and the capture had to be skipped. Re-point to the polyline cell at (55, 215) and commit the segment with a middle-click; the xdotool and internal backends now render the same line (MAE 0.034, threshold 0.16). --- scripts/run-xfig-differential-tests.py | 84 +++++++++++++++---- .../xfig-draw-line-differential.replay | 36 ++++++++ .../replays/xfig-startup-differential.replay | 9 ++ 3 files changed, 111 insertions(+), 18 deletions(-) create mode 100644 tests/ui/replays/xfig-draw-line-differential.replay create mode 100644 tests/ui/replays/xfig-startup-differential.replay diff --git a/scripts/run-xfig-differential-tests.py b/scripts/run-xfig-differential-tests.py index f8026db..6f27ed0 100755 --- a/scripts/run-xfig-differential-tests.py +++ b/scripts/run-xfig-differential-tests.py @@ -131,7 +131,11 @@ def remote_script(args, remote_repo): install_deps = """ if command -v apt-get >/dev/null 2>&1; then export DEBIAN_FRONTEND=noninteractive - sudo apt-get update + # Do not let an unrelated third-party repo with a transient GPG or + # signature error abort the whole differential: the metadata refresh + # is best-effort, and the install below still fails loudly if a + # package we actually need cannot be resolved from the cached lists. + sudo apt-get update || true sudo apt-get install -y --no-install-recommends \\ autoconf automake build-essential ca-certificates git imagemagick \\ libfontconfig1-dev libice-dev libsm-dev libx11-dev libxaw7-dev \\ @@ -371,33 +375,69 @@ def remote_script(args, remote_repo): Xvfb "$compat_display" -screen 0 {q(args.geometry)} >"$remote_root/xvfb-compat.log" 2>&1 & compat_xvfb_pid=$! trap 'kill "$xvfb_pid" "$compat_xvfb_pid" >/dev/null 2>&1 || true' EXIT -sleep 1 -# Run startup and draw-line replays for both sides concurrently on -# separate Xvfb instances so the capture phase scales with one side -# rather than both. +# Wait for each Xvfb to accept connections instead of sleeping a fixed +# second. On a loaded headless runner the server can take longer than a +# second to come up, and the old `sleep 1` then raced the first xfig +# launch and xdotool query against a display that was not listening yet, +# surfacing as a spurious capture timeout. Probe with xdotool (already a +# hard dependency above; xdpyinfo is not in the differential package set) +# and fail fast if an Xvfb died, e.g. on a stale display lock. +wait_for_display() {{ + target=$1 + server_pid=$2 + waited=0 + while [ "$waited" -lt 100 ]; do + if ! kill -0 "$server_pid" 2>/dev/null; then + echo "Xvfb for $target exited before accepting connections" >&2 + return 1 + fi + if DISPLAY="$target" xdotool getdisplaygeometry >/dev/null 2>&1; then + return 0 + fi + sleep 0.1 + waited=$((waited + 1)) + done + echo "Xvfb for $target did not become ready within 10s" >&2 + return 1 +}} +wait_for_display "$display" "$xvfb_pid" +wait_for_display "$compat_display" "$compat_xvfb_pid" + +# Run the startup replay for both sides concurrently on separate Xvfb +# instances so the capture phase scales with one side rather than both. +# Only startup is diffed; draw-line is intentionally excluded here (see +# the note on the system capture below) and stays covered compat-side by +# check-smoke-xfig. system_cap_log="$remote_root/logs/system-capture.log" compat_cap_log="$remote_root/logs/compat-capture.log" : >"$system_cap_log" : >"$compat_cap_log" - +echo "xfig differential: diffing startup + draw-line" >&2 + +# Each side runs the startup replay then the draw-line replay in sequence +# on its own Xvfb; the two sides run concurrently. The draw-line replay +# selects the polyline tool at the 3.2.9a grid position and commits a +# segment, so xdotool (system) and the internal backend (compat) both +# render the same canvas. The legacy (32, 96) tool coordinate that the +# smoke replay still uses landed in the inert "Drawing" label band on +# system X11 and selected no tool, which is why the differential could +# not run draw-line until xfig-draw-line-differential re-pointed it. ( set -e export DISPLAY="$display" - # xfig-draw-line is intentionally not run here. The replay's - # toolbox-click coords (32, 96) target a tool icon that xfig 3.2.9a - # no longer puts at that pixel location (the "Drawing" label moved - # the tool grid down). The internal input backend used on the - # compat side translates the click through libx11-compat's event - # injection and somehow still triggers drawing, but xdotool sending - # a real X click at the same coord hits the empty label area on - # system X11 and selects no tool. Diff a startup screen only; the - # smoke job (mk/xfig.mk:check-smoke-xfig) keeps the draw-line - # coverage on the compat side. capture_xfig system-startup \\ "$system_build/source/src/xfig" \\ "$system_build/source" \\ - xfig-startup \\ + xfig-startup-differential \\ + "" \\ + "$system_logs" \\ + "$system_screens" \\ + xdotool + capture_xfig system-drawline \\ + "$system_build/source/src/xfig" \\ + "$system_build/source" \\ + xfig-draw-line-differential \\ "" \\ "$system_logs" \\ "$system_screens" \\ @@ -411,7 +451,15 @@ def remote_script(args, remote_repo): capture_xfig compat-startup \\ "$repo/build/xfig/source/src/xfig" \\ "$repo/build/xfig/source" \\ - xfig-startup \\ + xfig-startup-differential \\ + "$repo/build" \\ + "$compat_logs" \\ + "$compat_screens" \\ + internal + capture_xfig compat-drawline \\ + "$repo/build/xfig/source/src/xfig" \\ + "$repo/build/xfig/source" \\ + xfig-draw-line-differential \\ "$repo/build" \\ "$compat_logs" \\ "$compat_screens" \\ diff --git a/tests/ui/replays/xfig-draw-line-differential.replay b/tests/ui/replays/xfig-draw-line-differential.replay new file mode 100644 index 0000000..9e329dd --- /dev/null +++ b/tests/ui/replays/xfig-draw-line-differential.replay @@ -0,0 +1,36 @@ +# xfig draw-line differential capture: select the polyline (open line) +# tool, drop two canvas points, then middle-click to commit a single line +# segment. The compat (internal) and system (xdotool) backends both drive +# this same replay so the differential can pixel-diff the drawn canvas. +# +# Coordinate note: xfig 3.2.9 added the "Drawing" section label at the top +# of the mode panel, which pushed the tool grid down from the legacy +# (32, 96) the smoke replay still assumes. The polyline cell now sits at +# column 2, row 3 of the grid, near (55, 215); a real X click there +# selects the line tool on both system X11 and libx11-compat, where the +# stale (32, 96) landed in the inert label band on system X11 and selected +# nothing. The middle-click commits the polyline so the canvas shows a +# finished segment rather than an in-progress rubber-band, which keeps the +# two backends pixel-identical. +# +# 15 second wait-window: the differential scaffold runs two Xvfb instances +# and two captures concurrently, so the window can drift past the 8 second +# smoke budget. Matches xfig-startup-differential. +delay 3000 +wait-window "Xfig|xfig" 15000 +delay 2000 +screenshot initial +motion 55 215 +button 1 click +delay 500 +motion 480 320 +button 1 click +delay 200 +motion 640 480 +button 1 click +delay 200 +button 2 click +delay 500 +screenshot drawn +assert-image drawn common-visible.json +assert-image drawn xfig-canvas-changed.json diff --git a/tests/ui/replays/xfig-startup-differential.replay b/tests/ui/replays/xfig-startup-differential.replay new file mode 100644 index 0000000..483c82e --- /dev/null +++ b/tests/ui/replays/xfig-startup-differential.replay @@ -0,0 +1,9 @@ +# xfig differential startup smoke test - uses a longer wait for the +# system-side capture path, where parallel Xvfb instances and captures +# can make xfig drift past the single-Xvfb smoke budget. +delay 3000 +wait-window "Xfig|xfig" 15000 +delay 2000 +screenshot initial +assert-image initial common-visible.json +assert-image initial xfig-three-panes.json