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

formats: teach the renderer to rotate (Option A) -- and report that it does not close the title

Browse Source 
The human chose Option A: teach sylpheed-formats own renderer to draw
rotation_deg so it and the port stay comparable and verify-screen keeps meaning
someone is wrong.

Three pieces, because rotation alone does nothing on the title. blit gains a
rotated path that draws by inverse mapping over the rotated bounding box, turning
about the pivot, whose absolute position is invariant under scale; zero rotation
keeps the original forward-mapped path byte for byte so non-rotating screens
cannot regress. compose draws a nested .rat leaf when the leaf carries geometry
the parent does not, which is the sweeps case, but not as a blanket rule since a
button s leaf duplicates its parent. And --at poses leaves at a keyframe time,
because the sweeps hold off-screen at x=1521 so a resting composite omits them.

A trap found the hard way: posing EVERYTHING at one global time is wrong, because
a top-level group s final keyframes are its exit ramp and rest() deliberately
stops before them. Posing the title at t=358 walked every parent into its exit
and drove the disagreement from 10.92 to 61.74. So at poses leaves only.

Controls: 0 and 360 degrees byte-identical to the unrotated path, 90 degrees
swaps a 10x4 to 4x10, area conserved within 15 percent, centroid stays on the
pivot. 116 lib tests pass, main_menu unchanged at 9.26.

And the verification did not show what it was meant to, which is reported rather
than buried: scanning the pose time against the title capture gives 10.73 to
11.17 against a 10.92 baseline -- flat, no minimum, best 1.7 percent. The
whole-frame mean is dominated by the tone curve, and the renderer still does not
draw ptlogo1/ptlogo2 at all, which is a far larger spatial gap than two
translucent sweeps. So rotation is correct in isolation and no screen regressed,
but whether it closes the port s 1.81 percent is not established here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QsEPXWVaEpyfudtR6re1Pd
This commit is contained in:
sylph-decoder
2026-08-29 18:59:26 +00:00
parent 21a2c649e6
commit 81f42f9041
5 changed files with 396 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
crates/sylpheed-cli/src/main.rs
View File

@@ -195,6 +195,14 @@ enum ScreenCommands {
/// `--build`**, which is why it is a flag and not the default.
#[arg(long)]
all: bool,
/// Pose every element at this KEYFRAME TIME instead of at its resting
/// pose (60 units = 1 second). The resting pose is the last *hold*
/// keyframe, which is the settled screen — wrong for anything still
/// moving. The title's two light sweeps hold off the right edge, so a
/// resting composite omits them; `--at 358` puts them where a capture
/// taken mid-sweep has them.
#[arg(long)]
at: Option<u32>,
},
}
@@ -353,8 +361,9 @@ async fn main() -> Result<()> {
black,
all,
primitives,
at,
} => cmd_screen_render(
&pak, &output, build, focus, animated, black, all, primitives,
&pak, &output, build, focus, animated, black, all, primitives, at,
),
},
Commands::Save { cmd } => match cmd {
@@ -581,6 +590,7 @@ fn cmd_screen_render(
black: bool,
all: bool,
primitives: bool,
at: Option<u32>,
) -> Result<()> {
use sylpheed_formats::ui_layout::{self, ComposeOptions};
let builds = screen_builds(pak, all)?;
@@ -599,6 +609,7 @@ fn cmd_screen_render(
ComposeOptions::default().backdrop
},
include_primitives: primitives,
at,
},
None,
);

20
crates/sylpheed-formats/examples/leafdbg.rs Normal file
View File

@@ -0,0 +1,20 @@
use sylpheed_formats::{pak, ui_layout};
fn main(){
let ar=pak::PakArchive::open(std::env::args().nth(1).unwrap()).unwrap();
let by=ar.read(&ar.entries()[4]).unwrap();
let b=ui_layout::parse_build(&by).unwrap();
for n in ["ptloop01.rat","ptloop02.rat"] {
let el=b.elements.iter().find(|e| e.name==n).unwrap();
println!("{n}: kind={:#x} animated={} rec={:?}", el.kind, el.animated,
b.records.get(n).map(|&(o,s)|(o,s)));
if let Some(&(o,s))=b.records.get(n) {
match ui_layout::parse_build(&by[o..o+s]) {
Some(lb)=>for le in &lb.elements {
println!(" leaf {:?} sprite={:?} in_sprites={}", le.name, le.sprite,
le.sprite.as_ref().map(|x| b.sprites.contains_key(x)).unwrap_or(false));
},
None=>println!(" leaf parse FAILED"),
}
}
}
}

9
crates/sylpheed-formats/examples/sprite_of.rs Normal file
View File

@@ -0,0 +1,9 @@
use sylpheed_formats::{pak, ui_layout};
fn main(){
let ar=pak::PakArchive::open(std::env::args().nth(1).unwrap()).unwrap();
let b=ui_layout::parse_build(&ar.read(&ar.entries()[4]).unwrap()).unwrap();
for el in b.elements.iter().filter(|e| e.name.contains("ptloop")||e.name.contains("ptbtn")) {
println!("{:<18} sprite={:?} has_leaf={}", el.name, el.sprite,
b.records.contains_key(&el.name));
}
}

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

@@ -192,6 +192,68 @@ impl Element {
///
/// Falls back to the longest-dwell rule when no two adjacent keyframes
/// agree — a group that ramps through every frame and never holds.
/// The element's pose at keyframe time `t`, linearly interpolated.
///
/// `rest()` returns the last HOLD keyframe, which is the settled screen. That
/// is the wrong pose for anything still moving: the title's light sweeps hold
/// at `x = 1521`, off the right edge, so a resting composite deletes them
/// rather than settling them. A capture taken mid-animation can only be
/// compared against a render posed at the same instant.
///
/// The ramp is linear (`docs/re/ui-keyframe-time-unit.md`), and a group
/// **holds** at its last keyframe rather than looping, so `t` past the end
/// clamps.
pub fn pose_at(&self, t: u32) -> Option<Keyframe> {
let ks = &self.keyframes;
if ks.is_empty() {
return None;
}
let timed: Vec<(u32, &Keyframe)> =
ks.iter().filter_map(|k| k.time.map(|tt| (tt, k))).collect();
if timed.is_empty() {
return Some(ks[ks.len() - 1].clone());
}
if t <= timed[0].0 {
return Some(timed[0].1.clone());
}
if t >= timed[timed.len() - 1].0 {
return Some(timed[timed.len() - 1].1.clone());
}
for w in timed.windows(2) {
let ((t0, a), (t1, b)) = (w[0], w[1]);
if t >= t0 && t <= t1 {
if t1 == t0 {
return Some(b.clone());
}
let f = (t - t0) as f64 / (t1 - t0) as f64;
let li = |x: i32, y: i32| x + ((y - x) as f64 * f).round() as i32;
let lu = |x: u32, y: u32| (x as f64 + (y as f64 - x as f64) * f).round() as u32;
// ARGB / RGBA words interpolate per BYTE, not as integers.
let lc = |x: u32, y: u32| {
let mut o = 0u32;
for sh in [24, 16, 8, 0] {
let (cx, cy) = ((x >> sh) & 0xff, (y >> sh) & 0xff);
o |= (lu(cx, cy) & 0xff) << sh;
}
o
};
return Some(Keyframe {
fade: lc(a.fade, b.fade),
rotation_deg: li(a.rotation_deg, b.rotation_deg),
unknown_4: li(a.unknown_4, b.unknown_4),
unknown_8: li(a.unknown_8, b.unknown_8),
scale_x: lu(a.scale_x, b.scale_x),
scale_y: lu(a.scale_y, b.scale_y),
tint: lc(a.tint, b.tint),
x: li(a.x, b.x),
y: li(a.y, b.y),
time: Some(t),
});
}
}
Some(timed[timed.len() - 1].1.clone())
}
pub fn rest(&self) -> Option<&Keyframe> {
// `lastall`: the LAST keyframe for every element, bypassing the plateau
// rule entirely. This is what the shifted time reading predicts — under
@@ -743,6 +805,12 @@ pub struct ComposeOptions {
/// Left on with the derived order, an opaque black quad sorts last and wipes
/// 32 of `GP_DIALOG`'s builds.
pub include_primitives: bool,
/// Pose every element at this keyframe time instead of at its resting pose.
///
/// `None` keeps the settled composite, which is what every existing caller
/// wants. A capture taken mid-animation needs the render posed at the same
/// instant — see [`Element::pose_at`].
pub at: Option<u32>,
}
impl Default for ComposeOptions {
@@ -752,6 +820,7 @@ impl Default for ComposeOptions {
include_animated: false,
backdrop: [14, 14, 20, 255],
include_primitives: false,
at: None,
}
}
}
@@ -1005,6 +1074,16 @@ pub fn compose_with_order(
{
continue;
}
// 🔴 `at` poses LEAVES ONLY, never the top-level elements.
//
// Posing everything at one global time was tried and is wrong: a
// top-level group's final keyframes are its **exit ramp** — the fade-out
// played when the screen leaves — and `rest()` deliberately stops at the
// last *hold* keyframe before it. Posing the title at t=358 walked every
// parent into its exit and drove the render's disagreement with the
// capture from 10.92 to 61.74. The leaf is the thing still animating at
// that instant, and it runs on its own timeline
// (`docs/re/structures/ui-leaf-vs-parent-alpha.md`).
let Some(kf) = el.rest() else { continue };
// An untextured primitive: a solid quad of the keyframe's `fade` colour,
// sized by the declared pivot. `kind & 0x10` marks these exactly — see
@@ -1037,6 +1116,56 @@ pub fn compose_with_order(
missing.push(sprite.clone());
continue;
};
// A nested `.rat` leaf sometimes carries the geometry while the parent
// carries none — the title's light sweeps are the case: the parent sits
// fixed at (441,270) scale 100 %, and the leaf holds the 600 %/800 %
// scale, the +30°/−45° rotation and the whole sweep. Drawing the parent
// put the sprite upright in the middle of the screen.
//
// ⚠️ NOT a blanket rule. A button's base record has a leaf that
// DUPLICATES it, and there the parent wins
// (`docs/re/structures/ui-button-focus-record.md`). The discriminator is
// which record actually carries geometry, so the leaf is used only when
// its pose genuinely differs — see `ui-leaf-vs-parent-alpha.md`.
let leaf = build.records.get(&el.name).and_then(|&(lo, ls)| {
if lo + ls > bundle.len() {
return None;
}
let lb = parse_build(&bundle[lo..lo + ls])?;
let pose = |e: &Element| match opts.at {
Some(t) => e.pose_at(t),
None => e.rest().cloned(),
};
let differs = lb.elements.iter().any(|le| {
pose(le).map_or(false, |lk| {
lk.rotation_deg != 0 || lk.scale_x != kf.scale_x || lk.scale_y != kf.scale_y
})
});
if differs { Some(lb) } else { None }
});
if let Some(lb) = leaf {
let mut any = false;
for le in &lb.elements {
let lk = match opts.at {
Some(t) => le.pose_at(t),
None => le.rest().cloned(),
};
let Some(lk) = lk else { continue };
// A leaf element resolves no sprite of its own: sprite names are
// resolved against the bundle a build was parsed from, and a leaf
// is parsed from its own slice. Its NAME is the sprite name, and
// the sprite itself lives in the PARENT bundle's table.
let lsp = le.sprite.clone().unwrap_or_else(|| le.name.clone());
let Some(&(so, ss)) = build.sprites.get(&lsp) else { continue };
let Some(limg) = t8ad::parse(&bundle[so..so + ss]) else { continue };
blit(&mut canvas, w, h, &limg, &lk, le.pivot_x, le.pivot_y);
any = true;
}
if any {
drawn.push(el.index);
continue;
}
}
blit(&mut canvas, w, h, &img, kf, el.pivot_x, el.pivot_y);
drawn.push(el.index);
}
@@ -1155,6 +1284,11 @@ fn blit(
// Keep the pivot point fixed as the element scales.
let ox = kf.x - (pivot_x as i32 * (sx_pct as i32 - 100)) / 100;
let oy = kf.y - (pivot_y as i32 * (sy_pct as i32 - 100)) / 100;
// The pivot's ABSOLUTE position is invariant under scale, which is the whole
// point of the two lines above: at 100 % `ox = kf.x` so the pivot sits at
// `kf.x + pivot_x`; at 200 % `ox = kf.x - pivot_x` and the pivot sits at
// `ox + 2·pivot_x`, the same place. So rotation turns about it.
let (pax, pay) = (kf.x + pivot_x as i32, kf.y + pivot_y as i32);
// Two modulate colours multiply into one: `tint` (RGBA, and `0xffffffff` on
// essentially every keyframe seen) and `fade` (**ARGB** — the high byte is
// the alpha that ramps, the low 24 bits a colour multiply that is `0xffffff`
@@ -1173,6 +1307,73 @@ fn blit(
((kf.tint >> 8) & 0xff) * fb / 255,
(kf.tint & 0xff) * fa / 255,
);
// ---- rotated path -------------------------------------------------------
// `+12` is a screen-plane rotation in DEGREES, clockwise-positive with Y
// down (`docs/re/structures/ui-keyframe-rotation.md`). The game submits
// rotated quads for it; this used to draw them axis-aligned, which put the
// title's two light sweeps upright instead of at +30° / −45° and left at
// least two thirds of that screen's disagreement with the capture
// (`title-residual-tone-vs-geometry.md`).
//
// Zero rotation keeps the original forward-mapped path byte for byte, so
// the screens that do not rotate cannot regress. A rotated element is drawn
// by INVERSE mapping instead: forward-mapping a rotation leaves gaps.
let rot = ((kf.rotation_deg % 360) + 360) % 360;
if rot != 0 {
let th = (rot as f64).to_radians();
let (cs, sn) = (th.cos(), th.sin());
// Axis-aligned bounds of the rotated destination rect.
let corners = [
(ox, oy),
(ox + dw as i32, oy),
(ox + dw as i32, oy + dh as i32),
(ox, oy + dh as i32),
];
let (mut x0, mut y0, mut x1, mut y1) = (i32::MAX, i32::MAX, i32::MIN, i32::MIN);
for (cx, cy) in corners {
let (rx, ry) = ((cx - pax) as f64, (cy - pay) as f64);
let px = pax as f64 + rx * cs - ry * sn;
let py = pay as f64 + rx * sn + ry * cs;
x0 = x0.min(px.floor() as i32);
y0 = y0.min(py.floor() as i32);
x1 = x1.max(px.ceil() as i32);
y1 = y1.max(py.ceil() as i32);
}
for ty in y0.max(0)..=y1.min(ch as i32 - 1) {
for tx in x0.max(0)..=x1.min(cw as i32 - 1) {
// Rotate the destination pixel BACK to find its source pixel.
let (rx, ry) = ((tx - pax) as f64 + 0.5, (ty - pay) as f64 + 0.5);
let ux = rx * cs + ry * sn;
let uy = -rx * sn + ry * cs;
let dx = ux + (pax - ox) as f64;
let dy = uy + (pay - oy) as f64;
if dx < 0.0 || dy < 0.0 || dx >= dw as f64 || dy >= dh as f64 {
continue;
}
let sxi = ((dx as u32) * sw / dw).min(sw - 1);
let syi = ((dy as u32) * sh / dh).min(sh - 1);
let si = ((syi * sw + sxi) * 4) as usize;
if si + 3 >= img.rgba.len() {
continue;
}
let sr = img.rgba[si] as u32 * tr / 255;
let sg = img.rgba[si + 1] as u32 * tg / 255;
let sb = img.rgba[si + 2] as u32 * tb / 255;
let sa = img.rgba[si + 3] as u32 * ta / 255;
if sa == 0 {
continue;
}
let di = ((ty as u32 * cw + tx as u32) * 4) as usize;
for (k, sc) in [sr, sg, sb].into_iter().enumerate() {
let dc = canvas[di + k] as u32;
canvas[di + k] = ((sc * sa + dc * (255 - sa)) / 255) as u8;
}
canvas[di + 3] = 255;
}
}
return;
}
// ---- unrotated path (unchanged) -----------------------------------------
for row in 0..dh {
let ty = oy + row as i32;
if ty < 0 {
@@ -1220,6 +1421,70 @@ fn blit(
mod tests {
use super::*;
/// A solid opaque rectangle sprite, for exercising `blit` geometry.
fn solid(w: u32, h: u32) -> t8ad::T8adImage {
t8ad::T8adImage { width: w, height: h, rgba: vec![255u8; (w * h * 4) as usize],
flags: 0 }
}
fn kf_at(x: i32, y: i32, rot: i32) -> Keyframe {
Keyframe { fade: 0xff_ff_ff_ff, rotation_deg: rot, unknown_4: 0, unknown_8: 0,
scale_x: 100, scale_y: 100, tint: 0xffff_ffff, x, y, time: Some(0) }
}
fn draw(img: &t8ad::T8adImage, kf: &Keyframe, px: u32, py: u32) -> Vec<u8> {
let mut c = vec![0u8; 64 * 64 * 4];
blit(&mut c, 64, 64, img, kf, px, py);
c
}
fn covered(c: &[u8]) -> Vec<(i32, i32)> {
let mut v = Vec::new();
for y in 0..64 { for x in 0..64 {
if c[((y * 64 + x) * 4 + 3) as usize] != 0 { v.push((x as i32, y as i32)); }
}}
v
}
/// 🔴 CONTROL for the rotated path. An estimator that is wrong on a known
/// angle cannot be trusted on an unknown one, so the rotated blit is pinned
/// against angles whose answer is arithmetic rather than measured.
#[test]
fn rotation_control_known_angles() {
let img = solid(10, 4);
// pivot at the sprite's centre, so rotation turns in place
let (px, py) = (5u32, 2u32);
let base = draw(&img, &kf_at(20, 30, 0), px, py);
// 0° and 360° must be identical to the unrotated path, byte for byte:
// the fast path must be exactly the old behaviour.
assert_eq!(base, draw(&img, &kf_at(20, 30, 360), px, py),
"360 degrees must equal the unrotated path exactly");
// 90° must turn a 10x4 into a 4x10 about the same centre.
let r90 = covered(&draw(&img, &kf_at(20, 30, 90), px, py));
let b = covered(&base);
let bw = b.iter().map(|p| p.0).max().unwrap() - b.iter().map(|p| p.0).min().unwrap();
let bh = b.iter().map(|p| p.1).max().unwrap() - b.iter().map(|p| p.1).min().unwrap();
let rw = r90.iter().map(|p| p.0).max().unwrap() - r90.iter().map(|p| p.0).min().unwrap();
let rh = r90.iter().map(|p| p.1).max().unwrap() - r90.iter().map(|p| p.1).min().unwrap();
assert_eq!((bw, bh), (9, 3), "unrotated extent");
assert_eq!((rw, rh), (3, 9), "90 degrees must swap the extents");
// The covered area must be conserved to a few percent -- a rotation that
// loses or invents pixels is the forward-mapping bug this path avoids.
let (a0, a90) = (b.len() as f64, r90.len() as f64);
assert!((a0 - a90).abs() / a0 < 0.15,
"area changed too much under rotation: {a0} -> {a90}");
// And the centroid must stay on the pivot.
let cen = |v: &Vec<(i32, i32)>| {
let n = v.len() as f64;
(v.iter().map(|p| p.0 as f64).sum::<f64>() / n,
v.iter().map(|p| p.1 as f64).sum::<f64>() / n)
};
let (c0, c9) = (cen(&b), cen(&r90));
assert!((c0.0 - c9.0).abs() < 1.0 && (c0.1 - c9.1).abs() < 1.0,
"rotation moved the centroid: {c0:?} -> {c9:?}");
}
/// A synthetic build bundle: RATC magic, entry count at 0x14, a declaration
/// table at 0x20, then a placement region.
fn synth_build(decls: &[(&str, u32, u32, u32, u32)], groups: &[(usize, Vec<Keyframe>)]) -> Vec<u8> {

90
docs/re/structures/ui-rotation-implemented.md Normal file
View File

@@ -0,0 +1,90 @@
# ✅ Option A implemented — the reference renderer rotates. ⚠️ It does not close the title.
**Decision:** the human chose **Option A** (2026-08-29) — teach
`sylpheed-formats`' own renderer to draw `rotation_deg`, so it and the port stay
comparable and `verify-screen` keeps meaning *"someone is wrong"*.
**Status:** ✅ implemented and controlled. 🔴 **and it does not measurably improve
the title against the capture we hold** — reported here rather than quietly, because
the improvement was the reason for doing it.
## What changed
Three pieces, because rotation alone does nothing on the title:
1. **`blit` gained a rotated path.** `rotation_deg != 0` draws by **inverse
mapping** over the rotated bounding box; forward-mapping a rotation leaves
gaps. Rotation turns about the element's **pivot**, whose absolute position
`(kf.x + pivot_x, kf.y + pivot_y)` is invariant under scale.
✅ `rotation_deg == 0` keeps the original forward-mapped path **byte for
byte**, so screens that do not rotate cannot regress.
2. **`compose` draws a nested `.rat` leaf when the leaf carries geometry the
parent does not** — the title's sweeps are exactly that case (parent fixed at
(441,270) scale 100 %, leaf holding 600 %/800 % and ±30°/−45°).
⚠️ Not a blanket rule: a button's leaf *duplicates* its parent and the parent
wins ([`ui-leaf-vs-parent-alpha.md`](ui-leaf-vs-parent-alpha.md)), so the leaf
is used only when its pose genuinely differs.
⚠️ A leaf element resolves **no sprite of its own** — names resolve against
the bundle a build was parsed from, and a leaf is parsed from its own slice.
Its *name* is the sprite name, looked up in the parent bundle's table.
3. **`--at <units>` / `ComposeOptions::at`**, because the sweeps hold off-screen
at `x = 1521` and a resting composite therefore *omits* them.
## 🔴 A trap this found, and it cost a wrong number first
Posing **everything** at one global time is wrong. A top-level group's final
keyframes are its **exit ramp** — the fade-out played when the screen leaves —
and `rest()` deliberately stops at the last *hold* keyframe before it. Posing the
title at t=358 walked every parent into its exit and drove the disagreement from
**10.92 to 61.74**.
✅ `at` therefore poses **leaves only**; top-level elements keep `rest()`. That
follows the decoded rule directly: the leaf runs on its own timeline and the
parent's does not gate it.
## The controls
| | |
|---|---|
| 0° and **360°** vs the unrotated path | **byte-identical** |
| 90° on a 10×4 sprite | extents swap to **4×10** |
| covered area under rotation | conserved to **< 15 %** |
| centroid under rotation | stays on the pivot (< 1 px) |
`rotation_control_known_angles` pins all four. **116 lib tests pass.**
## 🔴 The verification, which did not show what it was meant to
Rendering the title against
[`live-title-build4-no-plate.png`](../captures/title-builds/live-title-build4-no-plate.png)
and scanning the pose time:
| | mean abs difference |
|---|---|
| before (rest, no leaves, no rotation) | **10.92** |
| after, scanned t = 0 … 600 | **10.73 – 11.17** |
| best (t = 420) | 10.73 — **1.7 %** better |
**Flat. No minimum.** Drawing the sweeps correctly does not measurably improve
this comparison, and two things explain why without rescuing it:
* the whole-frame mean is dominated by the **tone curve**, which
[`title-residual-tone-vs-geometry.md`](title-residual-tone-vs-geometry.md)
measures as the larger part of the *level* difference even where geometry is
right;
* our renderer still **does not draw `ptlogo1` / `ptlogo2` at all** (four
elements, reported as "not drawn"), and that is a far larger spatial gap than
two translucent sweeps.
⚠️ **So the honest claim is narrow:** rotation is implemented and correct in
isolation, and no screen regressed. Whether it closes the port's **1.81 % of
pixels differing** is **not established here** — that harness poses deliberately
and counts differing pixels rather than mean level, and it is the place to judge
it. ❔ The sweeps may simply be a small term.
## No regression
| screen | before | after |
|---|---|---|
| `main_menu` | 9.26 | **9.26** |
| `extras` | — | 9.75 |