Files
Sylpheed/crates/sylpheed-export/src/check.rs
Sylpheed port agent ace071dd7b port: delete exit_ramp_units, invert the format's own rule, and guard a scale-0 leaf
FOUR THINGS, and the first is what MISSION section 3 calls the measure of
progress.

DELETED `exit_ramp_units` AND `exit_ramp_seconds`. They were authored because the
disc had no time slot on a group's final keyframe, so the ramp into it was the
one unknown duration per screen. Under the corrected record layout that keyframe
does not exist -- a group is an 8-byte header then frames x {u32 time; 36-byte
pose} and every pose is timed. VERIFIED DEAD BEFORE DELETING: setting it to 9999
(166 s) moved the boot's transitions by 0.04 s, which is wall-clock jitter, and
both uses in ScreenView are gated on a condition that no longer fires on any of
the export's 866 keyframes.

INVERTED THE FORMAT'S OWN RULE. `check.rs` enforced "the final keyframe has no
`t`; the disc has no time slot there" and FORMAT.md stated it. Both are now
backwards, and the validator fired 150 times on a re-export. I had not run
`check` between pinning the tag and measuring against the oracle -- the pixel
harness was green while the format validator was failing on every screen with a
multi-keyframe group. A correctness harness does not replace a format one; they
fail at different layers.

GUARDED A SCALE-0 LEAF, which the Decoder hit in its own renderer: its leaf
branch marked the element drawn unconditionally while the blit returned early on
zero scale, so a scale-0 leaf suppressed its parent and blanked the element --
live on all four loading screens. This port did not have the bug only because
authored/rendering.json happens not to list pgloading_loop5. That is an accident
of a gate written for another reason, not a defence, so `_draw_leaf` now reports
whether it drew and `_draw` falls back to the parent.

ISOLATED THE PACING QUESTION rather than leaving it as a suspected regression.
Legacy association: publisher 4.70 agrees, developer 3.92 DIFFERS. Corrected:
publisher 4.26 DIFFERS, developer 3.62 agrees. Both misses are ~0.03 s outside a
composite bound. The association traded which screen is marginally out; it did
not regress the pacing.

Bumped the pin c -> d for the parser and audio changes. Its headline renderer
change does not reach this port: sylpheed-cli builds from the workspace crate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N7FiFFFwbvG2uxdcEh8HyF
2026-08-29 19:19:46 +00:00

383 lines
17 KiB
Rust

//! `sylpheed-export check` — validate an export tree against `docs/FORMAT.md`.
//!
//! This is the executable form of FORMAT.md, and the reason it exists is that
//! "the export is correct" is otherwise an assertion. It reads `export/` the way
//! the Godot project will — as a stranger, with no access to the disc, the
//! decoders or this exporter's internals — and fails on anything a consumer
//! could not act on:
//!
//! * a document whose `format` is not the version this build writes;
//! * a required field missing, or a colour that is not `0x` + 8 hex digits;
//! * a `paint_order` that is not a permutation of the element indices;
//! * a `buttons` entry naming an element that is not a button, or out of
//! resting-Y order;
//! * a sprite path that does not exist, or a PNG that does not decode;
//! * a name presented as recovered when it was authored;
//! * an audio file that is silent or clips -- the two audio failures that pass
//! every check that is not looking for them.
//!
//! It deliberately does **not** check that the export matches the disc. That is
//! what `sylpheed-cli screen render` is for.
use anyhow::{bail, Result};
use serde_json::Value;
use std::path::Path;
const SCREEN_FORMAT: &str = "sylpheed.screen/3";
const MANIFEST_FORMAT: &str = "sylpheed.manifest/1";
struct Ctx {
file: String,
errors: Vec<String>,
}
impl Ctx {
fn err(&mut self, msg: impl Into<String>) {
self.errors.push(format!("{}: {}", self.file, msg.into()));
}
fn require<'a>(&mut self, v: &'a Value, key: &str) -> Option<&'a Value> {
match v.get(key) {
Some(Value::Null) | None => {
self.err(format!("missing required field `{key}`"));
None
}
Some(x) => Some(x),
}
}
}
/// A colour is exported as `0x` + 8 hex digits, with its byte order in the key
/// name. Anything else means a consumer has to guess, which is the whole thing
/// the format exists to prevent.
fn is_hex32(v: Option<&Value>) -> bool {
v.and_then(Value::as_str)
.is_some_and(|s| s.len() == 10 && s.starts_with("0x") && s[2..].chars().all(|c| c.is_ascii_hexdigit()))
}
fn check_pose(c: &mut Ctx, where_: &str, p: &Value) {
for (key, want_len) in [("pos", 2usize), ("scale", 2)] {
match p.get(key).and_then(Value::as_array) {
Some(a) if a.len() == want_len && a.iter().all(Value::is_i64) => {}
_ => c.err(format!("{where_}: `{key}` must be {want_len} integers")),
}
}
for key in ["tint_rgba", "fade_argb"] {
if !is_hex32(p.get(key)) {
c.err(format!("{where_}: `{key}` must be 0x + 8 hex digits"));
}
}
}
fn check_screen(root: &Path, rel: &str, errors: &mut Vec<String>) -> Result<()> {
let raw = std::fs::read_to_string(root.join(rel))?;
let v: Value = serde_json::from_str(&raw)?;
let mut c = Ctx {
file: rel.to_string(),
errors: Vec::new(),
};
if v.get("format").and_then(Value::as_str) != Some(SCREEN_FORMAT) {
c.err(format!(
"format is {:?}, expected {SCREEN_FORMAT:?}",
v.get("format")
));
}
for key in ["exporter", "formats_rev", "name", "name_source"] {
c.require(&v, key);
}
// Rule 2 of the format: a modder must be able to tell a recovered name from
// an invented one, so the provenance is mandatory and closed.
match v.get("name_source").and_then(Value::as_str) {
Some("authored") => {
if v.get("name_why").and_then(Value::as_str).is_none_or(str::is_empty) {
c.err("name_source is `authored` but there is no `name_why`");
}
}
Some("index") => {}
other => c.err(format!("name_source must be `authored` or `index`, got {other:?}")),
}
if let Some(s) = v.get("source") {
for key in ["archive", "entry", "build"] {
if s.get(key).is_none() {
c.err(format!("source is missing `{key}`"));
}
}
} else {
c.err("missing required field `source`");
}
match v.get("design").and_then(Value::as_array) {
Some(d) if d.len() == 2 && d.iter().all(Value::is_u64) => {}
_ => c.err("`design` must be two positive integers"),
}
let Some(elements) = v.get("elements").and_then(Value::as_array) else {
c.err("missing required field `elements`");
errors.append(&mut c.errors);
return Ok(());
};
let mut indices = Vec::new();
let mut buttons_by_y: Vec<(i64, String)> = Vec::new();
for (i, el) in elements.iter().enumerate() {
let id = el.get("id").and_then(Value::as_str).unwrap_or("<no id>").to_string();
let at = format!("element {i} ({id})");
for key in ["index", "id", "declared", "role", "kind_raw", "pivot", "layer_source", "keyframes"] {
if el.get(key).is_none() {
c.err(format!("{at}: missing `{key}`"));
}
}
let Some(idx) = el.get("index").and_then(Value::as_u64) else {
c.err(format!("{at}: `index` is not an integer"));
continue;
};
if idx as usize != i {
c.err(format!("{at}: `index` {idx} does not match its position {i}"));
}
indices.push(idx as usize);
let role = el.get("role").and_then(Value::as_str).unwrap_or("");
if !matches!(role, "button" | "decoration" | "primitive" | "unknown") {
c.err(format!("{at}: role {role:?} is not one FORMAT.md defines"));
}
// A role of `unknown` must still carry the raw kind, or the information
// is simply lost.
if role == "unknown" && el.get("kind_raw").is_none() {
c.err(format!("{at}: role `unknown` without `kind_raw`"));
}
// A primitive has no texture, so its quad size has to come from the file.
if role == "primitive" && el.get("size").is_none() {
c.err(format!("{at}: primitive without a `size`"));
}
match el.get("layer_source").and_then(Value::as_str) {
Some("sprite") | Some("implied") => {
if !is_hex32(el.get("layer")) {
c.err(format!("{at}: layer_source claims a key but `layer` is not one"));
}
}
Some("none") => {
if el.get("layer").is_some() {
c.err(format!("{at}: layer_source `none` but a `layer` is present"));
}
}
other => c.err(format!("{at}: layer_source must be sprite/implied/none, got {other:?}")),
}
for key in ["sprite", "focus_sprite"] {
if let Some(p) = el.get(key).and_then(Value::as_str) {
let path = root.join(p);
if !path.exists() {
c.err(format!("{at}: `{key}` points at {p}, which does not exist"));
} else if let Err(e) = image::open(&path) {
c.err(format!("{at}: `{key}` {p} does not decode as an image: {e}"));
}
}
}
if let Some(r) = el.get("rest") {
check_pose(&mut c, &at, r);
if role == "button" {
if let Some(y) = r.get("pos").and_then(Value::as_array).and_then(|a| a[1].as_i64()) {
buttons_by_y.push((y, id.clone()));
}
}
}
if let Some(kfs) = el.get("keyframes").and_then(Value::as_array) {
for (k, kf) in kfs.iter().enumerate() {
check_pose(&mut c, &format!("{at} keyframe {k}"), kf);
}
// 🔴 INVERTED 2026-08-29, and the old rule is the more interesting
// half. It read: "the last keyframe of a group carries no time slot
// on the disc, and an invented one is exactly the kind of value this
// format refuses." That was true of the OLD keyframe association,
// where a group's data stopped four bytes short of its final block's
// time slot.
//
// Under the corrected layout (`formats-pin-2026-08-29c` onward) a
// group is an 8-byte header then `frames` x {u32 time; 36-byte
// pose}, so **pose 0's time is the group's lead-in word and EVERY
// POSE IS TIMED, including the last.** The rule now says the
// opposite, and an untimed keyframe is the thing to refuse.
//
// ⚠️ This fired 150 times on a re-export and I had not run `check`
// between pinning the tag and measuring against the oracle -- the
// pixel harness was green while the format validator was failing on
// every screen with a multi-keyframe group. A correctness harness
// does not replace a format one; they fail at different layers.
if kfs.len() > 1 && kfs.iter().any(|k| k.get("t").is_none()) {
c.err(format!("{at}: a keyframe has no `t`; every pose is timed under the corrected record layout"));
}
}
}
// Paint order must be a permutation of the element indices, or the runtime
// either drops an element or draws one twice.
match v.get("paint_order").and_then(Value::as_array) {
Some(po) => {
let mut got: Vec<usize> = po.iter().filter_map(|x| x.as_u64().map(|v| v as usize)).collect();
if got.len() != po.len() {
c.err("`paint_order` holds a non-integer");
}
let mut want = indices.clone();
got.sort_unstable();
want.sort_unstable();
if got != want {
c.err("`paint_order` is not a permutation of the element indices");
}
}
None => c.err("missing required field `paint_order`"),
}
// `buttons` is navigation order and is defined as resting Y, ascending. If
// it is not sorted, it is not the thing FORMAT.md says it is.
match v.get("buttons").and_then(Value::as_array) {
Some(b) => {
let listed: Vec<&str> = b.iter().filter_map(Value::as_str).collect();
buttons_by_y.sort_by(|a, b| a.0.cmp(&b.0).then_with(|| a.1.cmp(&b.1)));
let want: Vec<&str> = buttons_by_y.iter().map(|(_, n)| n.as_str()).collect();
if listed != want {
c.err(format!(
"`buttons` is {listed:?} but resting-Y order is {want:?}"
));
}
}
None => c.err("missing required field `buttons`"),
}
if v.get("unresolved").and_then(Value::as_array).is_none() {
c.err("missing required field `unresolved` (an empty list is a claim; absence is a gap)");
}
errors.append(&mut c.errors);
Ok(())
}
/// Validate a whole export tree. Returns the number of screens checked.
pub fn run(root: &Path) -> Result<usize> {
let manifest_path = root.join("manifest.json");
if !manifest_path.exists() {
bail!("{} has no manifest.json — is that an export tree?", root.display());
}
let m: Value = serde_json::from_str(&std::fs::read_to_string(&manifest_path)?)?;
let mut errors = Vec::new();
if m.get("format").and_then(Value::as_str) != Some(MANIFEST_FORMAT) {
errors.push(format!("manifest.json: format is not {MANIFEST_FORMAT:?}"));
}
for key in ["exporter", "formats_rev", "screens", "warnings"] {
if m.get(key).is_none() {
errors.push(format!("manifest.json: missing `{key}`"));
}
}
let screens = m
.get("screens")
.and_then(Value::as_array)
.map(|s| s.to_vec())
.unwrap_or_default();
for s in &screens {
let Some(file) = s.get("file").and_then(Value::as_str) else {
errors.push("manifest.json: a screen entry has no `file`".into());
continue;
};
if !root.join(file).exists() {
errors.push(format!("manifest.json: lists {file}, which does not exist"));
continue;
}
check_screen(root, file, &mut errors)?;
}
check_audio(root, &m, &mut errors);
if !errors.is_empty() {
for e in &errors {
eprintln!("{e}");
}
bail!("{} problem(s) in {}", errors.len(), root.display());
}
Ok(screens.len())
}
/// The `audio` array, checked the way a consumer would have to.
///
/// Two of these are content checks rather than schema checks, and they are here
/// on purpose. `docs/port/AUDIO-VERIFICATION.md` names silence as "the failure
/// that looks like success": a file of exactly the right duration, the right
/// channel count and the right size, full of zeroes, because something opened
/// the wrong thing. Every structural check passes it. So does clipping, which
/// the BGM can produce because it is a **sum of two stems** at unity gain.
///
/// The exporter measures both at export time and writes them here; this refuses
/// the tree if what it wrote is a file nobody would want to play. Neither is a
/// judgement about whether the audio is the RIGHT audio — nothing in this
/// binary can know that, and `docs/port/BLOCKED.md` says which parts are still
/// authored guesses.
fn check_audio(root: &Path, m: &Value, errors: &mut Vec<String>) {
let Some(audio) = m.get("audio").and_then(Value::as_array) else {
// Absent is correct for every export taken before P6.
return;
};
for a in audio {
let name = a.get("name").and_then(Value::as_str).unwrap_or("?");
let kind = a.get("kind").and_then(Value::as_str).unwrap_or("");
if !matches!(kind, "se" | "bgm" | "voice") {
errors.push(format!(
"manifest.json: audio `{name}` has kind {kind:?}, which a consumer cannot dispatch on"
));
}
for key in ["file", "command", "why"] {
if a.get(key).and_then(Value::as_str).is_none_or(str::is_empty) {
errors.push(format!("manifest.json: audio `{name}` has no `{key}`"));
}
}
let Some(file) = a.get("file").and_then(Value::as_str) else { continue };
if !root.join(file).exists() {
errors.push(format!("manifest.json: lists audio {file}, which does not exist"));
continue;
}
match a.get("peak_dbfs").and_then(Value::as_f64) {
None => errors.push(format!(
"manifest.json: audio `{name}` carries no `peak_dbfs` -- it was not measured, \
and silence is the audio failure that passes every check that is not looking \
for it"
)),
Some(p) if p <= -90.0 => errors.push(format!(
"{file}: peak is {p:.1} dBFS -- this file is silent"
)),
// The bound differs by kind, and the difference is the point. A
// `bgm` is something WE combined -- a sum of stems -- so a peak at
// or above full scale is our arithmetic and is refused outright. An
// `se` is a single wave off the disc: it is mastered near full
// scale, and a lossy decode of a near-full-scale signal overshoots
// by a fraction of a dB (`confirm` lands at +0.18). Refusing that
// would be refusing the disc's own mastering, and "fixing" it would
// mean attenuating a game asset to make a number smaller.
//
// 🟡 +1.0 dB is a JUDGEMENT, not a measurement: a few tenths is
// reconstruction overshoot, a whole dB is not. Nobody has measured
// the overshoot distribution across a corpus of cues, and if a cue
// ever trips this the right response is that measurement, not a
// looser bound.
// `voice` was on the strict side of this bound while it was a SUM of a
// region's chunks. It no longer is: a region carries three
// presentations of one take, so the exporter keeps ONE stream and
// performs no arithmetic on it. That puts `voice` with `se` -- a
// single wave off the disc, mastered near full scale, whose lossy
// decode overshoots by a fraction of a dB. `ADV`'s louder
// presentation measures +0.0003 dBFS at source; refusing that would
// be refusing the disc's own mastering.
Some(p) if kind == "bgm" && p >= 0.0 => errors.push(format!(
"{file}: peak is {p:.1} dBFS -- a SUM we produced clips"
)),
Some(p) if kind != "bgm" && p > 1.0 => errors.push(format!(
"{file}: peak is {p:.1} dBFS -- too far over full scale to be decode overshoot"
)),
Some(_) => {}
}
match a.get("duration_s").and_then(Value::as_f64) {
Some(d) if d > 0.0 => {}
_ => errors.push(format!(
"{file}: no positive `duration_s` -- a zero-length asset plays as silence"
)),
}
}
}