fix(agent): one image per slot, and a photo band that shows the photo

Five destination cards rendered as five identical blue bands. The photos
were fine and all different - the cards were the problem. Each card's
image slot carried TWO images (an absolutely-positioned image-filled
plate AND the real image node) inside a band authored 56px tall against
a 130px photo, so all that survived the clip was each photo's sky.

The duplicate plate is dropped (the image node wins - it is what the
image pipeline fills and re-searches), the band grows to the photo's own
declared height when it would hide more than half of it, and the starved
rail-card repair now reads fixed-width DESCENDANTS and assigns the
demand as a definite width: an out-of-flow photo plate contributes
nothing to a hug, so fit_content re-starved the very cards it was meant
to rescue (126px against a 200px plate). Replaying the measured document
through the repair loop: 4 geometry diagnostics to 0, cards 79px to
200px.
This commit is contained in:
Fini 2026-07-13 21:41:31 +08:00
parent 364859bf49
commit 929ea1dbf6
2 changed files with 260 additions and 16 deletions

View file

@ -188,6 +188,7 @@ pub fn geometry_validate_and_fix(sink: &mut dyn DocSink, root_id: &str) -> usize
collect_frame_overflow_fixes(&v, &rects, &mut cmds);
collect_oversized_image_fixes(&v, &mut cmds);
collect_absolute_fill_image_fixes(&v, &rects, &mut cmds);
collect_image_slot_fixes(&v, &mut cmds);
collect_grow_to_fit_fixes(&v, &rects, &mut cmds);
collect_starved_rail_card_fixes(&v, &rects, &mut cmds);
collect_row_gap_fixes(&v, &rects, &mut cmds);
@ -1126,13 +1127,27 @@ fn collect_grow_to_fit_fixes(
const RAIL_STARVE_EPS: f64 = 12.0;
const RAIL_MIN_CARDS: usize = 3;
/// The widest fixed-width DIRECT child plus the card's own side padding — the
/// width this card provably needs to show its authored content.
/// The widest fixed-width DESCENDANT plus the card's own side padding — the
/// width this card provably needs to show its authored content. Descendants,
/// not just direct children: a destination card's photo is often an
/// absolutely-positioned 200px plate two levels down, and that number is
/// still the card's authored width intent (measured test0711-1-glm, where a
/// direct-children-only scan saw nothing and left the cards at 79px).
fn fixed_content_demand(card: &Value) -> f64 {
let widest = children(card)
.iter()
.filter_map(fixed_width)
.fold(0.0_f64, f64::max);
fn widest_fixed(v: &Value, depth: usize) -> f64 {
if depth == 0 {
return 0.0;
}
children(v)
.iter()
.map(|c| {
fixed_width(c)
.unwrap_or(0.0)
.max(widest_fixed(c, depth - 1))
})
.fold(0.0_f64, f64::max)
}
let widest = widest_fixed(card, 3);
if widest == 0.0 {
0.0
} else {
@ -1166,10 +1181,20 @@ fn collect_starved_rail_card_fixes(
if all_starved {
for c in &cards {
if let Some(id) = c.get("id").and_then(Value::as_str) {
cmds.push(EditorCommand::SetNodeLayoutProp {
// The card takes its DEMAND as a definite width, not
// fit_content: an absolutely-positioned photo plate is
// out of flow, so hugging would size the card to its
// text alone and starve the photo all over again
// (measured: hug gave 126px against a 200px plate).
cmds.push(EditorCommand::UpdateNode {
node_id: NodeId::new(id.to_string()),
property: "width".to_string(),
value: LayoutPropValue::Keyword("fit_content".to_string()),
x: None,
y: None,
width: Some(fixed_content_demand(c).round() as i32),
height: None,
name: None,
fill_hex: None,
page_id: None,
});
}
}
@ -1245,6 +1270,109 @@ fn collect_absolute_fill_image_fixes(
}
}
/// TWO images in one slot, and a photo band cropped to a sliver.
///
/// Measured (test0711-1-glm): a destination card's image wrapper was authored
/// `height: 56` and held BOTH an absolutely-positioned 200x130 frame whose
/// FILL is the photo AND a sibling image NODE carrying the same slot's photo -
/// two images stacked in one box, of which only the top 56px survived the
/// wrapper's clip. Every card's surviving 56px was that photo's sky, so five
/// different photos rendered as five identical blue bands (user report: "最近
/// 怎么经常有这种情况").
///
/// Two repairs, both provable from the authored tree:
/// 1. **One image per slot** - when a wrapper holds an image-filled frame AND
/// an image node, the image NODE wins (it is what the image pipeline fills
/// and re-searches); the duplicate fill-frame is deleted.
/// 2. **The band keeps the photo's height** - the photo the model authored for
/// the slot declares a definite height; a wrapper shorter than half of it is
/// an authoring slip, not art direction, so the wrapper grows to the photo's
/// height. (A wrapper that already fits, or crops only mildly, is left
/// alone - intentional letterboxing stays intentional.)
const IMAGE_BAND_MIN_VISIBLE_FRACTION: f64 = 0.5;
fn is_image_filled_frame(v: &Value) -> bool {
matches!(
v.get("type").and_then(Value::as_str),
Some("frame" | "group" | "rectangle")
) && v
.get("fill")
.and_then(Value::as_array)
.is_some_and(|fills| {
fills.iter().any(|f| {
f.get("type").and_then(Value::as_str) == Some("image")
|| f.get("url").is_some()
|| f.get("src").is_some()
})
})
}
fn fixed_height(v: &Value) -> Option<f64> {
match v.get("height") {
Some(Value::Number(n)) => n.as_f64(),
Some(Value::String(s)) => s.parse::<f64>().ok(),
_ => None,
}
}
fn collect_image_slot_fixes(v: &Value, cmds: &mut Vec<EditorCommand>) {
let kids = children(v);
let image_nodes: Vec<&Value> = kids
.iter()
.filter(|c| c.get("type").and_then(Value::as_str) == Some("image"))
.collect();
let filled_frames: Vec<&Value> = kids.iter().filter(|c| is_image_filled_frame(c)).collect();
// (1) One image per slot: the node wins, the duplicate fill-frame goes.
// Only a CHILDLESS plate qualifies — a filled frame that carries content
// (a hero with a headline on it) is a real container, not a stray twin.
if !image_nodes.is_empty() {
for frame in filled_frames.iter().filter(|f| children(f).is_empty()) {
if let Some(id) = frame.get("id").and_then(Value::as_str) {
cmds.push(EditorCommand::DeleteNode {
node_id: NodeId::new(id.to_string()),
page_id: None,
});
}
}
}
// (2) The band keeps the photo's height. The photo's own declared height
// is the model's intent for the slot; take it from whichever carrier is
// surviving repair (1).
if let Some(band) = fixed_height(v) {
let photo_h = if image_nodes.is_empty() {
filled_frames
.iter()
.filter_map(|f| fixed_height(f))
.fold(0.0, f64::max)
} else {
image_nodes
.iter()
.chain(filled_frames.iter())
.filter_map(|f| fixed_height(f))
.fold(0.0, f64::max)
};
if photo_h > 0.0 && band < photo_h * IMAGE_BAND_MIN_VISIBLE_FRACTION {
if let Some(id) = v.get("id").and_then(Value::as_str) {
cmds.push(EditorCommand::UpdateNode {
node_id: NodeId::new(id.to_string()),
x: None,
y: None,
width: None,
height: Some(photo_h.round() as i32),
name: None,
fill_hex: None,
page_id: None,
});
}
}
}
for c in kids {
collect_image_slot_fixes(c, cmds);
}
}
/// A frame declaring a NUMERIC height but resolving MUCH taller — an
/// oversized child inflated it (jian grows the parent instead of letting the
/// child spill, so no edge ever crosses another and the width-overflow echo

View file

@ -2118,21 +2118,29 @@ fn starved_rail_rects() -> std::collections::HashMap<String, Rect> {
}
#[test]
fn starved_rail_cards_hug_and_rail_becomes_scroller() {
fn starved_rail_cards_take_their_demand_and_the_rail_becomes_a_scroller() {
let mut cmds = Vec::new();
collect_starved_rail_card_fixes(&starved_rail(), &starved_rail_rects(), &mut cmds);
let hugged: Vec<&str> = cmds
let sized: Vec<(&str, i32)> = cmds
.iter()
.filter_map(|c| match c {
EditorCommand::SetNodeLayoutProp {
EditorCommand::UpdateNode {
node_id,
property,
value: LayoutPropValue::Keyword(k),
} if property == "width" && k == "fit_content" => Some(node_id.as_str()),
width: Some(w),
..
} => Some((node_id.as_str(), *w)),
_ => None,
})
.collect();
assert_eq!(hugged.len(), 5, "all five cards hug: {cmds:?}");
assert_eq!(
sized.len(),
5,
"all five cards take a definite width: {cmds:?}"
);
assert!(
sized.iter().all(|(_, w)| *w == 188),
"each card takes its widest fixed content as its width: {sized:?}"
);
let rail_clipped = cmds.iter().any(|c| {
matches!(c,
EditorCommand::SetNodeLayoutProp { node_id, property, value: LayoutPropValue::Bool(true) }
@ -2209,3 +2217,111 @@ fn rail_with_one_flexible_card_is_not_forced_to_hug() {
collect_starved_rail_card_fixes(&rail, &starved_rail_rects(), &mut cmds);
assert!(cmds.is_empty(), "mixed rail left to the echo: {cmds:?}");
}
// ── one image per slot + un-cropped photo band ──
/// The measured test0711-1-glm card: a 56px image band holding BOTH a
/// 200x130 image-filled plate and the real image node.
fn double_image_slot() -> serde_json::Value {
json!({
"type": "frame", "id": "band", "name": "Dest Image Bali",
"width": "fill_container", "height": 56, "layout": "horizontal",
"clipContent": true,
"children": [
{ "type": "frame", "id": "plate", "name": "Dest Image Fill Bali",
"width": 200, "height": 130, "x": 0, "y": 0,
"fill": [{ "type": "image", "url": "data:image/jpeg;base64,AAAA" }] },
{ "type": "icon_font", "id": "heart", "width": 22, "height": 22, "x": 165, "y": 10 },
{ "type": "image", "id": "photo", "name": "Bali temple rice terraces",
"width": "fill_container", "height": "fill_container",
"src": "data:image/jpeg;base64,BBBB" }
]
})
}
#[test]
fn duplicate_image_plate_is_dropped_and_the_band_keeps_the_photo_height() {
let mut cmds = Vec::new();
collect_image_slot_fixes(&double_image_slot(), &mut cmds);
assert!(
cmds.iter().any(|c| matches!(c,
EditorCommand::DeleteNode { node_id, .. } if node_id.as_str() == "plate")),
"the childless image plate is the duplicate — the image node wins: {cmds:?}"
);
assert!(
!cmds.iter().any(|c| matches!(c,
EditorCommand::DeleteNode { node_id, .. } if node_id.as_str() == "photo")),
"the real image node survives"
);
let grown = cmds.iter().find_map(|c| match c {
EditorCommand::UpdateNode {
node_id, height, ..
} if node_id.as_str() == "band" => *height,
_ => None,
});
assert_eq!(
grown,
Some(130),
"a 56px band cropped a 130px photo to its sky — the band takes the photo's height"
);
}
#[test]
fn a_lone_image_filled_plate_is_never_deleted() {
// No image node in the slot — the plate IS the image. Untouched.
let slot = json!({
"type": "frame", "id": "band", "width": "fill_container", "height": 130,
"children": [
{ "type": "frame", "id": "plate", "width": 200, "height": 130,
"fill": [{ "type": "image", "url": "data:image/jpeg;base64,AAAA" }] }
]
});
let mut cmds = Vec::new();
collect_image_slot_fixes(&slot, &mut cmds);
assert!(
cmds.is_empty(),
"the only image in the slot stays: {cmds:?}"
);
}
#[test]
fn an_image_filled_hero_with_content_on_it_is_not_a_duplicate_plate() {
// A filled frame that CARRIES content is a real container (hero with a
// headline), not a stray twin — even beside an image node.
let slot = json!({
"type": "frame", "id": "band", "width": "fill_container", "height": 200,
"children": [
{ "type": "frame", "id": "hero", "width": 200, "height": 200,
"fill": [{ "type": "image", "url": "data:image/jpeg;base64,AAAA" }],
"children": [{ "type": "text", "id": "headline", "content": "Explore" }] },
{ "type": "image", "id": "photo", "width": 80, "height": 80,
"src": "data:image/jpeg;base64,BBBB" }
]
});
let mut cmds = Vec::new();
collect_image_slot_fixes(&slot, &mut cmds);
assert!(
!cmds
.iter()
.any(|c| matches!(c, EditorCommand::DeleteNode { .. })),
"a hero container is not deleted: {cmds:?}"
);
}
#[test]
fn a_mild_crop_stays_intentional() {
// 130px photo in a 100px band — letterboxing the model may well have
// meant. Only a band that hides MORE than half the photo is repaired.
let slot = json!({
"type": "frame", "id": "band", "width": "fill_container", "height": 100,
"clipContent": true,
"children": [
{ "type": "image", "id": "photo", "width": 200, "height": 130,
"src": "data:image/jpeg;base64,BBBB" }
]
});
let mut cmds = Vec::new();
collect_image_slot_fixes(&slot, &mut cmds);
assert!(cmds.is_empty(), "a mild crop is left alone: {cmds:?}");
}