From dc6b775aa5df75a143a1bddcaa3ffb85a2f14ab3 Mon Sep 17 00:00:00 2001 From: mikeyo98 Date: Wed, 5 Aug 2026 13:33:11 -0400 Subject: [PATCH 1/3] Composite zsorting fix - Try to follow the inochi2d creator composite zsort behaviour I observed: all composite descendants' zsort must be sorted together; (TODO) for nested composites, only the top one is an active composite, treat the rest as a normal node in zsorting. - Use a stack to keep track of descendents (optimization needed) --- inox2d/src/render.rs | 181 ++++++++++++++++++++++++++++++------------- 1 file changed, 126 insertions(+), 55 deletions(-) diff --git a/inox2d/src/render.rs b/inox2d/src/render.rs index c77fe74..5a6bf8a 100644 --- a/inox2d/src/render.rs +++ b/inox2d/src/render.rs @@ -1,7 +1,7 @@ mod deform_stack; mod vertex_buffers; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::mem::swap; use crate::node::{ @@ -14,6 +14,8 @@ use crate::puppet::{InoxNodeTree, Puppet, World}; pub use vertex_buffers::VertexBuffers; +/// TODO (maybe): additional info for each node to avoid stack operations (node's depth in tree for example) + /// Additional info per node for rendering a TexturedMesh: /// - offset and length of array for mesh point coordinates /// - offset and length of array for indices of mesh points defining the mesh @@ -60,11 +62,41 @@ impl RenderCtx { let mut vertex_buffers = VertexBuffers::default(); let mut root_drawables_count: usize = 0; - for node in nodes.iter() { + + // composite uuid => (descendant list, drawable count) + let mut composite_descendent_lists: HashMap, usize)> = HashMap::new(); + // (node_uuid, maybe_highest_composite_ancestor) + let mut ancestor_stack: Vec<(InoxNodeUuid, Option)> = Vec::new(); + + for node in nodes.pre_order_iter() { + // pop until top of stack is parent of current + while let Some(top) = ancestor_stack.last() { + if nodes.get_parent(node.uuid).uuid == top.0 { + break; + } + ancestor_stack.pop(); + } + + let parent_target_id = ancestor_stack.last().and_then(|curr_top| curr_top.1); + + // either a descendant of current highest composite + if let Some(target_id) = parent_target_id { + composite_descendent_lists + .entry(target_id) + .or_default() + .0 + .push(node.uuid); + ancestor_stack.push((node.uuid, Some(target_id))); + } + let drawable_kind = DrawableKind::new(node.uuid, comps, true); if let Some(drawable_kind) = drawable_kind { root_drawables_count += 1; + if let Some(target_id) = parent_target_id { + composite_descendent_lists.entry(target_id).or_default().1 += 1; + } + match drawable_kind { DrawableKind::TexturedMesh(components) => { let (index_offset, vert_offset) = vertex_buffers.push(components.mesh); @@ -86,33 +118,50 @@ impl RenderCtx { } } DrawableKind::Composite { .. } => { - // exclude non-drawable children - let children_list: Vec = nodes - .get_children(node.uuid) - .filter_map(|n| { - if DrawableKind::new(n.uuid, comps, false).is_some() { - Some(n.uuid) - } else { - None - } - }) - .collect(); - - // composite children are excluded from root_drawables_zsorted - root_drawables_count -= children_list.len(); + // currently only include a single level of children + // TODO: try to make the descendents into the children list + if parent_target_id.is_none() { + ancestor_stack.push((node.uuid, Some(node.uuid))); + } - comps.add( - node.uuid, - CompositeRenderCtx { - // sort later, before render - zsorted_children_list: children_list, - }, - ); + // let children_list: Vec = nodes + // .get_children(node.uuid) + // .filter_map(|n| { + // if DrawableKind::new(n.uuid, comps, false).is_some() { + // Some(n.uuid) + // } else { + // None + // } + // }) + // .collect(); + + // // composite children are excluded from root_drawables_zsorted + // root_drawables_count -= children_list.len(); + + // comps.add( + // node.uuid, + // CompositeRenderCtx { + // // sort later, before render + // zsorted_children_list: children_list, + // }, + // ); } }; } } + composite_descendent_lists + .iter() + .for_each(|(composite_id, (descendents, drawable_count))| { + root_drawables_count -= drawable_count; + comps.add( + *composite_id, + CompositeRenderCtx { + zsorted_children_list: descendents.clone(), + }, + ); + }); + let mut root_drawables_zsorted = Vec::new(); // similarly, populate later, before render root_drawables_zsorted.resize(root_drawables_count, InoxNodeUuid(0)); @@ -136,48 +185,69 @@ impl RenderCtx { pub(crate) fn update(&mut self, nodes: &InoxNodeTree, comps: &mut World) { let mut root_drawable_uuid_zsort_vec = Vec::<(InoxNodeUuid, f32)>::new(); + let mut stack: Vec<(InoxNodeUuid, Option)> = Vec::new(); + // root is definitely not a drawable. - for node in nodes.iter().skip(1) { + for node in nodes.pre_order_iter().skip(1) { + while let Some(top) = stack.last() { + if nodes.get_parent(node.uuid).uuid == top.0 { + break; + } + stack.pop(); + } + + let top_composite = stack.last().and_then(|top| top.1); + if let Some(top_composite) = top_composite { + stack.push((node.uuid, Some(top_composite))); + } if let Some(drawable_kind) = DrawableKind::new(node.uuid, comps, false) { - let parent = nodes.get_parent(node.uuid); let node_zsort = comps.get::(node.uuid).unwrap().0; - if !matches!( - DrawableKind::new(parent.uuid, comps, false), - Some(DrawableKind::Composite(_)) - ) { - // exclude composite children + if top_composite.is_none() { root_drawable_uuid_zsort_vec.push((node.uuid, node_zsort)); } + // if !matches!( + // DrawableKind::new(parent.uuid, comps, false), + // Some(DrawableKind::Composite(_)) + // ) { + // // exclude composite children + // root_drawable_uuid_zsort_vec.push((node.uuid, node_zsort)); + // } + match drawable_kind { // for Composite, update zsorted children list DrawableKind::Composite { .. } => { - // `swap()` usage is a trick that both: - // - returns mut borrowed comps early - // - does not involve any heap allocations - let mut zsorted_children_list = Vec::new(); - swap( - &mut zsorted_children_list, - &mut comps - .get_mut::(node.uuid) - .unwrap() - .zsorted_children_list, - ); + if top_composite.is_none() { + stack.push((node.uuid, Some(node.uuid))); + + // Nested composites behave like a normal part zsort-wise + // `swap()` usage is a trick that both: + // - returns mut borrowed comps early + // - does not involve any heap allocations + let mut zsorted_children_list = Vec::new(); + swap( + &mut zsorted_children_list, + &mut comps + .get_mut::(node.uuid) + .unwrap() + .zsorted_children_list, + ); - zsorted_children_list.sort_by(|a, b| { - let zsort_a = comps.get::(*a).unwrap(); - let zsort_b = comps.get::(*b).unwrap(); - zsort_a.total_cmp(zsort_b).reverse() - }); - - swap( - &mut zsorted_children_list, - &mut comps - .get_mut::(node.uuid) - .unwrap() - .zsorted_children_list, - ); + zsorted_children_list.sort_by(|a, b| { + let zsort_a = comps.get::(*a).unwrap(); + let zsort_b = comps.get::(*b).unwrap(); + zsort_a.total_cmp(zsort_b).reverse() + }); + + swap( + &mut zsorted_children_list, + &mut comps + .get_mut::(node.uuid) + .unwrap() + .zsorted_children_list, + ); + } } // for TexturedMesh, obtain and write deforms into vertex_buffer DrawableKind::TexturedMesh(..) => { @@ -324,6 +394,7 @@ impl InoxRendererExt for T { DrawableKind::TexturedMesh(components) => { self.draw_textured_mesh_content(as_mask, &components, comps.get(*uuid).unwrap(), *uuid) } + // TODO: Composite inside composite not handled yet DrawableKind::Composite { .. } => panic!("Composite inside Composite not allowed."), } } @@ -337,7 +408,7 @@ impl InoxRendererExt for T { /// /// This does not guarantee the display of a puppet on screen due to these possible reasons: /// - Only provided `InoxRenderer` method implementations are called. - /// + /// /// For example, maybe the caller still need to transfer content from a texture buffer to the screen surface buffer. /// - The provided `InoxRender` implementation is wrong. /// - `puppet` here does not belong to the `model` this `renderer` is initialized with. This will likely result in panics for non-existent node uuids. From 252dc125bdd6363f3339589be2dcae95c6e6ece3 Mon Sep 17 00:00:00 2001 From: mikeyo98 Date: Fri, 7 Aug 2026 04:17:30 -0400 Subject: [PATCH 2/3] Allow composite inside composite - add readable variable naming --- inox2d/src/render.rs | 76 ++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 52 deletions(-) diff --git a/inox2d/src/render.rs b/inox2d/src/render.rs index 5a6bf8a..9322677 100644 --- a/inox2d/src/render.rs +++ b/inox2d/src/render.rs @@ -77,23 +77,23 @@ impl RenderCtx { ancestor_stack.pop(); } - let parent_target_id = ancestor_stack.last().and_then(|curr_top| curr_top.1); + let top_composite = ancestor_stack.last().and_then(|curr_top| curr_top.1); // either a descendant of current highest composite - if let Some(target_id) = parent_target_id { + if let Some(top_composite) = top_composite { composite_descendent_lists - .entry(target_id) + .entry(top_composite) .or_default() .0 .push(node.uuid); - ancestor_stack.push((node.uuid, Some(target_id))); + ancestor_stack.push((node.uuid, Some(top_composite))); } let drawable_kind = DrawableKind::new(node.uuid, comps, true); if let Some(drawable_kind) = drawable_kind { root_drawables_count += 1; - if let Some(target_id) = parent_target_id { + if let Some(target_id) = top_composite { composite_descendent_lists.entry(target_id).or_default().1 += 1; } @@ -118,33 +118,9 @@ impl RenderCtx { } } DrawableKind::Composite { .. } => { - // currently only include a single level of children - // TODO: try to make the descendents into the children list - if parent_target_id.is_none() { + if top_composite.is_none() { ancestor_stack.push((node.uuid, Some(node.uuid))); } - - // let children_list: Vec = nodes - // .get_children(node.uuid) - // .filter_map(|n| { - // if DrawableKind::new(n.uuid, comps, false).is_some() { - // Some(n.uuid) - // } else { - // None - // } - // }) - // .collect(); - - // // composite children are excluded from root_drawables_zsorted - // root_drawables_count -= children_list.len(); - - // comps.add( - // node.uuid, - // CompositeRenderCtx { - // // sort later, before render - // zsorted_children_list: children_list, - // }, - // ); } }; } @@ -185,20 +161,20 @@ impl RenderCtx { pub(crate) fn update(&mut self, nodes: &InoxNodeTree, comps: &mut World) { let mut root_drawable_uuid_zsort_vec = Vec::<(InoxNodeUuid, f32)>::new(); - let mut stack: Vec<(InoxNodeUuid, Option)> = Vec::new(); + let mut ancestor_stack: Vec<(InoxNodeUuid, Option)> = Vec::new(); // root is definitely not a drawable. for node in nodes.pre_order_iter().skip(1) { - while let Some(top) = stack.last() { + while let Some(top) = ancestor_stack.last() { if nodes.get_parent(node.uuid).uuid == top.0 { break; } - stack.pop(); + ancestor_stack.pop(); } - let top_composite = stack.last().and_then(|top| top.1); + let top_composite = ancestor_stack.last().and_then(|top| top.1); if let Some(top_composite) = top_composite { - stack.push((node.uuid, Some(top_composite))); + ancestor_stack.push((node.uuid, Some(top_composite))); } if let Some(drawable_kind) = DrawableKind::new(node.uuid, comps, false) { let node_zsort = comps.get::(node.uuid).unwrap().0; @@ -207,21 +183,14 @@ impl RenderCtx { root_drawable_uuid_zsort_vec.push((node.uuid, node_zsort)); } - // if !matches!( - // DrawableKind::new(parent.uuid, comps, false), - // Some(DrawableKind::Composite(_)) - // ) { - // // exclude composite children - // root_drawable_uuid_zsort_vec.push((node.uuid, node_zsort)); - // } - match drawable_kind { // for Composite, update zsorted children list DrawableKind::Composite { .. } => { + // Nested composites behave like a normal part zsort-wise + // i.e. not owning or managing a zsorted children list if top_composite.is_none() { - stack.push((node.uuid, Some(node.uuid))); + ancestor_stack.push((node.uuid, Some(node.uuid))); - // Nested composites behave like a normal part zsort-wise // `swap()` usage is a trick that both: // - returns mut borrowed comps early // - does not involve any heap allocations @@ -388,14 +357,16 @@ impl InoxRendererExt for T { self.begin_composite_content(as_mask, components, render_ctx, id); for uuid in &render_ctx.zsorted_children_list { - let drawable_kind = DrawableKind::new(*uuid, comps, false) - .expect("All children in zsorted_children_list should be a Drawable."); - match drawable_kind { - DrawableKind::TexturedMesh(components) => { - self.draw_textured_mesh_content(as_mask, &components, comps.get(*uuid).unwrap(), *uuid) + if let Some(drawable_kind) = DrawableKind::new(*uuid, comps, false) + //.expect("All children in zsorted_children_list should be a Drawable."); + { + match drawable_kind { + DrawableKind::TexturedMesh(components) => { + self.draw_textured_mesh_content(as_mask, &components, comps.get(*uuid).unwrap(), *uuid) + } + // TODO: Composite inside composite not handled yet + DrawableKind::Composite { .. } => continue, //panic!("Composite inside Composite not allowed."), } - // TODO: Composite inside composite not handled yet - DrawableKind::Composite { .. } => panic!("Composite inside Composite not allowed."), } } @@ -419,6 +390,7 @@ impl InoxRendererExt for T { .expect("RenderCtx of puppet must be initialized before calling draw().") .root_drawables_zsorted { + let node = puppet.nodes.get_node(*uuid); self.draw_drawable(false, &puppet.node_comps, *uuid); } } From 4c12ddb808e40ad3371afb52253f9647ebe42324 Mon Sep 17 00:00:00 2001 From: jza221 Date: Tue, 11 Aug 2026 12:07:46 -0700 Subject: [PATCH 3/3] Some testing shows this doesn't add any noticeable cpu time during puppet.end_frame(). No further optimization until PR review. - Improved comments for readability --- inox2d/src/render.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/inox2d/src/render.rs b/inox2d/src/render.rs index 9322677..4f86422 100644 --- a/inox2d/src/render.rs +++ b/inox2d/src/render.rs @@ -14,8 +14,6 @@ use crate::puppet::{InoxNodeTree, Puppet, World}; pub use vertex_buffers::VertexBuffers; -/// TODO (maybe): additional info for each node to avoid stack operations (node's depth in tree for example) - /// Additional info per node for rendering a TexturedMesh: /// - offset and length of array for mesh point coordinates /// - offset and length of array for indices of mesh points defining the mesh @@ -63,13 +61,14 @@ impl RenderCtx { let mut root_drawables_count: usize = 0; - // composite uuid => (descendant list, drawable count) + // Composite uuid => (descendant list, drawable count) let mut composite_descendent_lists: HashMap, usize)> = HashMap::new(); - // (node_uuid, maybe_highest_composite_ancestor) + // Vec<(Node uuid, maybe highest Composite ancestor)> + // This stack skips on nodes that don't have a Composite ancestor let mut ancestor_stack: Vec<(InoxNodeUuid, Option)> = Vec::new(); for node in nodes.pre_order_iter() { - // pop until top of stack is parent of current + // Pop until top of stack is parent of current node while let Some(top) = ancestor_stack.last() { if nodes.get_parent(node.uuid).uuid == top.0 { break; @@ -79,8 +78,8 @@ impl RenderCtx { let top_composite = ancestor_stack.last().and_then(|curr_top| curr_top.1); - // either a descendant of current highest composite if let Some(top_composite) = top_composite { + // Current node is a descendant of current highest Composite (could be anything, a Node, Part, Meshgroup, Composite etc.) composite_descendent_lists .entry(top_composite) .or_default() @@ -119,6 +118,8 @@ impl RenderCtx { } DrawableKind::Composite { .. } => { if top_composite.is_none() { + // Empty stack -> current node is the highest Composite on its branch + // (otherwise this Composite would be pushed twice) ancestor_stack.push((node.uuid, Some(node.uuid))); } } @@ -357,16 +358,13 @@ impl InoxRendererExt for T { self.begin_composite_content(as_mask, components, render_ctx, id); for uuid in &render_ctx.zsorted_children_list { - if let Some(drawable_kind) = DrawableKind::new(*uuid, comps, false) - //.expect("All children in zsorted_children_list should be a Drawable."); - { - match drawable_kind { - DrawableKind::TexturedMesh(components) => { - self.draw_textured_mesh_content(as_mask, &components, comps.get(*uuid).unwrap(), *uuid) - } - // TODO: Composite inside composite not handled yet - DrawableKind::Composite { .. } => continue, //panic!("Composite inside Composite not allowed."), + let drawable_kind = DrawableKind::new(*uuid, comps, false) + .expect("All children in zsorted_children_list should be a Drawable."); + match drawable_kind { + DrawableKind::TexturedMesh(components) => { + self.draw_textured_mesh_content(as_mask, &components, comps.get(*uuid).unwrap(), *uuid) } + DrawableKind::Composite { .. } => continue, // Allow composite inside composite } } @@ -390,7 +388,6 @@ impl InoxRendererExt for T { .expect("RenderCtx of puppet must be initialized before calling draw().") .root_drawables_zsorted { - let node = puppet.nodes.get_node(*uuid); self.draw_drawable(false, &puppet.node_comps, *uuid); } }