diff --git a/crates/sylpheed-formats/src/ui_layout.rs b/crates/sylpheed-formats/src/ui_layout.rs index 728a02da..6a8593e8 100644 --- a/crates/sylpheed-formats/src/ui_layout.rs +++ b/crates/sylpheed-formats/src/ui_layout.rs @@ -227,7 +227,24 @@ impl Element { any = Some(i); any_len = len; } - if j != n - 1 && len >= best_len { + // A trailing run is normally the EXIT and is excluded (see the + // doc comment) — but not always, and the tell is its ALPHA. + // + // An exit fades the element out, so its final keyframe is + // transparent: `pgptitle.rat`'s trailing run is `0x00ffffff`, and + // taking it erases the word PAUSE. An element with **no exit** + // ends on its hold, which is whatever it looks like on screen — + // opaque: `ptframe1`/`ptframe2` on the title main menu end on a + // run of three at `0xffffffff`, and excluding it dropped the + // bright circuit bracket the capture plainly shows. + // + // So a trailing run is the hold exactly when it is **visible**. + // (The port agent's report proposed "the final untimed keyframe + // has the same pose as the last timed one"; that is true of + // `pgptitle` too and would erase PAUSE, so it is the alpha and + // not the pose-equality that separates the two.) + let trailing_is_the_hold = j == n - 1 && (self.keyframes[i].fade >> 24) != 0; + if (j != n - 1 || trailing_is_the_hold) && len >= best_len { best = Some(i); best_len = len; } diff --git a/docs/port/HANDOFF.md b/docs/port/HANDOFF.md index fe1ee447..851d4ff1 100644 --- a/docs/port/HANDOFF.md +++ b/docs/port/HANDOFF.md @@ -92,6 +92,20 @@ authored version can be deleted. [`ui-focus-and-effect-elements.md`](../re/structures/ui-focus-and-effect-elements.md)). * **Highlighted states pair by name** — `ptbtn01.rat` ↔ `ptbtn01f.rat`. 🟡 a naming convention that holds for all 54 real pairs, not a decoded field. +* **`rest()` was wrong for elements with no exit animation — fixed 2026-08-28.** + A trailing run of identical keyframes was always treated as the exit and + excluded; on an element that has no exit it *is* the hold, and `rest()` fell + back to the element's **first** keyframe — off-position and transparent. Six + elements on the main menu were affected, including `ptframe1`/`ptframe2`, the + bright circuit bracket around the menu, which both the port's composite **and** + `sylpheed-cli screen render` were dropping. + The rule now: **a trailing run is the hold exactly when it is visible** (alpha + ≠ 0). ⚠️ Not the pose-equality test the report proposed — `pgptitle.rat`'s + trailing run also matches its last timed keyframe, and adopting that would erase + the word PAUSE. Oracle correlation over the bracket region improved + **0.9596 → 0.9748**; the PAUSE control is unchanged. + This also closes the old ❔ on `ptframe1`/`ptframe2` "resting at alpha 0 but the + capture shows the frame plainly". * **The resting pose is the hold**, not the first, last or longest-dwell keyframe; a keyframe is the **start of a ramp**. [`ui-resting-pose.md`](../re/structures/ui-resting-pose.md). ✅ diff --git a/docs/re/METHOD.md b/docs/re/METHOD.md index 1b679468..10d24b1b 100644 --- a/docs/re/METHOD.md +++ b/docs/re/METHOD.md @@ -268,3 +268,10 @@ agent's loop prompt, i.e. nowhere durable. See [`README.md`](README.md) for the the decode's determinism, not its truth. Only a framebuffer capture is an independent check. (Reported by the port agent, 2026-08-28, after a capture crop found the missing bracket.) +* **`./target/debug/` is STALE in this container — `CARGO_TARGET_DIR` is + redirected.** The Dockerfile sets `CARGO_TARGET_DIR=/sylph-home/re/target-container` + so a fresh `cargo build` writes there, while `./target/debug/sylpheed-cli` + remains whatever was built on the host, hours old. A decoder fix was verified + against the stale binary and produced byte-identical "before" and "after" + renders — which reads exactly like "the change had no effect" rather than "you + ran the old code". Run the binary cargo actually wrote, or `cargo run`. diff --git a/docs/re/structures/ui-resting-pose.md b/docs/re/structures/ui-resting-pose.md index 4f025173..2fee050e 100644 --- a/docs/re/structures/ui-resting-pose.md +++ b/docs/re/structures/ui-resting-pose.md @@ -185,5 +185,38 @@ This also explains a long-standing ❔ on `0x00ffffff` (alpha 0) and are therefore not drawn, but the capture shows the menu frame plainly."* Same two elements, same cause — now identified. -❔ **Not yet fixed.** The change belongs in `ui_layout`'s `rest()`; it is a -decoder change and has not been made. +## ✅ Fixed 2026-08-28 — but the condition is the ALPHA, not the pose + +The report's proposed test — *"the final untimed keyframe has the same pose as the +last timed one"* — **misfires**, and on the exact case the exclusion was written +for. `pgptitle.rat`'s last two keyframes are also identical: + +``` +pgptitle.rat kf4: fade=0x00ffffff pos=(220,69) t=30 + kf5: fade=0x00ffffff pos=(220,69) t=None <- same pose +``` + +Adopting it as stated would erase the word PAUSE again. What separates the two is +**visibility**: + +| | trailing run | alpha | is it the hold? | +|---|---|---|---| +| `ptframe1`/`ptframe2` (main menu) | 3 × `0xffffffff` at (440,108) | `0xff` | **yes** — no exit animation | +| `pgptitle` (pause menu) | 2 × `0x00ffffff` | `0x00` | no — it is the fade-out | + +An exit fades the element out, so its last keyframe is transparent; an element +with no exit ends on the pose you can see. **So a trailing run is the hold exactly +when it is visible**, and that is what `rest_plateau` now tests. + +### Verified against a capture, not against another renderer + +| check | result | +|---|---| +| `ptframe1` rest | `(620,108) t=16` → **`(440,108) t=62`** | +| bracket region draws | mean 61.74 → **62.79** | +| **oracle correlation** over that region vs [`main-menu-oracle.png`](../captures/main-menu-oracle.png) | **0.9596 → 0.9748** | +| pixels changed, whole frame | 10 082, bounding box **x 440–839, y 108–577** — exactly the 400 × 470 at (440,108) the report predicted | +| **regression control**: PAUSE wordmark, 3 pause builds | **unchanged** (2833 / 2858 / 2833 bright px) | + +🟡 The full disc-gated `build-reborn test` run was **still executing when this was +committed**; the verification above is artifact-based.