craft-mesh OBJ: map options by argument count, material libraries read once (from open EffectCraft PRs) (#854)

* craft-mesh OBJ: map_* options are passed over by their argument counts

A map line with options named its image by its final token, so
'map_Kd -o 0.5 0.5 my checker.png' named 'checker.png'. Options are now
skipped by the arguments they take (-o, -s and -t take one to three
numbers), so a name with spaces after options is one name. After an option
the reader does not know, the name is still the final token.

Ported from storytold/effectcraft#1324.

* craft-mesh OBJ: a material library named again is read once

Exporters repeat the mtllib line for every object. Each repeat asked the
resolver again, read the library again, added its materials and images a
second time, and counted its bytes against max_data_bytes once per line, so
a legitimate file with many objects could reach the limit.

Ported from storytold/effectcraft#2965 (its OBJ part: 'a library already
loaded is skipped').

---------

Co-authored-by: Lloyd Nicholson <nichlloyd@gmail.com>

crates/mesh/src/obj.rs

@@ -14,7 +14,7 @@
 //! `map_Kd`, `map_Ke`, `map_Bump`/`bump`/`norm` name base colour, emissive and normal images.
 //! An image is only named ([`Image::uri`]); reading and decoding it is the caller's.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::fmt::Write as _;
 
 use crate::{
@@ -32,10 +32,42 @@ pub fn sibling_name(name: &str) -> Option<&str> {
     (!file.is_empty() && file.len() < name.len()).then_some(file)
 }
 
-/// The image a `map_*` line names: the rest of the line (a name with spaces is one name),
-/// or its final token when the line carries options (`-bm 1 file.png`).
-fn map_name(rest: &str) -> &str {
-    if rest.starts_with('-') { rest.split_whitespace().last().unwrap_or(rest) } else { rest }
+/// The image a `map_*` line names: the rest of the line after its options
+/// (`-bm 1 -o 0.5 0.5 my file.png`). Options are passed over by their argument counts, so a
+/// name with spaces is one name; after an option this reader does not know, the name is the
+/// final token.
+fn map_name(args: &str) -> &str {
+    fn split(s: &str) -> (&str, &str) {
+        s.split_once(char::is_whitespace).map_or((s, ""), |(a, b)| (a, b.trim_start()))
+    }
+    let mut rest = args.trim();
+    loop {
+        let (token, after) = split(rest);
+        if after.is_empty() {
+            break;
+        }
+        // How many arguments the option takes, and from which one they may be left out:
+        // `-o`, `-s` and `-t` take one to three numbers.
+        let (count, optional_from) = match token {
+            "-blendu" | "-blendv" | "-bm" | "-boost" | "-cc" | "-clamp" | "-imfchan" | "-texres" | "-type" => (1, 1),
+            "-mm" => (2, 2),
+            "-o" | "-s" | "-t" => (3, 1),
+            t if t.strip_prefix('-').is_some_and(|o| o.starts_with(|c: char| c.is_ascii_alphabetic())) => {
+                return args.split_whitespace().last().unwrap_or(args);
+            }
+            _ => break,
+        };
+        rest = after;
+        for i in 0..count {
+            let (arg, after) = split(rest);
+            // The last token is always the name.
+            if after.is_empty() || (i >= optional_from && arg.parse::<f32>().is_err()) {
+                break;
+            }
+            rest = after;
+        }
+    }
+    rest.trim_end()
 }
 
 /// Materials of an MTL library and the images they name. `TextureRef::image` indexes the
@@ -144,6 +176,8 @@ struct Reader<'a> {
     objects: Vec<Object>,
     scene: Scene,
     material_ids: HashMap<String, usize>,
+    /// The `mtllib` names already read: exporters repeat the line for every object.
+    libraries: HashSet<String>,
     material: Option<usize>,
     group: String,
     smoothing: u32,
@@ -218,7 +252,9 @@ impl Reader<'_> {
     }
 
     fn library(&mut self, name: &str, resolve: Resolver) -> Result<()> {
-        if name.is_empty() {
+        // A library named again is already read: reading it again would add its materials and
+        // images a second time and count its bytes against the limit once per line.
+        if name.is_empty() || !self.libraries.insert(name.to_string()) {
             return Ok(());
         }
         if self.scene.external.len() >= self.limits.max_materials {
@@ -412,6 +448,7 @@ pub fn read(bytes: &[u8], limits: &Limits, resolve: Resolver) -> Result<Scene> {
         objects: vec![],
         scene: Scene::default(),
         material_ids: HashMap::new(),
+        libraries: HashSet::new(),
         material: None,
         group: String::new(),
         smoothing: 0,

crates/mesh/src/tests/obj.rs

@@ -281,3 +281,39 @@ fn writer_places_meshes_keeps_colours_and_names_materials_apart() {
     scene.meshes[0].parts[0].corners[0].position = 9;
     assert!(write(&scene, None, &Limits::default()).is_err());
 }
+
+#[test]
+fn map_options_are_passed_over_by_their_argument_counts() {
+    // `-o` takes one to three numbers, so the name after it may hold spaces; the final token alone
+    // would be `checker.png`.
+    let mtl = "newmtl A\nmap_Kd -o 0.5 0.5 my checker.png\nmap_Bump -bm 0.3 -clamp on rough bump.png\nmap_Ke -s 2 2 2 -mm 0 1 glow.png\n\
+               newmtl B\nmap_Kd -o 1 wood.png\nmap_Bump -unknown 3 my bump.png\nmap_Ke -bm\n";
+    let (materials, images) = read_mtl(mtl.as_bytes(), &Limits::default()).unwrap();
+    let name = |t: Option<crate::TextureRef>| t.map(|t| images[t.image].uri.clone().unwrap());
+    assert_eq!(name(materials[0].base_color_texture).as_deref(), Some("my checker.png"));
+    assert_eq!(name(materials[0].normal_texture).as_deref(), Some("rough bump.png"));
+    assert_eq!(name(materials[0].emissive_texture).as_deref(), Some("glow.png"));
+    assert_eq!(name(materials[1].base_color_texture).as_deref(), Some("wood.png"));
+    // an option this reader does not know: the final token
+    assert_eq!(name(materials[1].normal_texture).as_deref(), Some("bump.png"));
+    // a lone token is the name, whatever it looks like
+    assert_eq!(name(materials[1].emissive_texture).as_deref(), Some("-bm"));
+}
+
+#[test]
+fn a_library_named_again_is_read_once() {
+    // Exporters repeat `mtllib` for every object. Each repeat used to read the library again:
+    // its materials and images twice, and its bytes counted against the limit once per line.
+    let mtl = "newmtl Red\nKd 1 0 0\nmap_Kd red.png\n";
+    let src = "mtllib m.mtl\no a\nv 0 0 0\nv 1 0 0\nv 0 1 0\nusemtl Red\nf 1 2 3\nmtllib m.mtl\no b\nusemtl Red\nf 1 2 3\nmtllib m.mtl\n";
+    let mut asked = 0;
+    let limits = Limits { max_data_bytes: mtl.len() * 2, ..Limits::default() };
+    let s = read(src.as_bytes(), &limits, &mut |_| {
+        asked += 1;
+        Some(mtl.as_bytes().to_vec())
+    })
+    .unwrap();
+    assert_eq!(asked, 1);
+    assert_eq!((s.materials.len(), s.images.len()), (1, 1));
+    assert_eq!(s.external, ["m.mtl", "red.png"]);
+}