From 1d9320445d3d84df3bc9fe8977da4cf480da1fbf Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 23:56:44 +0000 Subject: [PATCH] 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 | 44 +++++++++++++++++++++++++++-------- crates/model/src/fbx/tests.rs | 14 ++++++----- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/crates/model/src/fbx.rs b/crates/model/src/fbx.rs index fe98f5a7b..4f952847c 100644 --- a/crates/model/src/fbx.rs +++ b/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); @@ -862,6 +868,10 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, 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, String), Option> = 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 = 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>| -> Result> { if let Some(t) = texture_of.get(&tid) { @@ -900,16 +910,30 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, 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>, 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)] diff --git a/crates/model/src/fbx/tests.rs b/crates/model/src/fbx/tests.rs index e83081bdd..fcd34c4f0 100644 --- a/crates/model/src/fbx/tests.rs +++ b/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 { } #[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]