address review: holes go to the innermost outline; glTF wrap variants keyed by decoded image

crates/model/src/extrude.rs

@@ -132,20 +132,27 @@ pub fn group_contours(contours: Vec<Vec<P>>) -> Vec<Outline> {
         .collect();
     // The containers of each contour: the larger contours holding a vertex of it (nested contours
     // do not cross); the bounding box must hold the vertex first. Depth is their count, so
-    // overlapping (non-nested) paths keep every container they sit inside; the innermost
-    // container (smallest area) takes a hole.
-    let mut parent: Vec<Option<usize>> = vec![None; n];
-    let mut depth = vec![0usize; n];
-    for i in 0..n {
+    // overlapping (non-nested) paths keep every container they sit inside.
+    let containers = |i: usize, only: &dyn Fn(usize) -> bool| -> Vec<usize> {
         let p = cs[i][0];
-        let containers: Vec<usize> = (0..n)
-            .filter(|&j| j != i && area[j] > area[i])
+        (0..n)
+            .filter(|&j| j != i && area[j] > area[i] && only(j))
             .filter(|&j| (bounds[j][0][0]..=bounds[j][1][0]).contains(&p[0]) && (bounds[j][0][1]..=bounds[j][1][1]).contains(&p[1]))
             .filter(|&j| point_in(p, &cs[j]))
-            .collect();
-        depth[i] = containers.len();
-        parent[i] = containers.into_iter().min_by(|&a, &b| area[a].total_cmp(&area[b]));
-    }
+            .collect()
+    };
+    let depth: Vec<usize> = (0..n).map(|i| containers(i, &|_| true).len()).collect();
+    // A hole belongs to the innermost containing contour that is itself an outline: with
+    // overlapping paths the smallest container can be a hole, which has no outline to take it.
+    let parent: Vec<Option<usize>> = (0..n)
+        .map(|i| {
+            if depth[i].is_multiple_of(2) {
+                None
+            } else {
+                containers(i, &|j| depth[j].is_multiple_of(2)).into_iter().min_by(|&a, &b| area[a].total_cmp(&area[b]))
+            }
+        })
+        .collect();
     let mut outlines: Vec<Outline> = vec![];
     let mut slot = vec![usize::MAX; n];
     let mut order: Vec<usize> = (0..n).collect();

crates/model/src/gltf.rs

@@ -388,14 +388,16 @@ pub(crate) fn parse_with_budget(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<V
     };
     let mut textures: Vec<Arc<Texture>> = vec![];
     let mut tex_map: Vec<Option<usize>> = vec![];
-    // One texture per image and wrapping: repeated references share it instead of copying
-    // the pixels.
+    // One texture per decoded image and wrapping: repeated references share it instead of copying
+    // the pixels. The image is keyed by its allocation, so image entries naming the same file
+    // (which share one decode) also share their wrap variants.
     let mut texture_of: HashMap<(usize, [Wrap; 2]), usize> = HashMap::new();
     for t in arr(&j, "textures") {
-        let Some((src, img)) = idx(t, "source").and_then(|s| Some((s, images.get(s).cloned().flatten()?))) else {
+        let Some(img) = idx(t, "source").and_then(|s| images.get(s).cloned().flatten()) else {
             tex_map.push(None);
             continue;
         };
+        let src = Arc::as_ptr(&img) as usize;
         let samp = idx(t, "sampler").and_then(|s| arr(&j, "samplers").get(s));
         let ws = samp.and_then(|s| s.get("wrapS")).and_then(Value::as_u64).unwrap_or(10497);
         let wt = samp.and_then(|s| s.get("wrapT")).and_then(Value::as_u64).unwrap_or(10497);

crates/model/src/tests.rs

@@ -442,6 +442,31 @@ fn overlapping_contours_keep_every_container_in_their_depth() {
     assert_eq!(g.iter().map(|o| o.holes.len()).sum::<usize>(), 0);
 }
 
+#[test]
+fn a_contour_inside_a_hole_takes_the_innermost_outline_not_the_hole() {
+    let rect = |x0: f64, y0: f64, x1: f64, y1: f64| vec![[x0, y0], [x1, y0], [x1, y1], [x0, y1]];
+    // A and B overlap; C lies in A only (a hole of A); D lies in A, B and C (three deep, a hole).
+    // The smallest container of D is the hole C, which has no outline of its own: D goes to B.
+    let g = group_contours(vec![rect(0.0, 0.0, 10.0, 10.0), rect(5.0, -2.0, 13.0, 8.0), rect(1.0, 1.0, 9.0, 9.0), rect(6.0, 2.0, 7.0, 3.0)]);
+    assert_eq!(g.len(), 2);
+    assert_eq!(g.iter().map(|o| o.holes.len()).collect::<Vec<_>>(), [1, 1], "C is a hole of A, D a hole of B (not dropped)");
+}
+
+#[test]
+fn gltf_images_naming_one_file_share_their_wrap_variants() {
+    let png = {
+        let img = image::RgbaImage::from_fn(2, 2, |x, y| if (x + y) % 2 == 0 { image::Rgba([255, 0, 0, 255]) } else { image::Rgba([0, 0, 255, 255]) });
+        let mut out = std::io::Cursor::new(Vec::new());
+        img.write_to(&mut out, image::ImageFormat::Png).unwrap();
+        out.into_inner()
+    };
+    // Two image entries with one URI share a decode; two textures clamp it, so they share one copy.
+    let doc = br#"{"asset":{"version":"2.0"},"images":[{"uri":"t.png"},{"uri":"t.png"}],
+        "samplers":[{"wrapS":33071,"wrapT":33071}],"textures":[{"source":0,"sampler":0},{"source":1,"sampler":0}]}"#;
+    let m = gltf::parse(doc, &|u| (u == "t.png").then(|| png.clone())).unwrap();
+    assert_eq!(m.textures.len(), 1, "one clamped copy for both textures");
+}
+
 #[test]
 fn gltf_cameras_and_punctual_lights() {
     let doc = r#"{