From 18c670cc00bc817b4ddd6b7160b1c89f178f9aa6 Mon Sep 17 00:00:00 2001 From: Moritz Moeller Date: Tue, 25 Aug 2026 13:06:27 +0200 Subject: [PATCH 1/2] feat: throttled progress/cancel callback for load_obj_buf LoadOptions gains progress_callback: Option (Arc ControlFlow<()> + Send + Sync>, an options field rather than a second _with_progress function), invoked every 1000 lines during load_obj_buf's parse loop. Returning ControlFlow::Break stops the load and returns the new LoadError::Cancelled. No behavior change when progress_callback is None (the default): verified by a test comparing output with and without a no-op callback. load_obj_buf_async is untouched. --- src/lib.rs | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++- src/tests.rs | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index c559dba..693a825 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -241,8 +241,10 @@ use std::{ fmt, fs::File, io::{prelude::*, BufReader}, + ops::ControlFlow, path::{Path, PathBuf}, str::{FromStr, SplitWhitespace}, + sync::Arc, }; #[cfg(feature = "use_f64")] @@ -276,6 +278,7 @@ pub const GPU_LOAD_OPTIONS: LoadOptions = LoadOptions { triangulate: true, ignore_points: true, ignore_lines: true, + progress_callback: None, }; /// Typical [`LoadOptions`] for using meshes with an offline rendeder. @@ -293,6 +296,7 @@ pub const OFFLINE_RENDERING_LOAD_OPTIONS: LoadOptions = LoadOptions { triangulate: false, ignore_points: true, ignore_lines: true, + progress_callback: None, }; /// A mesh made up of triangles loaded from some `OBJ` file. @@ -404,6 +408,61 @@ pub struct Mesh { pub material_id: Option, } +/// A snapshot of progress made so far while parsing an `OBJ` buffer in +/// [`load_obj_buf()`]. +/// +/// Passed to a [`LoadProgressCallback`] registered via +/// [`LoadOptions::progress_callback`]. The callback is throttled -- it is not +/// invoked for every line read. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LoadProgress { + /// Number of lines read from the buffer so far. + pub lines_read: u64, + /// Number of bytes read from the buffer so far. + /// + /// This is a lower bound: line-ending bytes stripped by + /// [`BufRead::lines()`](std::io::BufRead::lines) are not counted, since + /// they are not seen by the parser. + pub bytes_read: u64, +} + +/// A throttled progress-report and cooperative-cancellation callback. +/// +/// Wraps a closure that is invoked periodically while [`load_obj_buf()`] +/// parses a buffer. Returning [`ControlFlow::Break`] from the closure aborts +/// the load and causes [`load_obj_buf()`] to return +/// [`LoadError::Cancelled`]. +/// +/// Register one via [`LoadOptions::progress_callback`]. +#[derive(Clone)] +pub struct LoadProgressCallback(Arc); + +type LoadProgressCallbackFn = dyn Fn(&LoadProgress) -> ControlFlow<()> + Send + Sync; + +impl LoadProgressCallback { + /// Creates a new [`LoadProgressCallback`] from a closure. + pub fn new(f: impl Fn(&LoadProgress) -> ControlFlow<()> + Send + Sync + 'static) -> Self { + Self(Arc::new(f)) + } + + /// Invokes the wrapped closure with the given `progress` snapshot. + fn call(&self, progress: &LoadProgress) -> ControlFlow<()> { + (self.0)(progress) + } +} + +impl fmt::Debug for LoadProgressCallback { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("LoadProgressCallback(..)") + } +} + +impl PartialEq for LoadProgressCallback { + fn eq(&self, _other: &Self) -> bool { + true // Not data. + } +} + /// Options for processing the mesh during loading. /// /// Passed to [`load_obj()`], [`load_obj_buf()`] and [`load_obj_buf_async()`]. @@ -427,7 +486,7 @@ pub struct Mesh { /// * [`OFFLINE_RENDERING_LOAD_OPTIONS`] – if you're rendering meshes with e.g. /// an offline path tracer or the like. #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] -#[derive(Debug, Default, Clone, Copy)] +#[derive(Debug, Default, Clone)] pub struct LoadOptions { /// Merge identical positions. /// @@ -529,6 +588,16 @@ pub struct LoadOptions { /// Polygon meshes that contains faces with two vertices only usually do so /// because of bad topology. pub ignore_lines: bool, + /// Optional progress-report and cooperative-cancellation callback. + /// + /// If set, [`load_obj_buf()`] invokes it periodically (throttled; not on + /// every line) while parsing, passing it a [`LoadProgress`] snapshot. + /// Returning [`ControlFlow::Break`] from the callback aborts the load and + /// causes [`load_obj_buf()`] to return [`LoadError::Cancelled`]. + /// + /// Not invoked by [`load_obj_buf_async()`]. + #[cfg_attr(feature = "arbitrary", arbitrary(default))] + pub progress_callback: Option, } impl LoadOptions { @@ -649,6 +718,7 @@ pub enum LoadError { FaceColorOutOfBounds, InvalidLoadOptionConfig, GenericFailure, + Cancelled, } impl fmt::Display for LoadError { @@ -672,6 +742,7 @@ impl fmt::Display for LoadError { LoadError::FaceColorOutOfBounds => "face vertex color index out of bounds", LoadError::InvalidLoadOptionConfig => "mutually exclusive load options", LoadError::GenericFailure => "generic failure", + LoadError::Cancelled => "load cancelled by progress callback", }; f.write_str(msg) @@ -2037,10 +2108,23 @@ where return Err(LoadError::InvalidLoadOptionConfig); } + // How often (in lines) to invoke `load_options.progress_callback`, if set. + // Kept coarse so the callback's cost stays negligible next to parsing. + const PROGRESS_REPORT_INTERVAL: u64 = 1000; + let mut models = TmpModels::new(); let mut materials = TmpMaterials::new(); + let mut lines_read: u64 = 0; + let mut bytes_read: u64 = 0; + for line in reader.lines() { + lines_read += 1; + // `BufRead::lines()` strips the line terminator, so this + // undercounts by one byte per line. Good enough for progress + // reporting. + bytes_read += line.as_ref().map(|l| l.len() as u64 + 1).unwrap_or(0); + let parse_return = parse_obj_line(line, load_options, &mut models, &materials)?; match parse_return { ParseReturnType::LoadMaterial(mat_file) => { @@ -2048,6 +2132,18 @@ where } ParseReturnType::None => {} } + + if let Some(callback) = &load_options.progress_callback { + if lines_read.is_multiple_of(PROGRESS_REPORT_INTERVAL) { + let progress = LoadProgress { + lines_read, + bytes_read, + }; + if let ControlFlow::Break(()) = callback.call(&progress) { + return Err(LoadError::Cancelled); + } + } + } } // For the last object in the file we won't encounter another object name to diff --git a/src/tests.rs b/src/tests.rs index f4a86af..038d811 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -3,9 +3,16 @@ use std::{ env, fs::File, io::{BufReader, Cursor}, + ops::ControlFlow, + path::Path, + sync::{ + atomic::{AtomicU64, Ordering}, + Arc, + }, }; use crate as tobj; +use tobj::{load_mtl_buf, load_obj_buf, LoadError, LoadOptions, LoadProgressCallback}; const CORNELL_BOX_OBJ: &str = include_str!("../obj/cornell_box.obj"); const CORNELL_BOX_MTL1: &str = include_str!("../obj/cornell_box.mtl"); @@ -665,6 +672,91 @@ fn test_custom_material_loader_files() { validate_cornell(models, mats); } +#[test] +fn test_progress_callback_noop_matches_no_callback() { + let material_loader = |p: &Path| match p.to_str().unwrap() { + "cornell_box.mtl" => load_mtl_buf(&mut BufReader::new(CORNELL_BOX_MTL1.as_bytes())), + "cornell_box2.mtl" => load_mtl_buf(&mut BufReader::new(CORNELL_BOX_MTL2.as_bytes())), + _ => unreachable!(), + }; + + let without_callback = load_obj_buf( + &mut Cursor::new(CORNELL_BOX_OBJ.as_bytes()), + &LoadOptions { + triangulate: true, + single_index: true, + ..Default::default() + }, + material_loader, + ); + + let with_callback = load_obj_buf( + &mut Cursor::new(CORNELL_BOX_OBJ.as_bytes()), + &LoadOptions { + triangulate: true, + single_index: true, + progress_callback: Some(LoadProgressCallback::new(|_progress| { + ControlFlow::Continue(()) + })), + ..Default::default() + }, + material_loader, + ); + + // A no-op progress callback must not change the parse result in any way. + assert_eq!( + format!("{:?}", without_callback), + format!("{:?}", with_callback) + ); +} + +#[test] +fn test_progress_callback_cancels_load() { + // More lines than the progress-report throttle interval, so the + // callback is guaranteed to fire (and cancel the load) before EOF. + let obj = "v 0.0 0.0 0.0\n".repeat(2500); + + let result = load_obj_buf( + &mut Cursor::new(obj.as_bytes()), + &LoadOptions { + progress_callback: Some(LoadProgressCallback::new( + |_progress| ControlFlow::Break(()), + )), + ..Default::default() + }, + |_| unreachable!("no mtllib in the synthetic buffer"), + ); + + assert_eq!(result.unwrap_err(), LoadError::Cancelled); +} + +#[test] +fn test_progress_callback_is_throttled() { + let line_count = 10_000usize; + let obj = "v 0.0 0.0 0.0\n".repeat(line_count); + + let call_count = Arc::new(AtomicU64::new(0)); + let call_count_clone = call_count.clone(); + let result = load_obj_buf( + &mut Cursor::new(obj.as_bytes()), + &LoadOptions { + progress_callback: Some(LoadProgressCallback::new(move |_progress| { + call_count_clone.fetch_add(1, Ordering::SeqCst); + ControlFlow::Continue(()) + })), + ..Default::default() + }, + |_| unreachable!("no mtllib in the synthetic buffer"), + ); + + assert!(result.is_ok()); + // The callback must be throttled, i.e. called far less often than once + // per line. + let calls = call_count.load(Ordering::SeqCst); + assert!(calls > 0); + assert!((calls as usize) < line_count); +} + #[test] fn test_invalid_index() { let m = tobj::load_obj( From 1d39f267b64011d24d9924395dd353f87e66a6c1 Mon Sep 17 00:00:00 2001 From: Moritz Moeller Date: Tue, 25 Aug 2026 13:15:59 +0200 Subject: [PATCH 2/2] feat!: Mesh::face_arities becomes Option> None now means "all faces are triangles" instead of an empty Vec, saving the allocation for triangle-only meshes -- the common case. New Mesh::{face_count, face_arity, is_triangulated, face_indices} helpers replace manual index-arithmetic over face_arities at call sites (see print_mesh.rs and the module doc example). Also tidies two loops (parse_face, TmpMaterials::extend) into iterator chains while in the area. BREAKING CHANGE: any caller reading Mesh::face_arities directly (as a Vec) needs to match on the Option, or switch to the new helper methods. --- examples/print_mesh.rs | 12 ++-- src/lib.rs | 141 +++++++++++++++++++++++++++++------------ src/tests.rs | 12 ++-- 3 files changed, 112 insertions(+), 53 deletions(-) diff --git a/examples/print_mesh.rs b/examples/print_mesh.rs index 808518e..21ed869 100644 --- a/examples/print_mesh.rs +++ b/examples/print_mesh.rs @@ -18,17 +18,13 @@ fn main() { println!("model[{}].name = \'{}\'", i, m.name); println!("model[{}].mesh.material_id = {:?}", i, mesh.material_id); - println!( - "model[{}].face_count = {}", - i, - mesh.face_arities.len() - ); + println!("model[{}].face_count = {}", i, mesh.face_count()); let mut next_face = 0; - for face in 0..mesh.face_arities.len() { - let end = next_face + mesh.face_arities[face] as usize; + for face in 0..mesh.face_count() { + let end = next_face + mesh.face_arity(face); - let face_indices = &mesh.indices[next_face..end]; + let face_indices = mesh.face_indices(face); println!(" face[{}].indices = {:?}", face, face_indices); if !mesh.texcoord_indices.is_empty() { diff --git a/src/lib.rs b/src/lib.rs index 693a825..8597edd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,18 +83,11 @@ //! println!("model[{}].name = \'{}\'", i, m.name); //! println!("model[{}].mesh.material_id = {:?}", i, mesh.material_id); //! -//! println!( -//! "Size of model[{}].face_arities: {}", -//! i, -//! mesh.face_arities.len() -//! ); +//! println!("model[{}].face_count = {}", i, mesh.face_count()); //! -//! let mut next_face = 0; -//! for f in 0..mesh.face_arities.len() { -//! let end = next_face + mesh.face_arities[f] as usize; -//! let face_indices: Vec<_> = mesh.indices[next_face..end].iter().collect(); +//! for f in 0..mesh.face_count() { +//! let face_indices = mesh.face_indices(f); //! println!(" face[{}] = {:?}", f, face_indices); -//! next_face = end; //! } //! //! // Normals and texture coordinates are also loaded, but not printed in this example @@ -384,14 +377,20 @@ pub struct Mesh { /// Otherwise normals and texture coordinates have *their own* indices, /// each. pub indices: Vec, - /// The number of vertices (arity) of each face. *Empty* if loaded with - /// `triangulate` set to `true` or if the mesh consists *only* of - /// triangles. + /// The number of vertices (arity) of each face. /// - /// The offset for the starting index of a face can be found by iterating - /// through the `face_arities` until reaching the desired face, accumulating - /// the number of vertices used so far. - pub face_arities: Vec, + /// - `None` means all faces are triangles (3 vertices each). + /// - `Some(vec)` contains the vertex count for each face, which may include + /// triangles (3), quads (4), or other polygons. + /// + /// When iterating through faces: + /// - If `None`, each face uses exactly 3 consecutive indices. + /// - If `Some(vec)`, the offset for face `i` is the sum of all previous + /// face arities. + /// + /// This optimization saves memory for triangle-only meshes, which are + /// common in real-time rendering contexts. + pub face_arities: Option>, /// The indices for vertex colors. Only present when the /// [`merging`](LoadOptions::merge_identical_points) feature is enabled, and /// empty unless the corresponding load option is set to `true`. @@ -408,6 +407,65 @@ pub struct Mesh { pub material_id: Option, } +impl Mesh { + /// Returns the number of faces in the mesh. + /// + /// For triangle-only meshes (when `face_arities` is `None`), + /// this is calculated as `indices.len() / 3`. + pub fn face_count(&self) -> usize { + match &self.face_arities { + None => self.indices.len() / 3, + Some(arities) => arities.len(), + } + } + + /// Returns the number of vertices (arity) for a specific face. + /// + /// Returns 3 for triangle-only meshes (when `face_arities` is `None`). + /// Panics if the face index is out of bounds. + pub fn face_arity(&self, face_index: usize) -> usize { + match &self.face_arities { + None => { + assert!( + face_index < self.indices.len() / 3, + "Face index out of bounds" + ); + 3 + } + Some(arities) => { + assert!(face_index < arities.len(), "Face index out of bounds"); + arities[face_index] as usize + } + } + } + + /// Returns true if all faces in the mesh are triangles. + pub fn is_triangulated(&self) -> bool { + self.face_arities.is_none() + } + + /// Returns the indices for a specific face. + /// + /// For triangle-only meshes, returns a slice of exactly 3 indices. + /// For mixed meshes, returns a slice with the appropriate number of + /// indices. + pub fn face_indices(&self, face_index: usize) -> &[u32] { + match &self.face_arities { + None => { + let start = face_index * 3; + assert!(start + 3 <= self.indices.len(), "Face index out of bounds"); + &self.indices[start..start + 3] + } + Some(arities) => { + assert!(face_index < arities.len(), "Face index out of bounds"); + let start: usize = arities[..face_index].iter().map(|&a| a as usize).sum(); + let end = start + arities[face_index] as usize; + &self.indices[start..end] + } + } + } +} + /// A snapshot of progress made so far while parsing an `OBJ` buffer in /// [`load_obj_buf()`]. /// @@ -565,7 +623,7 @@ pub struct LoadOptions { /// `ignore_lines` is/are set to `true`, resp. /// /// * The resulting `Mesh`'s [`face_arities`](Mesh::face_arities) will be - /// empty as all faces are guaranteed to have arity `3`. + /// `None` as all faces are guaranteed to have arity `3`. /// /// * Only polygons that are trivially convertible to triangle fans are /// supported. Arbitrary polygons may not behave as expected. The best @@ -877,13 +935,13 @@ fn parse_face( tex_sz: usize, norm_sz: usize, ) -> bool { - let mut indices = Vec::new(); - for f in face_str { - match VertexIndices::parse(f, pos_sz, tex_sz, norm_sz) { - Some(v) => indices.push(v), - None => return false, - } - } + let indices: Vec = match face_str + .map(|f| VertexIndices::parse(f, pos_sz, tex_sz, norm_sz)) + .collect() + { + Some(indices) => indices, + None => return false, + }; // Check what kind face we read and push it on match indices.len() { 1 => faces.push(Face::Point(indices[0])), @@ -981,7 +1039,7 @@ fn export_faces( add_vertex(&mut mesh, &mut index_map, a, pos, v_color, texcoord, normal)?; } else { is_all_triangles = false; - mesh.face_arities.push(1); + mesh.face_arities.get_or_insert_with(Vec::new).push(1); } } } @@ -993,7 +1051,7 @@ fn export_faces( add_vertex(&mut mesh, &mut index_map, b, pos, v_color, texcoord, normal)?; } else { is_all_triangles = false; - mesh.face_arities.push(2); + mesh.face_arities.get_or_insert_with(Vec::new).push(2); } } } @@ -1002,7 +1060,7 @@ fn export_faces( add_vertex(&mut mesh, &mut index_map, b, pos, v_color, texcoord, normal)?; add_vertex(&mut mesh, &mut index_map, c, pos, v_color, texcoord, normal)?; if !load_options.triangulate { - mesh.face_arities.push(3); + mesh.face_arities.get_or_insert_with(Vec::new).push(3); } } Face::Quad(ref a, ref b, ref c, ref d) => { @@ -1017,7 +1075,7 @@ fn export_faces( } else { add_vertex(&mut mesh, &mut index_map, d, pos, v_color, texcoord, normal)?; is_all_triangles = false; - mesh.face_arities.push(4); + mesh.face_arities.get_or_insert_with(Vec::new).push(4); } } Face::Polygon(ref indices) => { @@ -1035,7 +1093,9 @@ fn export_faces( add_vertex(&mut mesh, &mut index_map, i, pos, v_color, texcoord, normal)?; } is_all_triangles = false; - mesh.face_arities.push(indices.len() as u32); + mesh.face_arities + .get_or_insert_with(Vec::new) + .push(indices.len() as u32); } } } @@ -1043,7 +1103,7 @@ fn export_faces( if is_all_triangles { // This is a triangle-only mesh. - mesh.face_arities = Vec::new(); + mesh.face_arities = None; } Ok(mesh) @@ -1247,7 +1307,7 @@ fn export_faces_multi_index( )?; } else { is_all_triangles = false; - mesh.face_arities.push(1); + mesh.face_arities.get_or_insert_with(Vec::new).push(1); } } } @@ -1289,7 +1349,7 @@ fn export_faces_multi_index( )?; } else { is_all_triangles = false; - mesh.face_arities.push(2); + mesh.face_arities.get_or_insert_with(Vec::new).push(2); } } } @@ -1328,7 +1388,7 @@ fn export_faces_multi_index( normal, )?; if !load_options.triangulate { - mesh.face_arities.push(3); + mesh.face_arities.get_or_insert_with(Vec::new).push(3); } } Face::Quad(ref a, ref b, ref c, ref d) => { @@ -1413,7 +1473,7 @@ fn export_faces_multi_index( normal, )?; is_all_triangles = false; - mesh.face_arities.push(4); + mesh.face_arities.get_or_insert_with(Vec::new).push(4); } } Face::Polygon(ref indices) => { @@ -1471,7 +1531,9 @@ fn export_faces_multi_index( )?; } is_all_triangles = false; - mesh.face_arities.push(indices.len() as u32); + mesh.face_arities + .get_or_insert_with(Vec::new) + .push(indices.len() as u32); } } } @@ -1479,7 +1541,7 @@ fn export_faces_multi_index( if is_all_triangles { // This is a triangle-only mesh. - mesh.face_arities = Vec::new(); + mesh.face_arities = None; } #[cfg(feature = "merging")] @@ -1728,9 +1790,8 @@ impl TmpMaterials { // materials by our current length let mat_offset = self.materials.len(); self.materials.append(&mut mats); - for m in map { - self.mat_map.insert(m.0, m.1 + mat_offset); - } + self.mat_map + .extend(map.into_iter().map(|(name, idx)| (name, idx + mat_offset))); } Err(e) => { self.mtlerr = Some(e); diff --git a/src/tests.rs b/src/tests.rs index 038d811..6da8f78 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -273,17 +273,19 @@ fn non_triangulated_quad() { assert!(mats.is_empty()); // First one is a quad formed by two triangles - // so face_arities is empty (all triangles) - assert!(models[0].mesh.face_arities.is_empty()); + // so face_arities is None (all triangles) + assert!(models[0].mesh.face_arities.is_none()); + assert!(models[0].mesh.is_triangulated()); // Second is a quad face - assert_eq!(models[1].mesh.face_arities.len(), 1); - assert_eq!(models[1].mesh.face_arities[0], 4); + assert_eq!(models[1].mesh.face_count(), 1); + assert_eq!(models[1].mesh.face_arity(0), 4); let expect_quad_indices = vec![0, 1, 2, 3]; assert_eq!(models[1].mesh.indices, expect_quad_indices); // Third is a triangle - assert!(models[2].mesh.face_arities.is_empty()); + assert!(models[2].mesh.face_arities.is_none()); + assert!(models[2].mesh.is_triangulated()); } #[test]