From f99317b89b854c292b95dbab40c222968c0a1d54 Mon Sep 17 00:00:00 2001
From: "Dylan M. Taylor" <dylan@dylanmtaylor.com>
Date: Tue, 3 Nov 2026 12:58:58 +0000
Subject: [PATCH] address review: keep largest contours and dedupe mtllib by
 set

---
 crates/model/src/extrude.rs | 16 +++++++++++-----
 crates/model/src/obj.rs     |  7 +++----
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/crates/model/src/extrude.rs b/crates/model/src/extrude.rs
index 9018eb186..4cf575f28 100644
--- a/crates/model/src/extrude.rs
+++ b/crates/model/src/extrude.rs
@@ -96,12 +96,12 @@ fn point_in(p: P, c: &[P]) -> bool {
     inside
 }
 
-/// Most contours grouped into outlines; containment is tested pairwise, so the rest are dropped.
+/// Most contours grouped into outlines; containment is tested pairwise, so the smallest are dropped.
 pub const MAX_CONTOURS: usize = 10_000;
 
 /// Group closed contours into outlines by nesting depth (even depth = outer, odd = hole of the
-/// innermost containing outer contour). Degenerate contours are dropped, as are those past
-/// [`MAX_CONTOURS`].
+/// innermost containing outer contour). Degenerate contours are dropped, as are the smallest
+/// past [`MAX_CONTOURS`] (a kept hole always keeps its larger container).
 pub fn group_contours(contours: Vec<Vec<P>>) -> Vec<Outline> {
     let mut cs: Vec<Vec<P>> = contours
         .into_iter()
@@ -115,8 +115,14 @@ pub fn group_contours(contours: Vec<Vec<P>>) -> Vec<Outline> {
         .filter(|c| c.len() >= 3 && signed_area(c).abs() > 1e-9)
         .collect();
     if cs.len() > MAX_CONTOURS {
-        log::warn!("model: {} contours, grouping the first {MAX_CONTOURS}", cs.len());
-        cs.truncate(MAX_CONTOURS);
+        log::warn!("model: {} contours, grouping the {MAX_CONTOURS} largest", cs.len());
+        // Keep the largest, in their original order.
+        let area: Vec<f64> = cs.iter().map(|c| signed_area(c).abs()).collect();
+        let mut keep: Vec<usize> = (0..cs.len()).collect();
+        keep.sort_by(|&a, &b| area[b].total_cmp(&area[a]));
+        keep.truncate(MAX_CONTOURS);
+        keep.sort_unstable();
+        cs = keep.into_iter().map(|i| std::mem::take(&mut cs[i])).collect();
     }
     let n = cs.len();
     let area: Vec<f64> = cs.iter().map(|c| signed_area(c).abs()).collect();
diff --git a/crates/model/src/obj.rs b/crates/model/src/obj.rs
index 32b3242a0..8b51cb6cf 100644
--- a/crates/model/src/obj.rs
+++ b/crates/model/src/obj.rs
@@ -9,7 +9,7 @@
 //! Phong exponent `Ns` (`sqrt(2 / (Ns + 2))`) and metallic 0; `map_Kd`, `map_Ke`,
 //! `map_Bump`/`bump`/`norm` load base colour, emissive and normal textures.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::sync::Arc;
 
 use crate::{AlphaMode, MAX_IMPORT_TEXTURE_PIXELS, Material, Mesh, Model, Primitive, Result, TexRef, Texture, perr};
@@ -167,7 +167,7 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<
     let mut material_of: HashMap<String, usize> = HashMap::new();
     let mut textures: Vec<Arc<Texture>> = vec![];
     let mut cache = TextureCache::default();
-    let mut libraries: Vec<String> = vec![];
+    let mut libraries: HashSet<String> = HashSet::new();
     // One primitive per material, in order of first use.
     let mut prims: Vec<(Option<usize>, Primitive, HashMap<Key, u32>, Vec<bool>)> = vec![];
     let mut prim_of: HashMap<Option<usize>, usize> = HashMap::new();
@@ -199,10 +199,9 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<
             "mtllib" => {
                 let name = line["mtllib".len()..].trim();
                 // A library named again is already loaded.
-                if libraries.iter().any(|l| l == name) {
+                if !libraries.insert(name.to_string()) {
                     continue;
                 }
-                libraries.push(name.to_string());
                 let mut found = false;
                 for file in std::iter::once(name).chain(name.split_whitespace()).chain(sibling_name(name)) {
                     if let Some(b) = resolve(file) {