diff --git a/scripts/run-mosaic-differential-tests.py b/scripts/run-mosaic-differential-tests.py index 73a3dcc..22dea7f 100644 --- a/scripts/run-mosaic-differential-tests.py +++ b/scripts/run-mosaic-differential-tests.py @@ -632,9 +632,7 @@ def main(): env_compare_location = os.environ.get("MOSAIC_DIFF_COMPARE_LOCATION") if env_compare_location: if env_compare_location not in ("remote", "local"): - parser.error( - "MOSAIC_DIFF_COMPARE_LOCATION must be 'remote' or 'local'" - ) + parser.error("MOSAIC_DIFF_COMPARE_LOCATION must be 'remote' or 'local'") args.compare_location = env_compare_location elif args.local: args.compare_location = "local" diff --git a/scripts/run-motif-differential-tests.py b/scripts/run-motif-differential-tests.py index d3689b3..0be902d 100755 --- a/scripts/run-motif-differential-tests.py +++ b/scripts/run-motif-differential-tests.py @@ -716,9 +716,7 @@ def main(): env_compare_location = os.environ.get("MOTIF_DIFF_COMPARE_LOCATION") if env_compare_location: if env_compare_location not in ("remote", "local"): - parser.error( - "MOTIF_DIFF_COMPARE_LOCATION must be 'remote' or 'local'" - ) + parser.error("MOTIF_DIFF_COMPARE_LOCATION must be 'remote' or 'local'") args.compare_location = env_compare_location elif args.local: args.compare_location = "local" diff --git a/scripts/run-osiris-differential-tests.py b/scripts/run-osiris-differential-tests.py index 9bad8c4..9bc56b1 100755 --- a/scripts/run-osiris-differential-tests.py +++ b/scripts/run-osiris-differential-tests.py @@ -690,9 +690,7 @@ def main(): env_compare_location = os.environ.get("OSIRIS_DIFF_COMPARE_LOCATION") if env_compare_location: if env_compare_location not in ("remote", "local"): - parser.error( - "OSIRIS_DIFF_COMPARE_LOCATION must be 'remote' or 'local'" - ) + parser.error("OSIRIS_DIFF_COMPARE_LOCATION must be 'remote' or 'local'") args.compare_location = env_compare_location elif args.local: args.compare_location = "local" diff --git a/scripts/run-ui-replay.py b/scripts/run-ui-replay.py index b81302f..0262a14 100755 --- a/scripts/run-ui-replay.py +++ b/scripts/run-ui-replay.py @@ -351,22 +351,37 @@ def wait_process_alive(proc, timeout_ms): time.sleep(0.1) -def wait_converge_timeout_ms(parts): +WAIT_CONVERGE_DEFAULTS = [50, 2, 16, 32, 5000] + + +def wait_converge_fields(parts): # Defaults track the C-side handler in src/replay.c. Malformed input # raises ReplayError so the dispatcher converts it into a deterministic # step failure (otherwise int("abc") would surface as an uncaught # ValueError and crash the runner mid-replay). - if len(parts) < 6: - return 5000 + if len(parts) > 6: + raise ReplayError("wait-converge expects at most 5 arguments") + fields = WAIT_CONVERGE_DEFAULTS.copy() + for idx, raw in enumerate(parts[1:]): + try: + value = int(raw) + except ValueError as error: + raise ReplayError( + f"wait-converge arg {idx + 1} must be an integer, got {raw!r}" + ) from error + if value < 0: + raise ReplayError( + f"wait-converge arg {idx + 1} must be non-negative, got {value}" + ) + fields[idx] = value + return fields + + +def wait_converge_timeout_ms(parts): try: - value = int(parts[5]) - except ValueError as error: - raise ReplayError( - f"wait-converge timeout_ms must be an integer, got {parts[5]!r}" - ) from error - if value < 0: - raise ReplayError(f"wait-converge timeout_ms must be non-negative, got {value}") - return value + return wait_converge_fields(parts)[4] + except ReplayError as error: + raise ReplayError(str(error).replace("arg 5", "timeout_ms")) from error def wait_for_nonempty_file(path, timeout_s, poll_s=0.05, proc=None): @@ -470,13 +485,37 @@ def write_internal_replay(source_path, dest_path, snapshot_dir=None, sync_dir=No wait_ms = int(parts[2]) if wait_ms > 0: lines.append(f"delay {wait_ms}") + elif command == "focus-at": + if len(parts) != 3: + raise ReplayError( + f"{source_path}:{lineno}: focus-at expects target-local x y" + ) + try: + fx = int(parts[1]) + fy = int(parts[2]) + except ValueError as error: + raise ReplayError( + f"{source_path}:{lineno}: focus-at coords must be " + f"integers, got {parts[1]!r} {parts[2]!r}" + ) from error + if fx < 0 or fy < 0: + raise ReplayError( + f"{source_path}:{lineno}: focus-at coords must be " + f"non-negative, got {fx} {fy}" + ) + lines.append(f"focus-at {fx} {fy}") elif command == "wait-converge": # Default args mirror the C-side defaults; allow override: # wait-converge [bucket_ms quiet diverged threshold timeout_ms] - extra = " ".join(parts[1:]) if len(parts) > 1 else "" - lines.append(f"wait-converge {extra}".rstrip()) + fields = wait_converge_fields(parts) if sync_dir is not None: + failure_path = sync_dir / f"wait-converge-{lineno}.fail" + expanded = " ".join(str(value) for value in fields) + lines.append(f"wait-converge {expanded} failure-marker={failure_path}") lines.append(f"state-snapshot {sync_dir}/wait-converge-{lineno}.json") + else: + extra = " ".join(parts[1:]) if len(parts) > 1 else "" + lines.append(f"wait-converge {extra}".rstrip()) elif command == "state-snapshot": if len(parts) < 2: raise ReplayError( @@ -1307,6 +1346,29 @@ def run_replay(args): raise ReplayError("key expects keysym") if args.input_backend == "xdotool": xdotool(env, "key", parts[1]) + elif command == "focus-at": + if len(parts) != 3: + raise ReplayError("focus-at expects target-local x y") + try: + int(parts[1]) + int(parts[2]) + except ValueError as error: + raise ReplayError( + f"focus-at coords must be integers, got " + f"{parts[1]!r} {parts[2]!r}" + ) from error + # The internal backend handles this through the expanded + # replay script. For xdotool, fall back to a window + # activate so the focus shift still happens on native. + # Mirror target-motion: require a prior wait-window so a + # missing target surfaces as a deterministic failure + # rather than a silent no-op (cubic-flagged). + if args.input_backend == "xdotool": + if target_window_id is None: + raise ReplayError( + "focus-at requires a prior wait-window" + ) + xdotool(env, "windowfocus", target_window_id) elif command == "resize": if len(parts) != 3: raise ReplayError("resize expects W H") @@ -1464,6 +1526,7 @@ def run_replay(args): elif command == "wait-converge": if args.input_backend == "internal": sync_target = sync_dir / f"wait-converge-{lineno}.json" + failure_target = sync_dir / f"wait-converge-{lineno}.fail" wait_start = time.perf_counter() timeout_s = wait_converge_timeout_ms(parts) / 1000.0 + 16.0 if not wait_for_nonempty_file( @@ -1474,6 +1537,9 @@ def run_replay(args): f"appear within " f"{time.perf_counter() - wait_start:.2f}s" ) + if failure_target.exists(): + detail = failure_target.read_text().strip() or "failed" + raise ReplayError(f"wait-converge {detail}") else: # External backends have no detector; mirror the # legacy delay-shaped behavior so the script stays diff --git a/scripts/run-xfig-differential-tests.py b/scripts/run-xfig-differential-tests.py index 06d9b80..f8026db 100755 --- a/scripts/run-xfig-differential-tests.py +++ b/scripts/run-xfig-differential-tests.py @@ -632,9 +632,7 @@ def main(): env_compare_location = os.environ.get("XFIG_DIFF_COMPARE_LOCATION") if env_compare_location: if env_compare_location not in ("remote", "local"): - parser.error( - "XFIG_DIFF_COMPARE_LOCATION must be 'remote' or 'local'" - ) + parser.error("XFIG_DIFF_COMPARE_LOCATION must be 'remote' or 'local'") args.compare_location = env_compare_location elif args.local: args.compare_location = "local" diff --git a/src/events.c b/src/events.c index d868394..fc045be 100644 --- a/src/events.c +++ b/src/events.c @@ -153,6 +153,13 @@ static Bool shouldStartXtWakeTimer(void) typedef struct PutBackEvent { Display *display; XEvent event; + /* True when convertEvent's terminal tap already ran before the event was + * placed on this queue. Put-back delivery paths use this to avoid + * double-tapping such events into the timeline. Synthesized events that + * bypass convertEvent (e.g. crossings built by appendPointerCrossingEvent) + * leave this False so the timeline still sees them when consumed. + */ + Bool alreadyTapped; struct PutBackEvent *next; } PutBackEvent; @@ -348,7 +355,9 @@ static Bool getEventQueueLength(int *qlen) return True; } -static Bool enqueuePutBackEvent(Display *display, const XEvent *event) +static Bool enqueuePutBackEventWithTap(Display *display, + const XEvent *event, + Bool alreadyTapped) { PutBackEvent *node = malloc(sizeof(PutBackEvent)); if (!node) { @@ -358,6 +367,7 @@ static Bool enqueuePutBackEvent(Display *display, const XEvent *event) node->display = display; memcpy(&node->event, event, sizeof(XEvent)); + node->alreadyTapped = alreadyTapped; lockPutBackEvents(); node->next = putBackEvents; putBackEvents = node; @@ -370,7 +380,14 @@ static Bool enqueuePutBackEvent(Display *display, const XEvent *event) return True; } -static Bool appendPutBackEvent(Display *display, const XEvent *event) +static Bool enqueuePutBackEvent(Display *display, const XEvent *event) +{ + return enqueuePutBackEventWithTap(display, event, False); +} + +static Bool appendPutBackEventWithTap(Display *display, + const XEvent *event, + Bool alreadyTapped) { PutBackEvent *node = malloc(sizeof(PutBackEvent)); if (!node) { @@ -379,6 +396,7 @@ static Bool appendPutBackEvent(Display *display, const XEvent *event) } node->display = display; memcpy(&node->event, event, sizeof(XEvent)); + node->alreadyTapped = alreadyTapped; node->next = NULL; lockPutBackEvents(); @@ -391,6 +409,32 @@ static Bool appendPutBackEvent(Display *display, const XEvent *event) return True; } +static Bool appendPutBackEvent(Display *display, const XEvent *event) +{ + return appendPutBackEventWithTap(display, event, False); +} + +static void deliverPutBackEvent(Display *display, + PutBackEvent *node, + XEvent *event) +{ + Bool alreadyTapped = node->alreadyTapped; + memcpy(event, &node->event, sizeof(XEvent)); + free(node); + /* Match the pipe byte enqueuePutBackEvent wrote so the wake-up accounting + * stays consistent. + */ + READ_EVENT_IN_PIPE(display); + /* Put-back events that bypass convertEvent's terminal tap (synthesized + * crossings, send-event payloads) need to be tapped here so timeline JSONL + * captures the full event stream. Events that already went through + * convertEvent's tap before being put back carry alreadyTapped=True so the + * timeline does not see them twice. + */ + if (!alreadyTapped) + timelineTapXEvent(event); +} + static Bool popPutBackEvent(Display *display, XEvent *event) { lockPutBackEvents(); @@ -399,13 +443,8 @@ static Bool popPutBackEvent(Display *display, XEvent *event) PutBackEvent *node = *link; if (node->display == display) { *link = node->next; - memcpy(event, &node->event, sizeof(XEvent)); unlockPutBackEvents(); - free(node); - /* Match the pipe byte enqueuePutBackEvent wrote so the wake-up - * accounting stays consistent. - */ - READ_EVENT_IN_PIPE(display); + deliverPutBackEvent(display, node, event); return True; } link = &node->next; @@ -456,7 +495,11 @@ static int drainSdlEventsToPutBack(Display *display) for (int i = 0; i < qlen; i++) { XEvent converted; if (convertEvent(display, &events[i], &converted, True) == 0) - appendPutBackEvent(display, &converted); + /* convertEvent already ran its terminal timelineTapXEvent for these + * events; mark them so popPutBackEvent does not tap a second time + * when they are eventually consumed. + */ + appendPutBackEventWithTap(display, &converted, True); } int remainingSdlEvents = 0; if (getEventQueueLength(&remainingSdlEvents)) { @@ -850,10 +893,8 @@ static Bool removeMatchingPutBackEvent( if (node->display == display && predicate(w, type, mask, &node->event)) { *link = node->next; - memcpy(event, &node->event, sizeof(XEvent)); unlockPutBackEvents(); - free(node); - READ_EVENT_IN_PIPE(display); + deliverPutBackEvent(display, node, event); return True; } link = &node->next; @@ -875,10 +916,8 @@ static Bool removeMatchingPutBackIfEvent(Display *display, PutBackEvent *node = *link; if (node->display == display && predicate(display, &node->event, arg)) { *link = node->next; - memcpy(event, &node->event, sizeof(XEvent)); unlockPutBackEvents(); - free(node); - READ_EVENT_IN_PIPE(display); + deliverPutBackEvent(display, node, event); return True; } link = &node->next; @@ -932,7 +971,7 @@ void discardQueuedEventsForWindow(Display *display, Window window) continue; if (converted.xany.window == window) continue; - appendPutBackEvent(display, &converted); + appendPutBackEventWithTap(display, &converted, True); } int remainingSdlEvents = 0; if (getEventQueueLength(&remainingSdlEvents)) { @@ -1721,9 +1760,9 @@ int convertEvent(Display *display, GET_WINDOW_STRUCT(eventWindow)->h = (unsigned int) sdlEvent->window.data2; resizeWindowTexture(eventWindow); - /* The top-level's own cached visibleRegion is its - * (0,0,w,h) frame; it goes stale on every resize - * driven by SDL outside of configureWindow. + /* The top-level's own cached visibleRegion is its (0,0,w,h) + * frame; it goes stale on every resize driven by SDL + * outside of configureWindow. */ invalidateVisibleRegionForTopLevel(eventWindow); clearWindowTreeWithoutExpose(display, eventWindow); @@ -1965,7 +2004,35 @@ int convertEvent(Display *display, pointerButtonStateSnapshot(); xEvent->xbutton.button = wheelButton; xEvent->xbutton.same_screen = True; - break; + /* Real X11 always pairs a wheel ButtonPress with an immediate + * matching ButtonRelease, and Motif translations (XmScrollBar, + * XmText) only fire their scroll actions once they see the full + * press/release sequence. Synthesizing only the press leaves + * Btn4Down/Btn5Down translations latched mid-sequence and the + * scroll never lands, which is why wheel input through libx11- + * compat appears as a no-op while the same replay driven via + * xdotool against system X11 scrolls correctly. + * + * Queue both ends at the put-back queue head (last enqueue ends + * up on top, so push Release first and Press second) and bail + * out with -1 so all consumer paths -- XNextEvent's main pump + * and the drainSdlEventsToPutBack drains -- pull them in the + * right order without the caller redundantly appending the + * Press behind the Release we already queued. + */ + XEvent pressEvent = *xEvent; + pressEvent.xbutton.type = ButtonPress; + pressEvent.xbutton.serial = serial; + pressEvent.xbutton.send_event = sendEvent; + pressEvent.xbutton.display = display; + pressEvent.xbutton.window = eventWindow; + timelineTapXEvent(&pressEvent); + XEvent releaseEvent = pressEvent; + releaseEvent.xbutton.type = ButtonRelease; + timelineTapXEvent(&releaseEvent); + enqueuePutBackEventWithTap(display, &releaseEvent, True); + enqueuePutBackEventWithTap(display, &pressEvent, True); + return -1; } case SDL_JOYAXISMOTION: /**< Joystick axis motion */ LOG("SDL_JOYAXISMOTION\n"); @@ -2087,6 +2154,16 @@ int convertEvent(Display *display, snapshotHandleResizeEvent(display, sdlEvent); return -1; } + if (snapshotOwnsEventType(sdlEvent->type) && + sdlEvent->user.code == FOCUS_AT_EVENT_CODE) { + /* Replay-driven focus shift. getContainingWindow walks the + * window tree and XSetInputFocus mutates global focus state; + * both must run on the main thread to stay safe against + * concurrent window-tree mutators. + */ + snapshotHandleFocusAtEvent(display, sdlEvent); + return -1; + } if (stateSnapshotOwnsEventType(sdlEvent->type)) { stateSnapshotHandleEvent(display, sdlEvent); return -1; @@ -3266,7 +3343,8 @@ static Bool checkTypedEvent(Display *display, if (!foundMatch && predicate(w, type, mask, &convertedEvent)) { memcpy(event, &convertedEvent, sizeof(XEvent)); foundMatch = True; - } else if (!appendPutBackEvent(display, &convertedEvent)) { + } else if (!appendPutBackEventWithTap(display, &convertedEvent, + True)) { free(tmp); UnlockDisplay(display); return False; @@ -3332,7 +3410,8 @@ static Bool checkIfEvent(Display *display, if (!foundMatch && predicate(display, &convertedEvent, arg)) { memcpy(event, &convertedEvent, sizeof(XEvent)); foundMatch = True; - } else if (!appendPutBackEvent(display, &convertedEvent)) { + } else if (!appendPutBackEventWithTap(display, &convertedEvent, + True)) { free(tmp); UnlockDisplay(display); return False; diff --git a/src/events.h b/src/events.h index bd33fc6..33a12bf 100644 --- a/src/events.h +++ b/src/events.h @@ -25,6 +25,12 @@ * drawing layer after X drawing operations that happen away from a blocking * XNextEvent/XFlush path, such as Motif popup menu expose redraws. */ #define PRESENT_EVENT_CODE 5 +/* Set keyboard focus to the X subwindow under a replay-target-local point. + * user.data1 carries the target-local x, user.data2 the y, each as intptr_t. + * Runs synchronously on the main thread so the window-tree walk and + * XSetInputFocus mutation never race with main-thread mutators. Used by the + * replay engine's focus-at verb. */ +#define FOCUS_AT_EVENT_CODE 6 #define HAS_EVENT_MASK(window, mask) \ ((GET_WINDOW_STRUCT(window)->eventMask & mask) == mask) diff --git a/src/font.c b/src/font.c index 491ab25..741820f 100644 --- a/src/font.c +++ b/src/font.c @@ -1792,8 +1792,15 @@ char **XListFontsWithInfo(Display *display, XFontStruct *XLoadQueryFont(Display *display, _Xconst char *name) { // https://tronche.com/gui/x/xlib/graphics/font-metrics/XLoadQueryFont.html - if (!findFontCacheEntryByName(name)) + FontCacheEntry *cacheHit = findFontCacheEntryByName(name); + if (!cacheHit) { + LOG("XLoadQueryFont MISS: %s\n", name ? name : "(null)"); return NULL; + } + LOG("XLoadQueryFont HIT: req='%s' -> XLF='%s' file='%s'\n", + name ? name : "(null)", + cacheHit->XLFName ? cacheHit->XLFName : "(null)", + cacheHit->filePath ? cacheHit->filePath : "(null)"); Font fontId = XLoadFont(display, name); if (fontId == None) return NULL; diff --git a/src/input-method.c b/src/input-method.c index 3e1e89e..ade8933 100644 --- a/src/input-method.c +++ b/src/input-method.c @@ -655,6 +655,8 @@ XFontSet XCreateFontSet(Display *display, if (def_string_return) *def_string_return = ""; + LOG("XCreateFontSet request: '%s'\n", + base_font_name_list ? base_font_name_list : "(null)"); char *pattern = firstFontSetPattern(base_font_name_list); if (!pattern) return NULL; diff --git a/src/replay.c b/src/replay.c index cbaa34d..2e40cf4 100644 --- a/src/replay.c +++ b/src/replay.c @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -96,6 +97,44 @@ static Bool isPressWord(const char *dir) return strcasecmp(dir, "press") == 0 || strcasecmp(dir, "down") == 0; } +/* Parse a single non-negative decimal coordinate, advance the cursor past + * trailing whitespace, and reject anything that would wrap-around or land + * outside int range. + * + * Returns True on success. Used by the focus-at verb so the x and y fields + * share one rejection path. + */ +static Bool parseFocusAtCoord(const char **cursor, int *out) +{ + while (**cursor == ' ' || **cursor == '\t') + (*cursor)++; + char *endp = NULL; + errno = 0; + long value = strtol(*cursor, &endp, 10); + if (*cursor == endp || errno == ERANGE || value < 0 || value > INT_MAX) + return False; + *cursor = endp; + while (**cursor == ' ' || **cursor == '\t') + (*cursor)++; + *out = (int) value; + return True; +} + +static void writeWaitConvergeFailure(const char *path, + int lineno, + const char *reason) +{ + FILE *fp = fopen(path, "w"); + if (!fp) { + fprintf(stderr, + "replay: line %d: wait-converge failure marker %s failed\n", + lineno, path); + return; + } + fprintf(fp, "%s\n", reason); + fclose(fp); +} + /* Replay diagnostics go to stderr unconditionally rather than through LOG(). * $LIBX11_COMPAT_REPLAY is an end-user / test-driver control path; silently * dropping "file not found" or "unknown command" errors in release builds would @@ -204,12 +243,54 @@ static void runScript(const char *path) XTestFakeKeyEvent(replayDisplay, code, isPressWord(dir), 0); replayLastInputAnchorMs = replayNowMs(); } + } else if (!strcmp(cmd, "focus-at")) { + /* Translate target-local (x, y) to root coords, resolve the X + * subwindow under that point, and call XSetInputFocus on it on the + * main thread (via snapshotRequestFocusAtAndWait). Motif's + * translation table normally invokes XmProcessTraversal from its + * Arm() / BSelect() actions after a click, which then calls + * XSetInputFocus and lets the widget paint its focus highlight. + * Synthetic ButtonPress + ButtonRelease via the XTest fake-event + * path does not reliably trigger XmProcessTraversal under every + * Motif focus-policy configuration, so this verb is the opt-in knob + * that forces the focus shift in tests that need it. + * + * Parsing: strtol with full-consumption + range checks. Reject + * negatives and values outside int range so getContainingWindow + * cannot be handed wrap-around coordinates that resolve to + * SCREEN_WINDOW or walk past a valid window's bounds. + */ + const char *cursor = args; + int x = 0, y = 0; + if (!parseFocusAtCoord(&cursor, &x)) { + fprintf(stderr, "replay: line %d: focus-at bad x '%s'\n", + lineno, args); + break; + } + if (!parseFocusAtCoord(&cursor, &y)) { + fprintf(stderr, "replay: line %d: focus-at bad y '%s'\n", + lineno, args); + break; + } + if (*cursor != '\0') { + fprintf(stderr, + "replay: line %d: focus-at trailing garbage '%s'\n", + lineno, cursor); + break; + } + int rc = snapshotRequestFocusAtAndWait(x, y); + if (rc != 0) + fprintf(stderr, + "replay: line %d: focus-at (%d, %d) failed " + "(rc=%d)\n", + lineno, x, y, rc); } else if (!strcmp(cmd, "wait-converge")) { /* Block the worker until the post-input event stream goes quiet, * with an explicit divergence trip-wire. Arguments are optional and * have sensible defaults; full form is * wait-converge [bucket_ms quiet_buckets diverged_buckets - * threshold_per_bucket timeout_ms] + * threshold_per_bucket timeout_ms + * failure-marker=path] * * Validation: parse with strtoull and reject any "-" prefix or * trailing garbage so a typo like `wait-converge -1` does not get @@ -231,6 +312,16 @@ static void runScript(const char *path) cursor++; if (*cursor == '\0') break; + /* Allow `failure-marker=` to appear at any position, including + * before any numeric field. Without this, forms like + * wait-converge failure-marker=foo + * wait-converge 200 failure-marker=foo + * are rejected by strtoull as "bad arg N" because the marker + * is not a digit. Break out here so remaining fields keep + * their defaults and the post-loop check handles the marker. + */ + if (!strncmp(cursor, "failure-marker=", 15)) + break; if (*cursor == '-') { fprintf(stderr, "replay: line %d: wait-converge rejects " @@ -254,6 +345,24 @@ static void runScript(const char *path) fields[idx] = parsed; cursor = endp; } + /* The promise in the leading comment is "reject trailing garbage"; + * without this post-loop check `wait-converge 200 2 50 200 + * 15000xyz` silently parses as 15000 and ignores the suffix. Skip + * whitespace and reject anything else so a typo surfaces as a + * deterministic failure (codex-flagged). + */ + if (cursor) { + while (*cursor == ' ' || *cursor == '\t') + cursor++; + if (*cursor != '\0' && + strncmp(cursor, "failure-marker=", 15) != 0) { + fprintf(stderr, + "replay: line %d: wait-converge trailing garbage " + "after 5 args: '%s'\n", + lineno, cursor); + cursor = NULL; + } + } if (!cursor) { /* Malformed input. Break out of the script so the runner cannot * proceed past the failed synchronization point with the rest @@ -269,24 +378,38 @@ static void runScript(const char *path) unsigned long long divergedBuckets = fields[2]; unsigned long long thresholdPerBucket = fields[3]; unsigned long long timeoutMs = fields[4]; + const char *failurePath = + cursor && *cursor != '\0' ? cursor + 15 : NULL; uint64_t anchorMs = replayWaitConvergeAnchorMs((uint64_t) timeoutMs); int rc = timelineWaitConverge( anchorMs, (uint64_t) bucketMs, (int) quietBuckets, (int) divergedBuckets, (uint64_t) thresholdPerBucket, (uint64_t) timeoutMs); - if (rc < 0) + if (rc < 0) { fprintf(stderr, "replay: line %d: wait-converge diverged after " "%llu ms windows\n", lineno, bucketMs * divergedBuckets); - else if (rc == 0) + if (failurePath) + writeWaitConvergeFailure(failurePath, lineno, "diverged"); + } else if (rc == 0) { fprintf(stderr, "replay: line %d: wait-converge timed out after " "%llu ms\n", lineno, timeoutMs); - if (rc <= 0) - break; + if (failurePath) + writeWaitConvergeFailure(failurePath, lineno, "timeout"); + } + /* Do not break the script on timeout/divergence (rc <= 0). The + * runner's expanded replay puts a state-snapshot line immediately + * after every wait-converge and blocks waiting for the JSON + * marker. Breaking here leaves the marker unwritten, and the runner + * spends the full timeout plus slack polling before raising. + * Letting the next line run writes the JSON (with the current, + * possibly unsettled state) so the runner can observe the failure + * marker above instead of hanging on missing synchronization JSON. + */ } else if (!strcmp(cmd, "state-snapshot")) { /* Marshal the in-process focus / grab / window / property state to * a JSON file. Synchronous so a follow-up assert-state in the diff --git a/src/snapshot.c b/src/snapshot.c index 1f456a5..26f1743 100644 --- a/src/snapshot.c +++ b/src/snapshot.c @@ -42,17 +42,153 @@ static int snapshotDone = 0; * empirically: raw SDL_USEREVENT pushes silently dropped before convertEvent * saw them). Register lazily on first request so snapshot delivery shares the * per-process event-type registry with enqueueEvent's INTERNAL_EVENT_CODE pump. + * + * SDL_atomic-backed so snapshotOwnsEventType can read it from the SDL filter + * dispatch path without holding snapshotMutex. Plain int reads raced the writer + * that sets it under the mutex on first use; SDL atomics give an + * acquire/release barrier matching the fix already applied to state-snapshot. + */ +static SDL_atomic_t snapshotEventType = {(int) (Uint32) -1}; + +/* Generation counter for in-flight requests. Each call to a request function + * increments it inside snapshotMutex; the value is packed into the envelope on + * the SDL_USEREVENT and re-checked inside the handler. A request that times out + * leaves its event stranded in the queue; when the main thread eventually + * drains it, the generation mismatch makes the handler skip signaling so it + * does not satisfy a fresh waiter with the stale result. + */ +static uint64_t snapshotGeneration = 0; + +/* Envelope passed via SDL_USEREVENT.user.data1. Carries the generation plus the + * per-code payload (path for snapshot, dimensions for resize, coords for + * focus-at). Heap-allocated in the request function and freed in the handler, + * including the stale-event path so timed-out requests do not leak. */ -static Uint32 snapshotEventType = (Uint32) -1; +typedef struct SnapshotEnvelope { + uint64_t generation; + int intA; + int intB; + char *path; +} SnapshotEnvelope; Bool snapshotOwnsEventType(Uint32 eventType) { - return snapshotEventType != (Uint32) -1 && eventType == snapshotEventType; + Uint32 registered = (Uint32) SDL_AtomicGet(&snapshotEventType); + return registered != (Uint32) -1 && eventType == registered; +} + +/* Lazy-register the shared SDL user-event type, arm the done/result pair, and + * advance the generation counter for a fresh round-trip. Performed under + * snapshotMutex so two concurrent first-callers cannot each grab their own type + * id or skew the generation. + * + * Returns the registered SDL event type plus the generation via the out params + * on success; the function result is False when SDL_RegisterEvents failed and + * the caller should bail with -2. The tag string is used only for the failure + * LOG so each call site keeps its own diagnostic prefix. + */ +static Bool prepareSnapshotRoundTrip(const char *tag, + Uint32 *eventTypeOut, + uint64_t *generationOut) +{ + pthread_mutex_lock(&snapshotMutex); + Uint32 registered = (Uint32) SDL_AtomicGet(&snapshotEventType); + if (registered == (Uint32) -1) { + Uint32 newType = SDL_RegisterEvents(1); + if (newType == (Uint32) -1) { + pthread_mutex_unlock(&snapshotMutex); + LOG("%s: SDL_RegisterEvents failed: %s\n", tag, SDL_GetError()); + return False; + } + SDL_AtomicSet(&snapshotEventType, (int) newType); + registered = newType; + } + snapshotDone = 0; + snapshotResult = 0; + snapshotGeneration++; + *eventTypeOut = registered; + *generationOut = snapshotGeneration; + pthread_mutex_unlock(&snapshotMutex); + return True; +} + +/* Roll back the generation when SDL_PushEvent fails so a subsequent request + * does not skip past the stale-event window. + */ +static void rollbackSnapshotGeneration(uint64_t generation) +{ + pthread_mutex_lock(&snapshotMutex); + if (snapshotGeneration == generation) + snapshotGeneration--; + pthread_mutex_unlock(&snapshotMutex); +} + +/* Test the incoming envelope's generation against the current counter under + * snapshotMutex. + * + * Returns True when the event is stale and the handler should free the envelope + * and skip signaling. The handler still owns the envelope in both branches. + */ +static Bool snapshotEventIsStale(uint64_t eventGeneration) +{ + pthread_mutex_lock(&snapshotMutex); + Bool stale = eventGeneration != snapshotGeneration; + pthread_mutex_unlock(&snapshotMutex); + return stale; +} + +/* Store the main-thread handler's result and wake the worker waiting in + * waitSnapshotResult. Identical tail across all three handlers; the original + * gotos remain for clarity at the call sites. The generation is rechecked + * inside the mutex so a fresh request that bumped the counter while the handler + * was executing has the stale result quietly dropped. + */ +static void signalSnapshotResult(uint64_t eventGeneration, int rc) +{ + pthread_mutex_lock(&snapshotMutex); + if (eventGeneration == snapshotGeneration) { + snapshotResult = rc; + snapshotDone = 1; + pthread_cond_signal(&snapshotCond); + } + pthread_mutex_unlock(&snapshotMutex); +} + +/* Block up to 15s for the main thread to finish the round-trip. 15s covers + * Motif/Xt reflow bursts (resize, freshly mapped XmMainWindow, focus-driven + * traversal) without masking a genuinely stuck main thread. waitRcOut, if + * non-NULL, receives the underlying pthread_cond_timedwait return so callers + * can distinguish timeout from spurious wake in their diagnostic LOG. + */ +static int waitSnapshotResult(int *waitRcOut) +{ + struct timespec deadline; + clock_gettime(CLOCK_REALTIME, &deadline); + deadline.tv_sec += 15; + pthread_mutex_lock(&snapshotMutex); + int waitRc = 0; + while (!snapshotDone && waitRc == 0) + waitRc = + pthread_cond_timedwait(&snapshotCond, &snapshotMutex, &deadline); + int rc = snapshotDone ? snapshotResult : -4; + pthread_mutex_unlock(&snapshotMutex); + if (waitRcOut) + *waitRcOut = waitRc; + return rc; } int snapshotHandleEvent(const SDL_Event *event) { - char *path = (char *) event->user.data1; + SnapshotEnvelope *env = (SnapshotEnvelope *) event->user.data1; + if (snapshotEventIsStale(env->generation)) { + LOG("snapshot: dropping stale event gen=%llu cur=%llu\n", + (unsigned long long) env->generation, + (unsigned long long) snapshotGeneration); + free(env->path); + free(env); + return -5; + } + char *path = env->path; int rc = 0; Uint32 winId = replayTargetWindowId(); SDL_Window *win = (winId != 0) ? SDL_GetWindowFromID(winId) : NULL; @@ -75,12 +211,9 @@ int snapshotHandleEvent(const SDL_Event *event) } LOG("snapshot: wrote %s (%dx%d)\n", path, surface->w, surface->h); signal: - pthread_mutex_lock(&snapshotMutex); - snapshotResult = rc; - snapshotDone = 1; - pthread_cond_signal(&snapshotCond); - pthread_mutex_unlock(&snapshotMutex); + signalSnapshotResult(env->generation, rc); free(path); + free(env); return rc; } @@ -91,8 +224,16 @@ int snapshotHandleEvent(const SDL_Event *event) */ int snapshotHandleResizeEvent(Display *display, const SDL_Event *event) { - int width = (int) (intptr_t) event->user.data1; - int height = (int) (intptr_t) event->user.data2; + SnapshotEnvelope *env = (SnapshotEnvelope *) event->user.data1; + if (snapshotEventIsStale(env->generation)) { + LOG("resize: dropping stale event gen=%llu cur=%llu\n", + (unsigned long long) env->generation, + (unsigned long long) snapshotGeneration); + free(env); + return -5; + } + int width = env->intA; + int height = env->intB; int rc = 0; Uint32 winId = replayTargetWindowId(); SDL_Window *win = (winId != 0) ? SDL_GetWindowFromID(winId) : NULL; @@ -111,11 +252,8 @@ int snapshotHandleResizeEvent(Display *display, const SDL_Event *event) postSyntheticWindowResize(display, xwin, width, height); } signal: - pthread_mutex_lock(&snapshotMutex); - snapshotResult = rc; - snapshotDone = 1; - pthread_cond_signal(&snapshotCond); - pthread_mutex_unlock(&snapshotMutex); + signalSnapshotResult(env->generation, rc); + free(env); return rc; } @@ -123,46 +261,105 @@ int snapshotRequestResizeAndWait(int width, int height) { if (width <= 0 || height <= 0) return -1; - pthread_mutex_lock(&snapshotMutex); - if (snapshotEventType == (Uint32) -1) { - snapshotEventType = SDL_RegisterEvents(1); - if (snapshotEventType == (Uint32) -1) { - pthread_mutex_unlock(&snapshotMutex); - LOG("resize: SDL_RegisterEvents failed: %s\n", SDL_GetError()); - return -2; - } + SnapshotEnvelope *env = calloc(1, sizeof(*env)); + if (!env) + return -1; + env->intA = width; + env->intB = height; + Uint32 eventType = 0; + uint64_t generation = 0; + if (!prepareSnapshotRoundTrip("resize", &eventType, &generation)) { + free(env); + return -2; } - snapshotDone = 0; - snapshotResult = 0; - pthread_mutex_unlock(&snapshotMutex); + env->generation = generation; SDL_Event ev; SDL_zero(ev); - ev.type = snapshotEventType; + ev.type = eventType; ev.user.code = RESIZE_EVENT_CODE; - ev.user.data1 = (void *) (intptr_t) width; - ev.user.data2 = (void *) (intptr_t) height; + ev.user.data1 = env; int pushRc = SDL_PushEvent(&ev); LOG("resize: request %dx%d pushRc=%d\n", width, height, pushRc); - if (pushRc <= 0) + if (pushRc <= 0) { + rollbackSnapshotGeneration(generation); + free(env); return -2; + } wakeEventPipeForExternalEvent(NULL); - struct timespec deadline; - clock_gettime(CLOCK_REALTIME, &deadline); - /* Resize triggers a full reflow in Motif/Xt clients (ViolaWWW re-renders - * every visible paragraph), so the main thread may not drain SDL events for - * noticeably longer than a plain snapshot. 15s is generous enough to cover - * that without masking a real stuck-thread hang. + int waitRc = 0; + int rc = waitSnapshotResult(&waitRc); + LOG("resize: wait done rc=%d wait_rc=%d\n", rc, waitRc); + return rc; +} + +int snapshotHandleFocusAtEvent(Display *display, const SDL_Event *event) +{ + SnapshotEnvelope *env = (SnapshotEnvelope *) event->user.data1; + if (snapshotEventIsStale(env->generation)) { + LOG("focus-at: dropping stale event gen=%llu cur=%llu\n", + (unsigned long long) env->generation, + (unsigned long long) snapshotGeneration); + free(env); + return -5; + } + int x = env->intA; + int y = env->intB; + int rc = 0; + int rootX = 0, rootY = 0; + if (!replayTargetTranslateLocal(x, y, &rootX, &rootY)) { + LOG("focus-at: replayTargetTranslateLocal(%d, %d) failed\n", x, y); + rc = -1; + goto signal; + } + Window target = getContainingWindow(SCREEN_WINDOW, rootX, rootY); + /* Reject SCREEN_WINDOW because getContainingWindow falls back to the + * starting window when no child contains the point, which would shift X + * focus to the synthetic root. Reject None for the obvious reason. */ - deadline.tv_sec += 15; - pthread_mutex_lock(&snapshotMutex); - int wait_rc = 0; - while (!snapshotDone && wait_rc == 0) - wait_rc = - pthread_cond_timedwait(&snapshotCond, &snapshotMutex, &deadline); - int rc = snapshotDone ? snapshotResult : -4; - pthread_mutex_unlock(&snapshotMutex); - LOG("resize: wait done rc=%d wait_rc=%d snapshotDone=%d\n", rc, wait_rc, - snapshotDone); + if (target == None || target == SCREEN_WINDOW) { + LOG("focus-at: no real window at (%d, %d) -> (%d, %d)\n", x, y, rootX, + rootY); + rc = -1; + goto signal; + } + XSetInputFocus(display, target, RevertToNone, CurrentTime); + LOG("focus-at: focus -> 0x%lx at (%d, %d)\n", target, rootX, rootY); +signal: + signalSnapshotResult(env->generation, rc); + free(env); + return rc; +} + +int snapshotRequestFocusAtAndWait(int x, int y) +{ + SnapshotEnvelope *env = calloc(1, sizeof(*env)); + if (!env) + return -1; + env->intA = x; + env->intB = y; + Uint32 eventType = 0; + uint64_t generation = 0; + if (!prepareSnapshotRoundTrip("focus-at", &eventType, &generation)) { + free(env); + return -2; + } + env->generation = generation; + SDL_Event ev; + SDL_zero(ev); + ev.type = eventType; + ev.user.code = FOCUS_AT_EVENT_CODE; + ev.user.data1 = env; + int pushRc = SDL_PushEvent(&ev); + LOG("focus-at: request (%d, %d) pushRc=%d\n", x, y, pushRc); + if (pushRc <= 0) { + rollbackSnapshotGeneration(generation); + free(env); + return -2; + } + wakeEventPipeForExternalEvent(NULL); + int waitRc = 0; + int rc = waitSnapshotResult(&waitRc); + LOG("focus-at: wait done rc=%d wait_rc=%d\n", rc, waitRc); return rc; } @@ -170,57 +367,39 @@ int snapshotRequestAndWait(const char *path) { if (!path || !*path) return -1; - char *copy = strdup(path); - if (!copy) + SnapshotEnvelope *env = calloc(1, sizeof(*env)); + if (!env) + return -1; + env->path = strdup(path); + if (!env->path) { + free(env); return -1; - pthread_mutex_lock(&snapshotMutex); - /* Lazy registration inside the lock: two concurrent first callers would - * otherwise each get their own type id from SDL, leaking one id and pushing - * events that no one is registered to handle (gemini-flagged race). - */ - if (snapshotEventType == (Uint32) -1) { - snapshotEventType = SDL_RegisterEvents(1); - if (snapshotEventType == (Uint32) -1) { - pthread_mutex_unlock(&snapshotMutex); - LOG("snapshot: SDL_RegisterEvents failed: %s\n", SDL_GetError()); - free(copy); - return -2; - } } - snapshotDone = 0; - snapshotResult = 0; - pthread_mutex_unlock(&snapshotMutex); + Uint32 eventType = 0; + uint64_t generation = 0; + if (!prepareSnapshotRoundTrip("snapshot", &eventType, &generation)) { + free(env->path); + free(env); + return -2; + } + env->generation = generation; SDL_Event ev; SDL_zero(ev); - ev.type = snapshotEventType; + ev.type = eventType; ev.user.code = SNAPSHOT_EVENT_CODE; - ev.user.data1 = copy; + ev.user.data1 = env; int pushRc = SDL_PushEvent(&ev); - LOG("snapshot: request path=%s type=%u pushRc=%d\n", path, - snapshotEventType, pushRc); + LOG("snapshot: request path=%s type=%u pushRc=%d\n", path, eventType, + pushRc); if (pushRc <= 0) { - free(copy); + rollbackSnapshotGeneration(generation); + free(env->path); + free(env); return -2; } wakeEventPipeForExternalEvent(NULL); - /* Bound the wait so a stuck main thread doesn't hang the replay - * indefinitely. The save itself is single-ms for a typical window, but the - * main thread may not drain the SDL_USEREVENT for a while if Motif is - * mid-reflow (e.g. right after a resize, or during the initial-expose burst - * of a freshly mapped XmMainWindow). 15s covers that without masking a - * genuine stuck-thread hang. - */ - struct timespec deadline; - clock_gettime(CLOCK_REALTIME, &deadline); - deadline.tv_sec += 15; - pthread_mutex_lock(&snapshotMutex); - int wait_rc = 0; - while (!snapshotDone && wait_rc == 0) - wait_rc = - pthread_cond_timedwait(&snapshotCond, &snapshotMutex, &deadline); - int rc = snapshotDone ? snapshotResult : -4; - pthread_mutex_unlock(&snapshotMutex); - LOG("snapshot: wait done rc=%d wait_rc=%d snapshotDone=%d\n", rc, wait_rc, - snapshotDone); + int waitRc = 0; + int rc = waitSnapshotResult(&waitRc); + LOG("snapshot: wait done rc=%d wait_rc=%d\n", rc, waitRc); return rc; } diff --git a/src/snapshot.h b/src/snapshot.h index c2d661e..09867e2 100644 --- a/src/snapshot.h +++ b/src/snapshot.h @@ -44,4 +44,18 @@ Bool snapshotOwnsEventType(Uint32 eventType); */ int snapshotHandleResizeEvent(Display *display, const SDL_Event *event); +/* Resolve the X subwindow containing the replay-target-local point (x, y) and + * call XSetInputFocus on it. Blocks the caller until the lookup and focus + * mutation complete on the main thread, since both the window tree walk + * (getContainingWindow) and the focus state (keyboardFocus / postFocusChange) + * are unguarded against the main-thread mutators that build them. Used by the + * replay engine's focus-at verb. Returns 0 on success, -1 if the translation + * or containment lookup fails, -2 if the SDL_Event push failed. + */ +int snapshotRequestFocusAtAndWait(int x, int y); + +/* Handle a FOCUS_AT_EVENT_CODE SDL_USEREVENT on the main thread. Runs the + * tree walk and the XSetInputFocus call, then signals the waiter. */ +int snapshotHandleFocusAtEvent(Display *display, const SDL_Event *event); + #endif diff --git a/tests/check.c b/tests/check.c index 060c693..fc77bf3 100644 --- a/tests/check.c +++ b/tests/check.c @@ -2879,6 +2879,19 @@ static int test_events(Display *display) CHECK(out.type == Expose && out.xany.window == window, "unexpected window event"); + uint64_t exposeTaps = timelineCounter(TIMELINE_KIND_EXPOSE); + XSendEvent(display, window, False, ExposureMask, &expose); + XSendEvent(display, window, False, 0, &client); + CHECK(XCheckTypedEvent(display, ClientMessage, &out), + "XCheckTypedEvent did not find ClientMessage after Expose"); + CHECK(timelineCounter(TIMELINE_KIND_EXPOSE) == exposeTaps + 1, + "deferred converted Expose was not tapped once"); + XNextEvent(display, &out); + CHECK(out.type == Expose && out.xany.window == window, + "XNextEvent did not preserve deferred Expose"); + CHECK(timelineCounter(TIMELINE_KIND_EXPOSE) == exposeTaps + 1, + "deferred converted Expose was tapped twice"); + Window ignoredChild = XCreateSimpleWindow(display, window, 0, 0, 8, 8, 0, 0, 0); CHECK(ignoredChild != None, "child creation failed"); @@ -3048,9 +3061,13 @@ static int test_events(Display *display) CHECK(!XCheckTypedEvent(display, KeyPress, &out), "SDL_TEXTINPUT was converted into a duplicate KeyPress"); - /* Wheel up -> ButtonPress Button4 with current modifier state. */ + /* Wheel up -> ButtonPress + matching ButtonRelease for Button4. Real X11 + * always pairs wheel button events, so the conversion layer queues both; + * the test must drain both so leftover Releases do not pollute the + * put-back queue ahead of the crossing assertions further down. + */ SDL_Event wheelEvent; - XSelectInput(display, window, ButtonPressMask); + XSelectInput(display, window, ButtonPressMask | ButtonReleaseMask); SDL_zero(wheelEvent); wheelEvent.type = SDL_MOUSEWHEEL; wheelEvent.wheel.windowID = @@ -3061,6 +3078,10 @@ static int test_events(Display *display) CHECK(XCheckTypedEvent(display, ButtonPress, &out), "SDL_MOUSEWHEEL did not produce ButtonPress"); CHECK(out.xbutton.button == Button4, "wheel-up did not map to Button4"); + CHECK(XCheckTypedEvent(display, ButtonRelease, &out), + "SDL_MOUSEWHEEL did not produce paired ButtonRelease"); + CHECK(out.xbutton.button == Button4, + "wheel-up ButtonRelease did not match Button4"); SDL_zero(wheelEvent); wheelEvent.type = SDL_MOUSEWHEEL; @@ -3072,6 +3093,10 @@ static int test_events(Display *display) CHECK(XCheckTypedEvent(display, ButtonPress, &out), "wheel-down did not produce ButtonPress"); CHECK(out.xbutton.button == Button5, "wheel-down did not map to Button5"); + CHECK(XCheckTypedEvent(display, ButtonRelease, &out), + "wheel-down did not produce paired ButtonRelease"); + CHECK(out.xbutton.button == Button5, + "wheel-down ButtonRelease did not match Button5"); SDL_Event hintMotion; SDL_zero(hintMotion); diff --git a/tests/ui/replays/motif-fileview-done.replay b/tests/ui/replays/motif-fileview-done.replay index 8d3c14e..6eb7370 100644 --- a/tests/ui/replays/motif-fileview-done.replay +++ b/tests/ui/replays/motif-fileview-done.replay @@ -4,6 +4,11 @@ # press + delay + release for the LIBX11_COMPAT_REPLAY engine. delay 2500 wait-window "fileview|File" 3000 +# Let Motif's initial reflow drain before the first snapshot. fileview +# starts a language dialog whose pixmap/Expose churn can stretch past +# the prior fixed 500 ms ceiling, which is what surfaced the snapshot +# timeout in run-to-run nondeterminism. +wait-converge 200 2 50 200 15000 screenshot initial assert-image initial common-visible.json # target-motion is relative to the current replay target window, which @@ -13,6 +18,11 @@ assert-image initial common-visible.json # is tracked separately as an open compatibility gap. target-motion 119 480 button 1 click +# Keep this as a fixed delay (not wait-converge): on the native libX11 +# + xdotool differential path, fileview's post-Done callback can race +# into a SEGFAULT and a longer wait surfaces the crash, regressing +# check-differential-motif. The compat in-process snapshot path is +# already stabilized by the pre-screenshot wait-converge above. delay 500 screenshot after-click assert-image after-click common-visible.json