fabi/Sylpheed
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

re: rest() fixed -- a trailing keyframe run is the hold when it is

Browse Source 
VISIBLE, not when its pose repeats

The port agent's report was right about the defect and about which
elements it hits, and its proposed condition does not survive the case
the exclusion was written for.

Their test -- the final untimed keyframe has the same pose as the last
timed one -- is also true of pgptitle.rat, whose trailing run is two
identical transparent frames. Adopting it would erase the word PAUSE
again, which a committed capture disproves. What separates the two is
alpha: an exit fades the element out so its last keyframe is transparent,
while an element with no exit ends on the pose you can see. So
rest_plateau now accepts a trailing run exactly when it is visible.

Verified against a capture rather than against another renderer, which is
the point their own report made: ptframe1's rest moves from (620,108)
t=16 to (440,108) t=62; the changed pixels are 10082 in a bounding box of
x 440-839, y 108-577, which is exactly the 400x470 at (440,108) they
predicted; and correlation against main-menu-oracle.png over that region
improves from 0.9596 to 0.9748. The PAUSE wordmark is unchanged across
all three pause builds.

This also closes a question ui-paint-order-key.md has carried for a
while, that ptframe1 and ptframe2 rest at alpha 0 while the capture shows
the menu frame plainly.

One trap cost most of this iteration and goes in METHOD: CARGO_TARGET_DIR
is redirected in this container, so ./target/debug/sylpheed-cli is hours
stale and every render I made against it was old code. Byte-identical
before and after reads as "no effect" when it actually means "you ran the
old binary".

The full disc-gated test suite was still executing when this was
committed; the verification above is artifact-based.
This commit is contained in:
Sylpheed RE agent
2026-08-28 21:19:00 +00:00
parent dcf2090d0f
commit 5e6cf0db92
4 changed files with 74 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
crates/sylpheed-formats/src/ui_layout.rs
View File

@@ -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;
}

14
docs/port/HANDOFF.md
View File

@@ -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). ✅

7
docs/re/METHOD.md
View File

@@ -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/<bin>` 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`.

37
docs/re/structures/ui-resting-pose.md
View File

@@ -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.