Merge pull request #6125 from penpot/alotor-perf-flex-layout-2

✨ Improvements on flex layout positioning

frontend/src/app/render_wasm/api.cljs

@@ -640,8 +640,8 @@
         margin-bottom (or (dm/get-prop margins :m3) 0)
         margin-left (or (dm/get-prop margins :m4) 0)
 
-        h-sizing (-> (dm/get-prop shape :layout-item-h-sizing) (or :auto) translate-layout-sizing)
-        v-sizing (-> (dm/get-prop shape :layout-item-v-sizing) (or :auto) translate-layout-sizing)
+        h-sizing (-> (dm/get-prop shape :layout-item-h-sizing) (or :fix) translate-layout-sizing)
+        v-sizing (-> (dm/get-prop shape :layout-item-v-sizing) (or :fix) translate-layout-sizing)
         align-self (-> (dm/get-prop shape :layout-item-align-self) translate-align-self)
 
         max-h (dm/get-prop shape :layout-item-max-h)

render-wasm/src/math.rs

@@ -5,6 +5,8 @@ pub type Matrix = skia::Matrix;
 pub type Vector = skia::Vector;
 pub type Point = skia::Point;
 
+const THRESHOLD: f32 = 0.001;
+
 pub trait VectorExt {
     fn new_points(a: &Point, b: &Point) -> Vector;
 }
@@ -16,7 +18,20 @@ impl VectorExt for Vector {
     }
 }
 
-#[derive(Debug, Clone, PartialEq)]
+pub fn is_close_to(current: f32, value: f32) -> bool {
+    (current - value).abs() <= THRESHOLD
+}
+
+pub fn identitish(m: Matrix) -> bool {
+    is_close_to(m.scale_x(), 1.0)
+        && is_close_to(m.scale_y(), 1.0)
+        && is_close_to(m.translate_x(), 0.0)
+        && is_close_to(m.translate_y(), 0.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,
     pub ne: Point,
@@ -95,9 +110,9 @@ impl Bounds {
         self.sw = mtx.map_point(self.sw);
     }
 
-    // pub fn box_bounds(&self, other: &Self) -> Option<Self> {
-    //     self.from_points(other.points())
-    // }
+    pub fn box_bounds(&self, other: &Self) -> Option<Self> {
+        self.from_points(other.points())
+    }
 
     pub fn from_points(&self, points: Vec<Point>) -> Option<Self> {
         let hv = self.horizontal_vec();

render-wasm/src/render.rs

@@ -690,7 +690,19 @@ impl RenderState {
                         if element.is_recursive() {
                             let children_clip_bounds = node_render_state
                                 .get_children_clip_bounds(element, modifiers.get(&element.id));
-                            for child_id in element.children_ids().iter().rev() {
+
+                            let mut children_ids = element.children_ids();
+
+                            // Z-index ordering on Layouts
+                            if element.has_layout() {
+                                children_ids.sort_by(|id1, id2| {
+                                    let z1 = tree.get(id1).map_or_else(|| 0, |s| s.z_index());
+                                    let z2 = tree.get(id2).map_or_else(|| 0, |s| s.z_index());
+                                    z1.cmp(&z2)
+                                });
+                            }
+
+                            for child_id in children_ids.iter().rev() {
                                 self.pending_nodes.push(NodeRenderState {
                                     id: *child_id,
                                     visited_children: false,

render-wasm/src/shapes.rs

@@ -722,6 +722,20 @@ impl Shape {
         }
     }
 
+    pub fn is_absolute(&self) -> bool {
+        match &self.layout_item {
+            Some(LayoutItem { is_absolute, .. }) => *is_absolute,
+            _ => false,
+        }
+    }
+
+    pub fn z_index(&self) -> i32 {
+        match &self.layout_item {
+            Some(LayoutItem { z_index, .. }) => *z_index,
+            _ => 0,
+        }
+    }
+
     pub fn is_layout_vertical_auto(&self) -> bool {
         match &self.layout_item {
             Some(LayoutItem { v_sizing, .. }) => v_sizing == &Sizing::Auto,

render-wasm/src/shapes/modifiers.rs

@@ -1,5 +1,4 @@
 use std::collections::{HashMap, HashSet, VecDeque};
-
 mod common;
 mod constraints;
 mod flex_layout;
@@ -9,7 +8,7 @@ use uuid::Uuid;
 
 use common::GetBounds;
 
-use crate::math::{Bounds, Matrix, Point};
+use crate::math::{identitish, Bounds, Matrix, Point};
 use crate::shapes::{
     ConstraintH, ConstraintV, Frame, Group, Layout, Modifier, Shape, TransformEntry, Type,
 };
@@ -23,7 +22,7 @@ fn propagate_children(
     transform: Matrix,
     bounds: &HashMap<Uuid, Bounds>,
 ) -> VecDeque<Modifier> {
-    if shape.children.len() == 0 {
+    if shape.children.len() == 0 || identitish(transform) {
         return VecDeque::new();
     }
 
@@ -39,7 +38,13 @@ fn propagate_children(
         let constraint_h = match &shape.shape_type {
             Type::Frame(Frame {
                 layout: Some(_), ..
-            }) => ConstraintH::Left,
+            }) => {
+                if child.is_absolute() {
+                    child.constraint_h(ConstraintH::Left)
+                } else {
+                    ConstraintH::Left
+                }
+            }
             Type::Frame(_) => child.constraint_h(ConstraintH::Left),
             _ => child.constraint_h(ConstraintH::Scale),
         };
@@ -47,7 +52,13 @@ fn propagate_children(
         let constraint_v = match &shape.shape_type {
             Type::Frame(Frame {
                 layout: Some(_), ..
-            }) => ConstraintV::Top,
+            }) => {
+                if child.is_absolute() {
+                    child.constraint_v(ConstraintV::Top)
+                } else {
+                    ConstraintV::Top
+                }
+            }
             Type::Frame(_) => child.constraint_v(ConstraintV::Top),
             _ => child.constraint_v(ConstraintV::Scale),
         };
@@ -108,7 +119,6 @@ pub fn propagate_modifiers(state: &State, modifiers: Vec<TransformEntry>) -> Vec
         while let Some(modifier) = entries.pop_front() {
             match modifier {
                 Modifier::Transform(entry) => {
-                    // println!("Transform {}", entry.id);
                     let Some(shape) = state.shapes.get(&entry.id) else {
                         continue;
                     };
@@ -154,8 +164,22 @@ pub fn propagate_modifiers(state: &State, modifiers: Vec<TransformEntry>) -> Vec
                             layout: Some(_), ..
                         }) => {
                             if !reflown.contains(&id) {
-                                layout_reflows.push(id);
-                                reflown.insert(id);
+                                let mut skip_reflow = false;
+                                if shape.is_layout_horizontal_fill()
+                                    || shape.is_layout_vertical_fill()
+                                {
+                                    if let Some(parent_id) = shape.parent_id {
+                                        if !reflown.contains(&parent_id) {
+                                            // If this is a fill layout but the parent has not been reflown yet
+                                            // we wait for the next iteration for reflow
+                                            skip_reflow = true;
+                                        }
+                                    }
+                                }
+
+                                if !skip_reflow {
+                                    layout_reflows.push(id);
+                                }
                             }
                         }
                         Type::Group(Group { masked: true }) => {
@@ -196,6 +220,10 @@ pub fn propagate_modifiers(state: &State, modifiers: Vec<TransformEntry>) -> Vec
         }
 
         for id in layout_reflows.iter() {
+            if reflown.contains(&id) {
+                continue;
+            }
+
             let Some(shape) = state.shapes.get(&id) else {
                 continue;
             };
@@ -203,10 +231,14 @@ pub fn propagate_modifiers(state: &State, modifiers: Vec<TransformEntry>) -> Vec
             let Type::Frame(frame_data) = &shape.shape_type else {
                 continue;
             };
-
             if let Some(Layout::FlexLayout(layout_data, flex_data)) = &frame_data.layout {
-                let mut children =
-                    flex_layout::reflow_flex_layout(shape, layout_data, flex_data, shapes, &bounds);
+                let mut children = flex_layout::reflow_flex_layout(
+                    shape,
+                    layout_data,
+                    flex_data,
+                    shapes,
+                    &mut bounds,
+                );
                 entries.append(&mut children);
             }
 
@@ -215,6 +247,7 @@ pub fn propagate_modifiers(state: &State, modifiers: Vec<TransformEntry>) -> Vec
                     grid_layout::reflow_grid_layout(shape, layout_data, grid_data, shapes, &bounds);
                 entries.append(&mut children);
             }
+            reflown.insert(*id);
         }
         layout_reflows = Vec::new();
     }

render-wasm/src/shapes/modifiers/flex_layout.rs

@@ -116,8 +116,8 @@ struct ChildAxis {
     max_across_size: f32,
     is_fill_main: bool,
     is_fill_across: bool,
-    is_absolute: bool,
     z_index: i32,
+    bounds: Bounds,
 }
 
 impl ChildAxis {
@@ -139,8 +139,8 @@ impl ChildAxis {
                 max_across_size: layout_item.and_then(|i| i.max_h).unwrap_or(MAX_SIZE),
                 is_fill_main: child.is_layout_horizontal_fill(),
                 is_fill_across: child.is_layout_vertical_fill(),
-                is_absolute: layout_item.map(|i| i.is_absolute).unwrap_or(false),
                 z_index: layout_item.map(|i| i.z_index).unwrap_or(0),
+                bounds: child_bounds.clone(),
             }
         } else {
             Self {
@@ -157,8 +157,8 @@ impl ChildAxis {
                 max_main_size: layout_item.and_then(|i| i.max_h).unwrap_or(MAX_SIZE),
                 is_fill_main: child.is_layout_vertical_fill(),
                 is_fill_across: child.is_layout_horizontal_fill(),
-                is_absolute: layout_item.map(|i| i.is_absolute).unwrap_or(false),
                 z_index: layout_item.map(|i| i.z_index).unwrap_or(0),
+                bounds: child_bounds.clone(),
             }
         };
 
@@ -174,6 +174,7 @@ impl ChildAxis {
 
 fn initialize_tracks(
     shape: &Shape,
+    layout_bounds: &Bounds,
     layout_axis: &LayoutAxis,
     layout_data: &LayoutData,
     flex_data: &FlexData,
@@ -194,19 +195,32 @@ fn initialize_tracks(
             continue;
         };
 
-        let child_bounds = bounds.find(child);
+        if child.is_absolute() || child.hidden() {
+            continue;
+        }
+
+        let default_bounds = bounds.find(child);
+        let child_bounds = layout_bounds
+            .box_bounds(&default_bounds)
+            .unwrap_or(default_bounds);
+
         let child_axis = ChildAxis::new(child, &child_bounds, layout_data);
 
-        let child_main_size = if child_axis.is_fill_main {
-            child_axis.min_main_size
-        } else {
-            child_axis.main_size
-        };
-        let child_across_size = if child_axis.is_fill_across {
-            child_axis.min_across_size
-        } else {
-            child_axis.across_size
-        };
+        let child_main_size = child_axis.margin_main_start
+            + child_axis.margin_main_end
+            + if child_axis.is_fill_main {
+                child_axis.min_main_size
+            } else {
+                child_axis.main_size
+            };
+
+        let child_across_size = child_axis.margin_across_start
+            + child_axis.margin_across_end
+            + if child_axis.is_fill_across {
+                child_axis.min_across_size
+            } else {
+                child_axis.across_size
+            };
 
         let child_max_across_size = if child_axis.is_fill_across {
             child_axis.max_across_size
@@ -271,6 +285,7 @@ fn distribute_fill_main_space(layout_axis: &LayoutAxis, tracks: &mut Vec<TrackDa
                     f32::min(child.max_main_size, child.main_size + current) - child.main_size;
                 child.main_size = child.main_size + delta;
                 left_space = left_space - delta;
+                track.main_size = track.main_size + delta;
 
                 if (child.main_size - child.max_main_size).abs() < MIN_SIZE {
                     to_resize_children.remove(i);
@@ -315,9 +330,10 @@ fn distribute_fill_across_space(layout_axis: &LayoutAxis, tracks: &mut Vec<Track
 
         for child in track.shapes.iter_mut() {
             if child.is_fill_across {
-                child.across_size = track
-                    .across_size
-                    .clamp(child.min_across_size, child.max_across_size);
+                let mut size =
+                    track.across_size - child.margin_across_start - child.margin_across_end;
+                size = size.clamp(child.min_across_size, child.max_across_size);
+                child.across_size = size;
             }
         }
     }
@@ -446,7 +462,15 @@ fn calculate_track_data(
     bounds: &HashMap<Uuid, Bounds>,
 ) -> Vec<TrackData> {
     let layout_axis = LayoutAxis::new(shape, layout_bounds, layout_data, flex_data);
-    let mut tracks = initialize_tracks(shape, &layout_axis, layout_data, flex_data, shapes, bounds);
+    let mut tracks = initialize_tracks(
+        shape,
+        layout_bounds,
+        &layout_axis,
+        layout_data,
+        flex_data,
+        shapes,
+        bounds,
+    );
 
     if !layout_axis.is_auto_main {
         distribute_fill_main_space(&layout_axis, &mut tracks);
@@ -510,24 +534,26 @@ fn next_anchor(
     prev_anchor: Point,
     total_shapes_size: f32,
 ) -> Point {
-    let delta = match layout_data.justify_content {
-        JustifyContent::SpaceBetween => {
-            let effective_gap =
-                (layout_axis.main_space() - total_shapes_size) / (track.shapes.len() - 1) as f32;
-            child_axis.main_size + f32::max(layout_axis.gap_main, effective_gap)
-        }
-        JustifyContent::SpaceAround => {
-            let effective_gap =
-                (layout_axis.main_space() - total_shapes_size) / (track.shapes.len()) as f32;
-            child_axis.main_size + f32::max(layout_axis.gap_main, effective_gap)
-        }
-        JustifyContent::SpaceEvenly => {
-            let effective_gap =
-                (layout_axis.main_space() - total_shapes_size) / (track.shapes.len() + 1) as f32;
-            child_axis.main_size + f32::max(layout_axis.gap_main, effective_gap)
-        }
-        _ => child_axis.main_size + layout_axis.gap_main,
-    };
+    let delta = child_axis.margin_main_start
+        + child_axis.margin_main_end
+        + match layout_data.justify_content {
+            JustifyContent::SpaceBetween => {
+                let effective_gap = (layout_axis.main_space() - total_shapes_size)
+                    / (track.shapes.len() - 1) as f32;
+                child_axis.main_size + f32::max(layout_axis.gap_main, effective_gap)
+            }
+            JustifyContent::SpaceAround => {
+                let effective_gap =
+                    (layout_axis.main_space() - total_shapes_size) / (track.shapes.len()) as f32;
+                child_axis.main_size + f32::max(layout_axis.gap_main, effective_gap)
+            }
+            JustifyContent::SpaceEvenly => {
+                let effective_gap = (layout_axis.main_space() - total_shapes_size)
+                    / (track.shapes.len() + 1) as f32;
+                child_axis.main_size + f32::max(layout_axis.gap_main, effective_gap)
+            }
+            _ => child_axis.main_size + layout_axis.gap_main,
+        };
     prev_anchor + layout_axis.main_v * delta
 }
 
@@ -539,22 +565,23 @@ fn child_position(
     child_axis: &ChildAxis,
     track: &TrackData,
 ) -> Point {
-    let delta = match child.layout_item {
-        Some(LayoutItem {
-            align_self: Some(align_self),
-            ..
-        }) => match align_self {
-            AlignSelf::Center => (track.across_size - child_axis.across_size) / 2.0,
-            AlignSelf::End => track.across_size - child_axis.across_size,
-            _ => 0.0,
-        },
-        _ => match layout_data.align_items {
-            AlignItems::Center => (track.across_size - child_axis.across_size) / 2.0,
-            AlignItems::End => track.across_size - child_axis.across_size,
-            _ => 0.0,
-        },
-    };
-    shape_anchor + layout_axis.across_v * delta
+    let delta = child_axis.margin_across_start
+        + match child.layout_item {
+            Some(LayoutItem {
+                align_self: Some(align_self),
+                ..
+            }) => match align_self {
+                AlignSelf::Center => (track.across_size - child_axis.across_size) / 2.0,
+                AlignSelf::End => track.across_size - child_axis.across_size,
+                _ => 0.0,
+            },
+            _ => match layout_data.align_items {
+                AlignItems::Center => (track.across_size - child_axis.across_size) / 2.0,
+                AlignItems::End => track.across_size - child_axis.across_size,
+                _ => 0.0,
+            },
+        };
+    shape_anchor + layout_axis.main_v * child_axis.margin_main_start + layout_axis.across_v * delta
 }
 
 pub fn reflow_flex_layout(
@@ -562,7 +589,7 @@ pub fn reflow_flex_layout(
     layout_data: &LayoutData,
     flex_data: &FlexData,
     shapes: &HashMap<Uuid, Shape>,
-    bounds: &HashMap<Uuid, Bounds>,
+    bounds: &mut HashMap<Uuid, Bounds>,
 ) -> VecDeque<Modifier> {
     let mut result = VecDeque::new();
     let layout_bounds = &bounds.find(&shape);
@@ -587,7 +614,7 @@ pub fn reflow_flex_layout(
                 child_axis,
                 track,
             );
-            let child_bounds = bounds.find(child);
+            let child_bounds = &child_axis.bounds;
             let delta_v = Vector::new_points(&child_bounds.nw, &position);
 
             let (new_width, new_height) = if layout_data.is_row() {
@@ -695,7 +722,9 @@ pub fn reflow_flex_layout(
         scale.pre_translate(-origin);
         scale.pre_concat(&parent_transform_inv);
 
+        let layout_bounds_after = layout_bounds.transform(&scale);
         result.push_back(Modifier::parent(shape.id, scale));
+        bounds.insert(shape.id, layout_bounds_after);
     }
     result
 }