From 2d63730bfa4a3372207486c2401b86966faa3090 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Tue, 11 Nov 2025 12:04:23 +0100 Subject: [PATCH] :sparkles: Improved performance in modifiers --- common/src/app/common/types/modifiers.cljc | 18 ++-- .../app/main/data/workspace/modifiers.cljs | 1 - render-wasm/src/math.rs | 7 ++ render-wasm/src/shapes.rs | 18 +++- render-wasm/src/shapes/modifiers.rs | 91 ++++++++++++------- .../src/shapes/modifiers/constraints.rs | 4 +- .../src/shapes/modifiers/flex_layout.rs | 2 +- .../src/shapes/modifiers/grid_layout.rs | 2 +- render-wasm/src/shapes/transform.rs | 27 +++++- 9 files changed, 113 insertions(+), 57 deletions(-) diff --git a/common/src/app/common/types/modifiers.cljc b/common/src/app/common/types/modifiers.cljc index ccf59454cd..2f79e6483f 100644 --- a/common/src/app/common/types/modifiers.cljc +++ b/common/src/app/common/types/modifiers.cljc @@ -767,6 +767,11 @@ :always (ctl/update-flex-child value)))) +(defn remove-children-set + [shapes children-to-remove] + (let [remove? (set children-to-remove)] + (d/removev remove? shapes))) + (defn apply-modifier [shape operation] (let [type (dm/get-prop operation :type)] @@ -793,7 +798,7 @@ :remove-children (let [value (dm/get-prop operation :value)] - (update shape :shapes remove-children value)) + (update shape :shapes remove-children-set value)) :scale-content (let [value (dm/get-prop operation :value)] @@ -810,11 +815,6 @@ (defn apply-structure-modifiers "Apply structure changes to a shape" [shape modifiers] - (let [remove-children - (fn [shapes children-to-remove] - (let [remove? (set children-to-remove)] - (d/removev remove? shapes)))] - - (as-> shape $ - (reduce apply-modifier $ (dm/get-prop modifiers :structure-parent)) - (reduce apply-modifier $ (dm/get-prop modifiers :structure-child))))) + (as-> shape $ + (reduce apply-modifier $ (dm/get-prop modifiers :structure-parent)) + (reduce apply-modifier $ (dm/get-prop modifiers :structure-child)))) diff --git a/frontend/src/app/main/data/workspace/modifiers.cljs b/frontend/src/app/main/data/workspace/modifiers.cljs index c12cc4b32e..a7db199f14 100644 --- a/frontend/src/app/main/data/workspace/modifiers.cljs +++ b/frontend/src/app/main/data/workspace/modifiers.cljs @@ -19,7 +19,6 @@ [app.common.types.component :as ctk] [app.common.types.container :as ctn] [app.common.types.modifiers :as ctm] - [app.common.types.modifiers :as ctm] [app.common.types.path :as path] [app.common.types.shape-tree :as ctst] [app.common.types.shape.attrs :refer [editable-attrs]] diff --git a/render-wasm/src/math.rs b/render-wasm/src/math.rs index 2aa121735c..9392cb00e8 100644 --- a/render-wasm/src/math.rs +++ b/render-wasm/src/math.rs @@ -51,6 +51,13 @@ pub fn identitish(m: &Matrix) -> bool { && is_close_to(m.skew_y(), 0.0) } +pub fn is_move_only_matrix(m: &Matrix) -> bool { + is_close_to(m.scale_x(), 1.0) + && is_close_to(m.scale_y(), 1.0) + && is_close_to(m.skew_x(), 0.0) + && is_close_to(m.skew_y(), 0.0) +} + #[derive(Debug, Copy, Clone, PartialEq)] pub struct Bounds { pub nw: Point, diff --git a/render-wasm/src/shapes.rs b/render-wasm/src/shapes.rs index d0792c9060..bdf1eae215 100644 --- a/render-wasm/src/shapes.rs +++ b/render-wasm/src/shapes.rs @@ -1,5 +1,7 @@ use skia_safe::{self as skia}; +use indexmap::IndexSet; + use crate::uuid::Uuid; use std::borrow::Cow; use std::cell::{OnceCell, RefCell}; @@ -342,9 +344,14 @@ impl Shape { matches!( self.shape_type, Type::Frame(Frame { - layout: Some(layouts::Layout::FlexLayout(_, FlexData { - direction: layouts::FlexDirection::RowReverse | layouts::FlexDirection::ColumnReverse, .. - })), + layout: Some(layouts::Layout::FlexLayout( + _, + FlexData { + direction: layouts::FlexDirection::RowReverse + | layouts::FlexDirection::ColumnReverse, + .. + } + )), .. }) ) @@ -1279,13 +1286,14 @@ impl Shape { } pub fn apply_structure(&mut self, structure: &Vec) { - let mut result: Vec = Vec::from_iter(self.children.iter().copied()); + let mut result = IndexSet::::from_iter(self.children.iter().copied()); let mut to_remove = HashSet::<&Uuid>::new(); for st in structure { match st.entry_type { StructureEntryType::AddChild => { - result.insert(st.index as usize, st.id); + let index = usize::min(result.len() - 1, st.index as usize); + result.shift_insert(index, st.id); } StructureEntryType::RemoveChild => { to_remove.insert(&st.id); diff --git a/render-wasm/src/shapes/modifiers.rs b/render-wasm/src/shapes/modifiers.rs index c5de2ae704..c7ca3e5484 100644 --- a/render-wasm/src/shapes/modifiers.rs +++ b/render-wasm/src/shapes/modifiers.rs @@ -6,11 +6,12 @@ mod flex_layout; pub mod common; pub mod grid_layout; -use crate::math::{self as math, bools, identitish, Bounds, Matrix, Point}; +use crate::math::{self as math, bools, identitish, is_close_to, Bounds, Matrix, Point}; use common::GetBounds; use crate::shapes::{ - ConstraintH, ConstraintV, Frame, Group, GrowType, Layout, Modifier, Shape, TransformEntry, Type, + ConstraintH, ConstraintV, Frame, Group, GrowType, Layout, Modifier, Shape, TransformEntry, + TransformEntrySource, Type, }; use crate::state::{ShapesPoolRef, State}; use crate::uuid::Uuid; @@ -75,7 +76,7 @@ fn propagate_children( child.ignore_constraints, ); - result.push_back(Modifier::transform(*child_id, transform)); + result.push_back(Modifier::transform_propagate(*child_id, transform)); } result @@ -182,33 +183,43 @@ fn propagate_transform( let mut transform = entry.transform; - // NOTA: No puedo utilizar un clone porque entonces estaríamos - // perdiendo la referencia al contenido del layout... - if let Type::Text(text_content) = &mut shape.shape_type.clone() { - if text_content.needs_update_layout() { - text_content.update_layout(shape.selrect); - } - match text_content.grow_type() { - GrowType::AutoHeight => { - let height = text_content.size.height; - let resize_transform = math::resize_matrix( - &shape_bounds_after, - &shape_bounds_after, - shape_bounds_after.width(), - height, - ); - shape_bounds_after = shape_bounds_after.transform(&resize_transform); - transform.post_concat(&resize_transform); + // Only check the text layout when the width/height changes + if !is_close_to(shape_bounds_before.width(), shape_bounds_after.width()) + || !is_close_to(shape_bounds_before.height(), shape_bounds_after.height()) + { + if let Type::Text(text_content) = &mut shape.shape_type.clone() { + match text_content.grow_type() { + GrowType::AutoHeight => { + if text_content.needs_update_layout() { + text_content.update_layout(shape.selrect); + } + let height = text_content.size.height; + let resize_transform = math::resize_matrix( + &shape_bounds_after, + &shape_bounds_after, + shape_bounds_after.width(), + height, + ); + shape_bounds_after = shape_bounds_after.transform(&resize_transform); + transform.post_concat(&resize_transform); + } + GrowType::AutoWidth => { + if text_content.needs_update_layout() { + text_content.update_layout(shape.selrect); + } + let width = text_content.width(); + let height = text_content.size.height; + let resize_transform = math::resize_matrix( + &shape_bounds_after, + &shape_bounds_after, + width, + height, + ); + shape_bounds_after = shape_bounds_after.transform(&resize_transform); + transform.post_concat(&resize_transform); + } + GrowType::Fixed => {} } - GrowType::AutoWidth => { - let width = text_content.width(); - let height = text_content.size.height; - let resize_transform = - math::resize_matrix(&shape_bounds_after, &shape_bounds_after, width, height); - shape_bounds_after = shape_bounds_after.transform(&resize_transform); - transform.post_concat(&resize_transform); - } - GrowType::Fixed => {} } } @@ -234,12 +245,19 @@ fn propagate_transform( shape_modif.post_concat(&transform); modifiers.insert(shape.id, shape_modif); - if shape.has_layout() { + let is_resize = !math::is_move_only_matrix(&transform); + let is_propagate = entry.source == TransformEntrySource::Propagate; + + // If this is a layout and we're only moving don't need to reflow + if shape.has_layout() && is_resize { entries.push_back(Modifier::reflow(shape.id)); } if let Some(parent) = shape.parent_id.and_then(|id| shapes.get(&id)) { - if parent.has_layout() || parent.is_group_like() { + // When the parent is either a group or a layout we only mark for reflow + // if the current transformation is not a move propagation. + // If it's a move propagation we don't need to reflow, the parent is already changed. + if (parent.has_layout() || parent.is_group_like()) && (is_resize || !is_propagate) { entries.push_back(Modifier::reflow(parent.id)); } } @@ -360,7 +378,14 @@ pub fn propagate_modifiers( ) -> Vec { let mut entries: VecDeque<_> = modifiers .iter() - .map(|entry| Modifier::Transform(entry.clone())) + .map(|entry| { + // If we receibe a identity matrix we force a reflow + if math::identitish(&entry.transform) { + Modifier::Reflow(entry.id) + } else { + Modifier::Transform(entry.clone()) + } + }) .collect(); let mut modifiers = HashMap::::new(); @@ -407,7 +432,7 @@ pub fn propagate_modifiers( modifiers .iter() - .map(|(key, val)| TransformEntry::new(*key, *val)) + .map(|(key, val)| TransformEntry::from_input(*key, *val)) .collect() } diff --git a/render-wasm/src/shapes/modifiers/constraints.rs b/render-wasm/src/shapes/modifiers/constraints.rs index 1a838c0a9a..190fd32734 100644 --- a/render-wasm/src/shapes/modifiers/constraints.rs +++ b/render-wasm/src/shapes/modifiers/constraints.rs @@ -1,4 +1,4 @@ -use crate::math::{Bounds, Matrix}; +use crate::math::{is_move_only_matrix, Bounds, Matrix}; use crate::shapes::{ConstraintH, ConstraintV}; pub fn calculate_resize( @@ -110,7 +110,7 @@ pub fn propagate_shape_constraints( // can propagate as is if (ignore_constrainst || constraint_h == ConstraintH::Scale && constraint_v == ConstraintV::Scale) - || transform.is_translate() + || is_move_only_matrix(&transform) { return transform; } diff --git a/render-wasm/src/shapes/modifiers/flex_layout.rs b/render-wasm/src/shapes/modifiers/flex_layout.rs index b3202c43f3..d3ed26b1db 100644 --- a/render-wasm/src/shapes/modifiers/flex_layout.rs +++ b/render-wasm/src/shapes/modifiers/flex_layout.rs @@ -623,7 +623,7 @@ pub fn reflow_flex_layout( transform.post_concat(&Matrix::translate(delta_v)); } - result.push_back(Modifier::transform(child.id, transform)); + result.push_back(Modifier::transform_propagate(child.id, transform)); shape_anchor = next_anchor( layout_data, diff --git a/render-wasm/src/shapes/modifiers/grid_layout.rs b/render-wasm/src/shapes/modifiers/grid_layout.rs index 2c3d19df1d..083d89d0f6 100644 --- a/render-wasm/src/shapes/modifiers/grid_layout.rs +++ b/render-wasm/src/shapes/modifiers/grid_layout.rs @@ -791,7 +791,7 @@ pub fn reflow_grid_layout( transform.post_concat(&Matrix::translate(delta_v)); } - result.push_back(Modifier::transform(child.id, transform)); + result.push_back(Modifier::transform_propagate(child.id, transform)); } if shape.is_layout_horizontal_auto() || shape.is_layout_vertical_auto() { diff --git a/render-wasm/src/shapes/transform.rs b/render-wasm/src/shapes/transform.rs index 7abf938b34..f3eba62d67 100644 --- a/render-wasm/src/shapes/transform.rs +++ b/render-wasm/src/shapes/transform.rs @@ -12,8 +12,8 @@ pub enum Modifier { } impl Modifier { - pub fn transform(id: Uuid, transform: Matrix) -> Self { - Modifier::Transform(TransformEntry::new(id, transform)) + pub fn transform_propagate(id: Uuid, transform: Matrix) -> Self { + Modifier::Transform(TransformEntry::from_propagate(id, transform)) } pub fn parent(id: Uuid, transform: Matrix) -> Self { Modifier::Transform(TransformEntry::parent(id, transform)) @@ -23,19 +23,35 @@ impl Modifier { } } +#[derive(PartialEq, Debug, Clone)] +pub enum TransformEntrySource { + Input, + Propagate, +} + #[derive(PartialEq, Debug, Clone)] #[repr(C)] pub struct TransformEntry { pub id: Uuid, pub transform: Matrix, + pub source: TransformEntrySource, pub propagate: bool, } impl TransformEntry { - pub fn new(id: Uuid, transform: Matrix) -> Self { + pub fn from_input(id: Uuid, transform: Matrix) -> Self { TransformEntry { id, transform, + source: TransformEntrySource::Input, + propagate: true, + } + } + pub fn from_propagate(id: Uuid, transform: Matrix) -> Self { + TransformEntry { + id, + transform, + source: TransformEntrySource::Propagate, propagate: true, } } @@ -43,6 +59,7 @@ impl TransformEntry { TransformEntry { id, transform, + source: TransformEntrySource::Propagate, propagate: false, } } @@ -70,7 +87,7 @@ impl SerializableResult for TransformEntry { 0.0, 1.0, ); - TransformEntry::new(id, transform) + TransformEntry::from_input(id, transform) } fn as_bytes(&self) -> Self::BytesType { @@ -176,7 +193,7 @@ mod tests { #[test] fn test_serialization() { - let entry = TransformEntry::new( + let entry = TransformEntry::from_input( Uuid::new_v4(), Matrix::new_all(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 0.0, 0.0, 1.0), );