From 08e9e382dba902e9cbce0d85c7dc8174922daae2 Mon Sep 17 00:00:00 2001 From: Orual Date: Sun, 2 Aug 2026 17:53:23 -0400 Subject: [PATCH] PM-78: correct repeated-instance quad expectations Epic: PM-86 Task: PM-78 --- crates/polymodel-ldraw-core/src/lib.rs | 17 ++- crates/polymodel-ldraw-core/src/traversal.rs | 151 +++++++++---------- 2 files changed, 87 insertions(+), 81 deletions(-) diff --git a/crates/polymodel-ldraw-core/src/lib.rs b/crates/polymodel-ldraw-core/src/lib.rs index 0bad697..cb0669b 100644 --- a/crates/polymodel-ldraw-core/src/lib.rs +++ b/crates/polymodel-ldraw-core/src/lib.rs @@ -120,6 +120,17 @@ mod tests { assert_eq!(second.scene.model_id, first.scene.model_id); assert_eq!(second.scene.instance_ids, first.scene.instance_ids); } + #[test] + fn repeated_instances_expand_subtrees() { + let bytes = b"0 FILE root.ldr\n1 16 0 0 0 1 0 0 0 1 0 0 0 0 child.dat\n1 16 0 0 0 1 0 0 0 1 0 0 0 0 child.dat\n0 NOFILE\n0 FILE child.dat\n1 16 0 0 0 1 0 0 0 1 0 0 0 0 grand.dat\n0 NOFILE\n0 FILE grand.dat\n3 16 0 0 0 1 0 0 0 1 0\n0 NOFILE\n"; + let ledger = ReservationLedger::new(); + let result = LdrawParser + .parse_bytes(bytes, options(&ledger)) + .expect("repeated child instances should parse"); + assert_eq!(result.scene.triangles, 2); + assert_eq!(result.scene.instance_ids.len(), 4); + } + #[test] fn limits_reject_before_publication() { let ledger = ReservationLedger::new(); @@ -353,7 +364,7 @@ mod tests { .collect::>(), vec!["main.ldr", "house.ldr", "sticker.ldr"] ); - assert_scene_counts(&result, 0, 1, 0, 0); + assert_scene_counts(&result, 0, 5, 0, 0); assert_sticker_geometry(&result); } @@ -561,7 +572,7 @@ mod tests { // Oracle: tri=N quad=M → rust triangles=N, quads=M ("graph-instances", "ViperMain.ldr", 9, 250, 0, 0, 0), ("external-thomas", "ViperMain.ldr", 9, 250, 0, 0, 0), - ("external-weldr", "main.ldr", 3, 5, 0, 0, 1), + ("external-weldr", "main.ldr", 3, 5, 0, 0, 5), ( "external-ldr-tools", "hatching-grounds.ldr", @@ -571,7 +582,7 @@ mod tests { 0, 0, ), - ("mpd-data", "main.ldr", 3, 7, 0, 0, 1), + ("mpd-data", "main.ldr", 3, 7, 0, 0, 5), ]; for ( diff --git a/crates/polymodel-ldraw-core/src/traversal.rs b/crates/polymodel-ldraw-core/src/traversal.rs index d8e44a0..c5a5c21 100644 --- a/crates/polymodel-ldraw-core/src/traversal.rs +++ b/crates/polymodel-ldraw-core/src/traversal.rs @@ -84,7 +84,6 @@ pub(crate) fn traverse( if !visiting.insert(key.clone()) { continue; } - let first_parse = !completed.contains(&key); let model_id = ids .entry(key.clone()) .or_insert_with(|| model.path.to_string()) @@ -138,84 +137,80 @@ pub(crate) fn traverse( reflection ^= reflected; stack.push_back(Visit::Exit { key: key.clone() }); - if first_parse { - for (include_position, include) in model.includes.iter().enumerate().rev() { - if options.cancellation.cancelled { - return Err(ParseError::Cancelled); - } - counters - .add(LimitKind::Instances, 1, &options.limits) - .map_err(|name| limit_error(name, Some(include.span)))?; - let target = models.iter().position(|candidate| { - candidate - .path - .as_str() - .eq_ignore_ascii_case(&include.name.replace('\\', "/")) - }); - let child_name = target - .map(|index| models[index].path.to_string()) - .unwrap_or_else(|| include.name.replace('\\', "/")); - instance_ids.push(format!( - "{}→{}#{}", - model.path, - child_name, - include_position + 1 - )); - let child_depth = depth - .checked_add(1) - .ok_or(ParseError::Overflow("include depth"))?; - if child_depth > options.limits.include_depth { - add_diag( - options.profile, - diagnostics, - counters, - &options.limits, - DiagnosticCode::IncludeDepthLimit, - include.span, - "include depth limit exceeded", - false, - )?; - continue; - } - let Some(target) = target else { - add_diag( - options.profile, - diagnostics, - counters, - &options.limits, - DiagnosticCode::UnresolvedReference, - include.span, - "include target not found in model set", - false, - )?; - continue; - }; - let child = &models[target]; - if visiting.contains(&child.key) { - add_diag( - options.profile, - diagnostics, - counters, - &options.limits, - DiagnosticCode::GraphCycle, - include.span, - "include cycle cut at active graph key", - false, - )?; - continue; - } - if !ids.contains_key(&child.key) { - ids.insert(child.key.clone(), child.path.to_string()); - } - stack.push_back(Visit::Enter { - index: target, - transform: transform.compose(&include.transform), - depth: child_depth, - reflected: reflected - ^ include.inverted - ^ include.transform.reflection(), - }); + for (include_position, include) in model.includes.iter().enumerate().rev() { + if options.cancellation.cancelled { + return Err(ParseError::Cancelled); } + counters + .add(LimitKind::Instances, 1, &options.limits) + .map_err(|name| limit_error(name, Some(include.span)))?; + let target = models.iter().position(|candidate| { + candidate + .path + .as_str() + .eq_ignore_ascii_case(&include.name.replace('\\', "/")) + }); + let child_name = target + .map(|index| models[index].path.to_string()) + .unwrap_or_else(|| include.name.replace('\\', "/")); + instance_ids.push(format!( + "{}→{}#{}", + model.path, + child_name, + include_position + 1 + )); + let child_depth = depth + .checked_add(1) + .ok_or(ParseError::Overflow("include depth"))?; + if child_depth > options.limits.include_depth { + add_diag( + options.profile, + diagnostics, + counters, + &options.limits, + DiagnosticCode::IncludeDepthLimit, + include.span, + "include depth limit exceeded", + false, + )?; + continue; + } + let Some(target) = target else { + add_diag( + options.profile, + diagnostics, + counters, + &options.limits, + DiagnosticCode::UnresolvedReference, + include.span, + "include target not found in model set", + false, + )?; + continue; + }; + let child = &models[target]; + if visiting.contains(&child.key) { + add_diag( + options.profile, + diagnostics, + counters, + &options.limits, + DiagnosticCode::GraphCycle, + include.span, + "include cycle cut at active graph key", + false, + )?; + continue; + } + if !ids.contains_key(&child.key) { + ids.insert(child.key.clone(), child.path.to_string()); + } + stack.push_back(Visit::Enter { + index: target, + transform: transform.compose(&include.transform), + depth: child_depth, + reflected: reflected ^ include.inverted ^ include.transform.reflection(), + }); } } } -- 2.51.2