FBX: textures past the limits are left out with an import warning, not a failed import; 2^29-pixel allowance

crates/model/src/fbx.rs

@@ -36,6 +36,12 @@ const MAX_ARRAY_BYTES: usize = 1 << 30;
 /// wire type) plus the meshes built from them (#1234).
 const MAX_IMPORT_BYTES: usize = 1 << 31;
 
+/// Most pixels one texture may hold (8192 × 8192) and all the textures of one import together
+/// (2^29: 8 GiB as float RGBA, thirty-two 4K maps). A texture past a limit is left out and the
+/// import says so.
+const MAX_TEXTURE_PIXELS: u64 = 1 << 26;
+const MAX_TEXTURE_IMPORT_PIXELS: u64 = 1 << 29;
+
 /// What is left of an import's memory budget.
 struct Budget(std::cell::Cell<usize>);
 
@@ -862,6 +868,10 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>, limit:
     // Decoded images by the source actually used (embedded Video id, else the resolved file
     // name): Texture objects aliasing one image share its pixels instead of each keeping a copy.
     let mut image_of: HashMap<(Option<i64>, String), Option<usize>> = HashMap::new();
+    // Textures have their own allowance (pixels, as `Texture::decode_within` counts them); the
+    // memory budget is for the file's own records.
+    let mut texture_pixels_left = MAX_TEXTURE_IMPORT_PIXELS;
+    let mut warnings: Vec<String> = vec![];
     // Ok(None): no usable image (the material goes untextured). Err: the memory budget ran out.
     let mut load_texture = |tid: i64, textures: &mut Vec<Arc<Texture>>| -> Result<Option<usize>> {
         if let Some(t) = texture_of.get(&tid) {
@@ -900,16 +910,30 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>, limit:
                 Some(i) => *i,
                 None => {
                     // Charge the decoded size before decoding, so a small file claiming huge
-                    // dimensions is refused before its pixels are allocated.
-                    let i = match Texture::dimensions(&bytes) {
-                        Ok((w, h)) => {
-                            budget.take((w as usize).checked_mul(h as usize).and_then(|n| n.checked_mul(16)), "a texture")?;
-                            Texture::decode(&bytes).ok().map(|t| {
-                                textures.push(Arc::new(t));
-                                textures.len() - 1
-                            })
+                    // dimensions is refused before its pixels are allocated. A texture past the
+                    // limits is left out (the material goes untextured) and the import says so.
+                    let loaded = Texture::dimensions(&bytes).and_then(|(w, h)| {
+                        let pixels = u64::from(w) * u64::from(h);
+                        if pixels > MAX_TEXTURE_PIXELS {
+                            return Err(perr(format!("a {w}×{h} texture (at most {MAX_TEXTURE_PIXELS} pixels)")));
+                        }
+                        if pixels > texture_pixels_left {
+                            return Err(perr(format!("a {w}×{h} texture: the model's textures would hold more than {MAX_TEXTURE_IMPORT_PIXELS} pixels")));
+                        }
+                        let t = Texture::decode(&bytes)?;
+                        texture_pixels_left -= pixels;
+                        Ok(t)
+                    });
+                    let i = match loaded {
+                        Ok(t) => {
+                            textures.push(Arc::new(t));
+                            Some(textures.len() - 1)
+                        }
+                        Err(e) => {
+                            log::warn!("model: FBX texture left out: {e}");
+                            warnings.push(format!("texture left out: {e}"));
+                            None
                         }
-                        Err(_) => None,
                     };
                     image_of.insert(key, i);
                     i
@@ -987,7 +1011,7 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>, limit:
     if meshes.is_empty() {
         return Err(perr("FBX file without meshes"));
     }
-    Ok(Model { meshes, nodes, roots: vec![0], materials, textures, ..Model::default() })
+    Ok(Model { meshes, nodes, roots: vec![0], materials, textures, warnings, ..Model::default() })
 }
 
 #[cfg(test)]

crates/model/src/fbx/tests.rs

@@ -448,12 +448,12 @@ Connections:  {"#;
     let m = parse(doc.as_bytes(), &resolve).unwrap();
     assert_eq!(m.textures.len(), 1);
     assert!(m.materials.iter().all(|x| x.base_color_tex.map(|t| t.texture) == Some(0)));
-    // Decoded pixels count against the import's memory budget (the two pixels take 32 bytes).
+    // Textures have their own allowance: a tight record budget does not drop them.
     let (_, nodes) = read(doc.as_bytes()).unwrap();
     let values = value_bytes(&nodes).unwrap();
     let mesh = mesh_bytes(&m.meshes[0]).unwrap();
-    assert_eq!(parse_within(doc.as_bytes(), &resolve, values + mesh + 32).unwrap().textures.len(), 1);
-    assert!(parse_within(doc.as_bytes(), &resolve, values + mesh + 31).unwrap_err().to_string().contains("memory limit"));
+    assert_eq!(parse_within(doc.as_bytes(), &resolve, values + mesh).unwrap().textures.len(), 1);
+    assert!(m.warnings.is_empty());
 }
 
 /// A PNG that is only a header claiming `w`×`h` pixels (an empty data chunk follows).
@@ -478,7 +478,7 @@ fn png_header_only(w: u32, h: u32) -> Vec<u8> {
 }
 
 #[test]
-fn texture_budget_is_charged_before_decoding_and_its_error_propagates() {
+fn a_texture_past_the_limits_is_left_out_with_a_warning_not_a_failed_import() {
     let objects = r#"
 	Texture: 600, "Texture::Base", "" {
 		RelativeFilename: "maps\base.png"
@@ -486,10 +486,12 @@ fn texture_budget_is_charged_before_decoding_and_its_error_propagates() {
 }
 Connections:  {"#;
     let doc = ASCII.replace("}\nConnections:  {", objects).replace("C: \"OO\",500,300\n", "C: \"OO\",500,300\n\tC: \"OP\",600,400, \"DiffuseColor\"\n");
-    // 30000×30000 pixels would take ~14 GiB; the header alone must be refused against the budget.
+    // 30000×30000 pixels would take ~14 GiB; the header alone is refused, before anything is allocated.
     let huge = png_header_only(30000, 30000);
     let resolve = |_: &str| Some(huge.clone());
-    assert!(parse(doc.as_bytes(), &resolve).unwrap_err().to_string().contains("memory limit"));
+    let m = parse(doc.as_bytes(), &resolve).unwrap();
+    assert!(m.textures.is_empty() && m.materials.iter().all(|x| x.base_color_tex.is_none()));
+    assert!(m.warnings.iter().any(|w| w.contains("30000")), "{:?}", m.warnings);
 }
 
 #[test]