diff --git a/crates/model/src/fbx.rs b/crates/model/src/fbx.rs index bca387ff2..6f79a77d0 100644 --- a/crates/model/src/fbx.rs +++ b/crates/model/src/fbx.rs @@ -230,7 +230,7 @@ impl Bin<'_> { } let mut out = vec![]; while *pos < end { - match self.record(pos, depth)? { + match self.record(pos, end, depth)? { Some(n) => out.push(n), None => break, } @@ -238,8 +238,9 @@ impl Bin<'_> { Ok(out) } - /// One node record; `None` for the null record that closes a list. - fn record(&self, pos: &mut usize, depth: usize) -> Result> { + /// One node record ending within `limit` (the enclosing record's end); `None` for the null + /// record that closes a list. + fn record(&self, pos: &mut usize, limit: usize, depth: usize) -> Result> { let start = *pos; let end = Self::to_usize(self.offset(pos)?)?; let nprops = self.offset(pos)?; @@ -248,7 +249,9 @@ impl Bin<'_> { if end == 0 && nprops == 0 && plen == 0 && nlen == 0 { return Ok(None); } - if end <= start || end > self.d.len() { + // A child reaching past its parent would be parsed again once the parent's end is + // restored. + if end <= start || end > limit.min(self.d.len()) { return Err(perr(format!("FBX: bad record end {end} at byte {start}"))); } let name = String::from_utf8_lossy(self.take(pos, nlen)?).into_owned(); @@ -302,7 +305,8 @@ impl Bin<'_> { let data = sub.take(pos, clen)?; let raw = match enc { 0 => data.to_vec(), - 1 => miniz_oxide::inflate::decompress_to_vec_zlib_with_limit(data, want).map_err(|e| perr(format!("FBX: array zlib: {e:?}")))?, + // The status only: the error's Debug form prints what was decompressed. + 1 => miniz_oxide::inflate::decompress_to_vec_zlib_with_limit(data, want).map_err(|e| perr(format!("FBX: array zlib: {:?}", e.status)))?, e => return Err(perr(format!("FBX: unknown array encoding {e}"))), }; let raw = raw.get(..want).ok_or_else(|| perr("FBX: array shorter than its length"))?; @@ -334,7 +338,7 @@ fn read_binary(bytes: &[u8], budget: &Budget) -> Result<(u32, Vec)> { let bin = Bin { d: bytes, wide: version >= 7500, budget }; let mut nodes = vec![]; while pos < bytes.len() { - match bin.record(&mut pos, 0) { + match bin.record(&mut pos, bytes.len(), 0) { Ok(Some(n)) => nodes.push(n), Ok(None) => break, // A damaged tail (footer, padding) after complete sections is ignored. @@ -651,6 +655,7 @@ fn build_mesh(name: &str, geom: &FbxNode, materials: &[usize], with_normal_maps: type Key = (u32, [u32; 3], [u32; 2]); let mut prims: Vec<(Option, Primitive, HashMap, Vec)> = vec![]; + let mut prim_of: HashMap, usize> = HashMap::new(); let mut corners: Vec<(usize, usize)> = vec![]; let mut poly = 0usize; for (pv, &raw) in pvi.iter().enumerate() { @@ -671,13 +676,10 @@ fn build_mesh(name: &str, geom: &FbxNode, materials: &[usize], with_normal_maps: materials.get(usize::try_from(k).ok()?).copied() }); let slot = slot.or_else(|| materials.first().copied()); - let at = match prims.iter().position(|p| p.0 == slot) { - Some(i) => i, - None => { - prims.push((slot, Primitive { material: slot, ..Default::default() }, HashMap::new(), vec![])); - prims.len() - 1 - } - }; + let at = *prim_of.entry(slot).or_insert_with(|| { + prims.push((slot, Primitive { material: slot, ..Default::default() }, HashMap::new(), vec![])); + prims.len() - 1 + }); let Some((_, prim, map, need)) = prims.get_mut(at) else { continue }; let mut ids = Vec::with_capacity(poly_corners.len()); for &(pv, v) in &poly_corners { @@ -816,6 +818,12 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, limit: links.push((a, b, e.str_at(3))); } } + // The same links by parent, so each object finds its children without scanning them all. + let mut children_of: HashMap)>> = HashMap::new(); + for &(a, b, prop) in &links { + children_of.entry(b).or_default().push((a, prop)); + } + let children_of = |id: i64| children_of.get(&id).map(Vec::as_slice).unwrap_or_default().iter().copied(); let model_ids: Vec = objects.children.iter().filter(|o| o.name == "Model").filter_map(|o| o.prop(0)?.as_i64()).collect(); let mut node_of: HashMap = HashMap::new(); @@ -851,14 +859,23 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, limit: let mut textures: Vec> = vec![]; let mut material_of: HashMap = HashMap::new(); let mut texture_of: HashMap> = HashMap::new(); + // Decoded images by their source (embedded Video, then file name): Texture objects aliasing + // one image share its pixels instead of each keeping a copy. + let mut image_of: HashMap<(Vec, Vec), Option> = HashMap::new(); let mut load_texture = |tid: i64, textures: &mut Vec>| -> Option { if let Some(t) = texture_of.get(&tid) { return *t; } let t = (|| { let tex = by_id.get(&tid)?; - let embedded = links.iter().filter(|l| l.1 == tid && kind(l.0) == Some("Video")).find_map(|l| { - let content = by_id.get(&l.0)?.child("Content")?; + let videos: Vec = children_of(tid).filter(|l| kind(l.0) == Some("Video")).map(|l| l.0).collect(); + let files: Vec = ["RelativeFilename", "FileName"].iter().filter_map(|k| Some(tex.child(k)?.str_at(0)?.replace('\\', "/"))).collect(); + let key = (videos, files); + if let Some(i) = image_of.get(&key) { + return *i; + } + let embedded = key.0.iter().find_map(|v| { + let content = by_id.get(v)?.child("Content")?; match content.props.first()? { Prop::Bytes(b) if !b.is_empty() => Some(b.clone()), Prop::Str(_) => { @@ -868,15 +885,15 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, limit: _ => None, } }); - let bytes = embedded.or_else(|| { - ["RelativeFilename", "FileName"].iter().find_map(|k| { - let f = tex.child(k)?.str_at(0)?.replace('\\', "/"); - resolve(&f).or_else(|| resolve(f.rsplit('/').next()?)) - }) - })?; - let t = Texture::decode(&bytes).ok()?; - textures.push(Arc::new(t)); - Some(textures.len() - 1) + let i = (|| { + let bytes = embedded.or_else(|| key.1.iter().find_map(|f| resolve(f).or_else(|| resolve(f.rsplit('/').next()?))))?; + let t = Texture::decode(&bytes).ok()?; + budget.take(t.data.len().checked_mul(16), "a texture").ok()?; + textures.push(Arc::new(t)); + Some(textures.len() - 1) + })(); + image_of.insert(key, i); + i })(); texture_of.insert(tid, t); t @@ -887,7 +904,7 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, limit: } let node = by_id.get(&mid)?; let mut m = material(short_name(node.str_at(1).unwrap_or("")), node); - for &(tid, _, prop) in links.iter().filter(|l| l.1 == mid && kind(l.0) == Some("Texture")) { + for (tid, prop) in children_of(mid).filter(|l| kind(l.0) == Some("Texture")) { let slot = match prop { Some("DiffuseColor") => &mut m.base_color_tex, Some("NormalMap") => &mut m.normal_tex, @@ -912,10 +929,10 @@ fn parse_within(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, limit: let mut mesh_of: HashMap<(i64, Vec), usize> = HashMap::new(); for &id in &model_ids { let Some(&ni) = node_of.get(&id) else { continue }; - let Some(gid) = links.iter().find(|l| l.1 == id && kind(l.0) == Some("Geometry")).map(|l| l.0) else { continue }; + let Some(gid) = children_of(id).find(|l| kind(l.0) == Some("Geometry")).map(|l| l.0) else { continue }; let Some(geom) = by_id.get(&gid) else { continue }; let mats: Vec = - links.iter().filter(|l| l.1 == id && kind(l.0) == Some("Material")).filter_map(|l| material_index(l.0, &mut materials, &mut textures)).collect(); + children_of(id).filter(|l| kind(l.0) == Some("Material")).filter_map(|l| material_index(l.0, &mut materials, &mut textures)).collect(); let name = nodes.get(ni).map(|n| n.name.clone()).unwrap_or_default(); let mesh_index = match mesh_of.get(&(gid, mats.clone())) { Some(&i) => i, diff --git a/crates/model/src/fbx/tests.rs b/crates/model/src/fbx/tests.rs index 1ba1978ba..145fd6299 100644 --- a/crates/model/src/fbx/tests.rs +++ b/crates/model/src/fbx/tests.rs @@ -409,3 +409,49 @@ fn fbx_models_instancing_one_geometry_share_its_mesh() { assert!(parse_within(instanced_scene().as_bytes(), &|_| None, values + 2 * mesh).is_ok()); assert!(parse_within(instanced_scene().as_bytes(), &|_| None, values + mesh).is_err()); } + +#[test] +fn binary_child_record_past_its_parent_is_an_error() { + let parent = FbxNode { name: "P".into(), props: vec![], children: vec![FbxNode { name: "C".into(), props: vec![], children: vec![] }] }; + let after = FbxNode { name: "Q".into(), props: vec![Prop::Int(1)], children: vec![] }; + let mut bin = write_binary(&[parent, after], 7400, false); + assert_eq!(read(&bin).unwrap().1.len(), 2); + // `P` starts after the magic and version; `C` after P's 12-byte header, name length and name. + let p = BINARY_MAGIC.len() + 4; + let p_end = u32::from_le_bytes(bin[p..p + 4].try_into().unwrap()); + let c = p + 14; + bin[c..c + 4].copy_from_slice(&(p_end + 1).to_le_bytes()); + assert!(read(&bin).unwrap_err().to_string().contains("bad record end")); +} + +#[test] +fn texture_aliases_of_one_image_share_its_pixels() { + let png = { + let img = image::RgbaImage::from_pixel(2, 1, image::Rgba([0, 255, 0, 255])); + let mut b = std::io::Cursor::new(vec![]); + img.write_to(&mut b, image::ImageFormat::Png).unwrap(); + b.into_inner() + }; + let objects = r#" + Texture: 600, "Texture::Base", "" { + RelativeFilename: "maps\base.png" + } + Texture: 700, "Texture::Base again", "" { + RelativeFilename: "maps\base.png" + } +} +Connections: {"#; + let doc = ASCII + .replace("}\nConnections: {", objects) + .replace("C: \"OO\",500,300\n", "C: \"OO\",500,300\n\tC: \"OP\",600,400, \"DiffuseColor\"\n\tC: \"OP\",700,500, \"DiffuseColor\"\n"); + let resolve = |uri: &str| (uri == "maps/base.png").then(|| png.clone()); + 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). + 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")); +}