Merge pull request #8299 from penpot/elenatorro-13242-review-performance

🔧 Improve layout performance

render-wasm/src/main.rs

@@ -301,11 +301,7 @@ pub extern "C" fn set_view_end() {
         #[cfg(feature = "profile-macros")]
         {
             let total_time = performance::get_time() - unsafe { VIEW_INTERACTION_START };
-            performance::console_log!(
+            performance::console_log!("[PERF] view_interaction: {}ms", total_time);
-                "[PERF] view_interaction (zoom_changed={}): {}ms",
-                zoom_changed,
-                total_time
-            );
         }
     });
 }

render-wasm/src/render.rs

@@ -33,8 +33,9 @@ use crate::wapi;
 pub use fonts::*;
 pub use images::*;
 
-// This is the extra are used for tile rendering.
+// This is the extra area used for tile rendering (tiles beyond viewport).
-const VIEWPORT_INTEREST_AREA_THRESHOLD: i32 = 2;
+// Higher values pre-render more tiles, reducing empty squares during pan but using more memory.
+const VIEWPORT_INTEREST_AREA_THRESHOLD: i32 = 3;
 const MAX_BLOCKING_TIME_MS: i32 = 32;
 const NODE_BATCH_THRESHOLD: i32 = 3;
 
@@ -2097,8 +2098,13 @@ impl RenderState {
                     }
                 } else {
                     performance::begin_measure!("render_shape_tree::uncached");
+                    // Only allow stopping (yielding) if the current tile is NOT visible.
+                    // This ensures all visible tiles render synchronously before showing,
+                    // eliminating empty squares during zoom. Interest-area tiles can still yield.
+                    let tile_is_visible = self.tile_viewbox.is_visible(&current_tile);
+                    let can_stop = allow_stop && !tile_is_visible;
                     let (is_empty, early_return) =
-                        self.render_shape_tree_partial_uncached(tree, timestamp, allow_stop)?;
+                        self.render_shape_tree_partial_uncached(tree, timestamp, can_stop)?;
 
                     if early_return {
                         return Ok(());
@@ -2223,17 +2229,20 @@ impl RenderState {
      * Given a shape, check the indexes and update it's location in the tile set
      * returns the tiles that have changed in the process.
      */
-    pub fn update_shape_tiles(&mut self, shape: &Shape, tree: ShapesPoolRef) -> Vec<tiles::Tile> {
+    pub fn update_shape_tiles(
+        &mut self,
+        shape: &Shape,
+        tree: ShapesPoolRef,
+    ) -> HashSet<tiles::Tile> {
         let TileRect(rsx, rsy, rex, rey) = self.get_tiles_for_shape(shape, tree);
 
-        let old_tiles = self
+        // Collect old tiles to avoid borrow conflict with remove_shape_at
+        let old_tiles: Vec<_> = self
             .tiles
             .get_tiles_of(shape.id)
-            .map_or(Vec::new(), |tiles| tiles.iter().copied().collect());
+            .map_or(Vec::new(), |t| t.iter().copied().collect());
 
-        let new_tiles = (rsx..=rex).flat_map(|x| (rsy..=rey).map(move |y| tiles::Tile::from(x, y)));
+        let mut result = HashSet::<tiles::Tile>::with_capacity(old_tiles.len());
-
-        let mut result = HashSet::<tiles::Tile>::new();
 
         // First, remove the shape from all tiles where it was previously located
         for tile in old_tiles {
@@ -2242,12 +2251,66 @@ impl RenderState {
         }
 
         // Then, add the shape to the new tiles
-        for tile in new_tiles {
+        for tile in (rsx..=rex).flat_map(|x| (rsy..=rey).map(move |y| tiles::Tile::from(x, y))) {
             self.tiles.add_shape_at(tile, shape.id);
             result.insert(tile);
         }
 
-        result.iter().copied().collect()
+        result
+    }
+
+    /*
+     * Incremental version of update_shape_tiles for pan/zoom operations.
+     * Updates the tile index and returns ONLY tiles that need cache invalidation.
+     *
+     * During pan operations, shapes don't move in world coordinates. The interest
+     * area (viewport) moves, which changes which tiles we track in the index, but
+     * tiles that were already cached don't need re-rendering just because the
+     * viewport moved.
+     *
+     * This function:
+     * 1. Updates the tile index (adds/removes shapes from tiles based on interest area)
+     * 2. Returns empty vec for cache invalidation (pan doesn't change tile content)
+     *
+     * Tile cache invalidation only happens when shapes actually move or change,
+     * which is handled by rebuild_touched_tiles, not during pan/zoom.
+     */
+    pub fn update_shape_tiles_incremental(
+        &mut self,
+        shape: &Shape,
+        tree: ShapesPoolRef,
+    ) -> Vec<tiles::Tile> {
+        let TileRect(rsx, rsy, rex, rey) = self.get_tiles_for_shape(shape, tree);
+
+        let old_tiles: HashSet<tiles::Tile> = self
+            .tiles
+            .get_tiles_of(shape.id)
+            .map_or(HashSet::new(), |tiles| tiles.iter().copied().collect());
+
+        let new_tiles: HashSet<tiles::Tile> = (rsx..=rex)
+            .flat_map(|x| (rsy..=rey).map(move |y| tiles::Tile::from(x, y)))
+            .collect();
+
+        // Tiles where shape is being removed from index (left interest area)
+        let removed: Vec<_> = old_tiles.difference(&new_tiles).copied().collect();
+        // Tiles where shape is being added to index (entered interest area)
+        let added: Vec<_> = new_tiles.difference(&old_tiles).copied().collect();
+
+        // Update the index: remove from old tiles
+        for tile in &removed {
+            self.tiles.remove_shape_at(*tile, shape.id);
+        }
+
+        // Update the index: add to new tiles
+        for tile in &added {
+            self.tiles.add_shape_at(*tile, shape.id);
+        }
+
+        // Don't invalidate cache for pan/zoom - the tile content hasn't changed,
+        // only the interest area moved. Tiles that were cached are still valid.
+        // New tiles that entered the interest area will be rendered fresh since
+        // they weren't in the cache anyway.
+        Vec::new()
     }
 
     /*
@@ -2273,12 +2336,22 @@ impl RenderState {
     pub fn rebuild_tiles_shallow(&mut self, tree: ShapesPoolRef) {
         performance::begin_measure!("rebuild_tiles_shallow");
 
-        let mut all_tiles = HashSet::<tiles::Tile>::new();
+        // Check if zoom changed - if so, we need full cache invalidation
+        // because tiles are rendered at specific zoom levels
+        let zoom_changed = self.zoom_changed();
+
+        let mut tiles_to_invalidate = HashSet::<tiles::Tile>::new();
         let mut nodes = vec![Uuid::nil()];
         while let Some(shape_id) = nodes.pop() {
             if let Some(shape) = tree.get(&shape_id) {
                 if shape_id != Uuid::nil() {
-                    all_tiles.extend(self.update_shape_tiles(shape, tree));
+                    if zoom_changed {
+                        // Zoom changed: use full update that tracks all affected tiles
+                        tiles_to_invalidate.extend(self.update_shape_tiles(shape, tree));
+                    } else {
+                        // Pan only: use incremental update that preserves valid cached tiles
+                        self.update_shape_tiles_incremental(shape, tree);
+                    }
                 } else {
                     // We only need to rebuild tiles from the first level.
                     for child_id in shape.children_ids_iter(false) {
@@ -2288,11 +2361,11 @@ impl RenderState {
             }
         }
 
-        // Invalidate changed tiles - old content stays visible until new tiles render
+        if zoom_changed {
-        self.surfaces.remove_cached_tiles(self.background_color);
+            // Zoom changed: clear all cached tiles since they're at wrong zoom level
-        for tile in all_tiles {
+            self.surfaces.remove_cached_tiles(self.background_color);
-            self.remove_cached_tile(tile);
         }
+        // Pan only: no cache invalidation needed - tiles content unchanged
 
         performance::end_measure!("rebuild_tiles_shallow");
     }
@@ -2341,7 +2414,7 @@ impl RenderState {
 
         let mut all_tiles = HashSet::<tiles::Tile>::new();
 
-        let ids = self.touched_ids.clone();
+        let ids = std::mem::take(&mut self.touched_ids);
 
         for shape_id in ids.iter() {
             if let Some(shape) = tree.get(shape_id) {
@@ -2356,8 +2429,6 @@ impl RenderState {
             self.remove_cached_tile(tile);
         }
 
-        self.clean_touched();
-
         performance::end_measure!("rebuild_touched_tiles");
     }
 
@@ -2414,6 +2485,7 @@ impl RenderState {
         self.touched_ids.insert(uuid);
     }
 
+    #[allow(dead_code)]
     pub fn clean_touched(&mut self) {
         self.touched_ids.clear();
     }

render-wasm/src/shapes.rs

@@ -1119,6 +1119,28 @@ impl Shape {
         }
     }
 
+    /// Returns children in forward (non-reversed) order - useful for layout calculations
+    pub fn children_ids_iter_forward(
+        &self,
+        include_hidden: bool,
+    ) -> Box<dyn Iterator<Item = &Uuid> + '_> {
+        if include_hidden {
+            return Box::new(self.children.iter());
+        }
+
+        if let Type::Bool(_) = self.shape_type {
+            Box::new([].iter())
+        } else if let Type::Group(group) = self.shape_type {
+            if group.masked {
+                Box::new(self.children.iter().skip(1))
+            } else {
+                Box::new(self.children.iter())
+            }
+        } else {
+            Box::new(self.children.iter())
+        }
+    }
+
     pub fn all_children(
         &self,
         shapes: ShapesPoolRef,

render-wasm/src/shapes/modifiers.rs

@@ -300,7 +300,20 @@ fn propagate_reflow(
         Type::Frame(Frame {
             layout: Some(_), ..
         }) => {
-            layout_reflows.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 parent_id != Uuid::nil() && !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.insert(*id);
+            }
         }
         Type::Group(Group { masked: true }) => {
             let children_ids = shape.children_ids(true);
@@ -417,28 +430,26 @@ pub fn propagate_modifiers(
                 }
             }
         }
-
+        // We sort the reflows so they are processed deepest-first in the
-        let mut layout_reflows_vec: Vec<Uuid> = layout_reflows.into_iter().collect();
+        // tree structure. This way we can be sure that the children layouts
-
+        // are already reflowed before their parents.
-        // We sort the reflows so they are process first the ones that are more
+        let mut layout_reflows_vec: Vec<Uuid> =
-        // deep in the tree structure. This way we can be sure that the children layouts
+            std::mem::take(&mut layout_reflows).into_iter().collect();
-        // are already reflowed.
         layout_reflows_vec.sort_unstable_by(|id_a, id_b| {
             let da = shapes.get_depth(id_a);
             let db = shapes.get_depth(id_b);
             db.cmp(&da)
         });
 
-        let mut bounds_temp = bounds.clone();
+        for id in &layout_reflows_vec {
-        for id in layout_reflows_vec.iter() {
             if reflown.contains(id) {
                 continue;
             }
-            reflow_shape(id, state, &mut reflown, &mut entries, &mut bounds_temp);
+            reflow_shape(id, state, &mut reflown, &mut entries, &mut bounds);
         }
-        layout_reflows = HashSet::new();
     }
 
+    #[allow(dead_code)]
     modifiers
         .iter()
         .map(|(key, val)| TransformEntry::from_input(*key, *val))

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

@@ -184,15 +184,18 @@ fn initialize_tracks(
 ) -> Vec<TrackData> {
     let mut tracks = Vec::<TrackData>::new();
     let mut current_track = TrackData::default();
-    let mut children = shape.children_ids(true);
     let mut first = true;
 
-    if flex_data.is_reverse() {
+    // When is_reverse() is true, we need forward order (children_ids_iter_forward).
-        children.reverse();
+    // When is_reverse() is false, we need reversed order (children_ids_iter).
-    }
+    let children_iter: Box<dyn Iterator<Item = Uuid>> = if flex_data.is_reverse() {
+        Box::new(shape.children_ids_iter_forward(true).copied())
+    } else {
+        Box::new(shape.children_ids_iter(true).copied())
+    };
 
-    for child_id in children.iter() {
+    for child_id in children_iter {
-        let Some(child) = shapes.get(child_id) else {
+        let Some(child) = shapes.get(&child_id) else {
             continue;
         };
 
@@ -293,7 +296,7 @@ fn distribute_fill_main_space(layout_axis: &LayoutAxis, tracks: &mut [TrackData]
                 track.main_size += delta;
 
                 if (child.main_size - child.max_main_size).abs() < MIN_SIZE {
-                    to_resize_children.remove(i);
+                    to_resize_children.swap_remove(i);
                 }
             }
         }
@@ -330,7 +333,7 @@ fn distribute_fill_across_space(layout_axis: &LayoutAxis, tracks: &mut [TrackDat
             left_space -= delta;
 
             if (track.across_size - track.max_across_size).abs() < MIN_SIZE {
-                to_resize_tracks.remove(i);
+                to_resize_tracks.swap_remove(i);
             }
         }
     }

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

@@ -6,7 +6,7 @@ use crate::shapes::{
 };
 use crate::state::ShapesPoolRef;
 use crate::uuid::Uuid;
-use std::collections::{HashMap, VecDeque};
+use std::collections::{HashMap, HashSet, VecDeque};
 
 use super::common::GetBounds;
 
@@ -537,7 +537,7 @@ fn cell_bounds(
 
 pub fn create_cell_data<'a>(
     layout_bounds: &Bounds,
-    children: &[Uuid],
+    children: &HashSet<Uuid>,
     shapes: ShapesPoolRef<'a>,
     cells: &Vec<GridCell>,
     column_tracks: &[TrackData],
@@ -614,7 +614,7 @@ pub fn grid_cell_data<'a>(
 
     let bounds = &mut HashMap::<Uuid, Bounds>::new();
     let layout_bounds = shape.bounds();
-    let children = shape.children_ids(false);
+    let children: HashSet<Uuid> = shape.children_ids_iter(false).copied().collect();
 
     let column_tracks = calculate_tracks(
         true,
@@ -707,7 +707,7 @@ pub fn reflow_grid_layout(
 ) -> VecDeque<Modifier> {
     let mut result = VecDeque::new();
     let layout_bounds = bounds.find(shape);
-    let children = shape.children_ids(true);
+    let children: HashSet<Uuid> = shape.children_ids_iter(true).copied().collect();
 
     let column_tracks = calculate_tracks(
         true,

render-wasm/src/tiles.rs

@@ -209,16 +209,19 @@ impl PendingTiles {
         }
     }
 
-    pub fn update(&mut self, tile_viewbox: &TileViewbox, surfaces: &Surfaces) {
+    // Generate tiles in spiral order from center
-        self.list.clear();
+    fn generate_spiral(rect: &TileRect) -> Vec<Tile> {
-
+        let columns = rect.width();
-        let columns = tile_viewbox.interest_rect.width();
+        let rows = rect.height();
-        let rows = tile_viewbox.interest_rect.height();
-
         let total = columns * rows;
 
-        let mut cx = tile_viewbox.interest_rect.center_x();
+        if total <= 0 {
-        let mut cy = tile_viewbox.interest_rect.center_y();
+            return Vec::new();
+        }
+
+        let mut result = Vec::with_capacity(total as usize);
+        let mut cx = rect.center_x();
+        let mut cy = rect.center_y();
 
         let ratio = (columns as f32 / rows as f32).ceil() as i32;
 
@@ -228,7 +231,7 @@ impl PendingTiles {
         let mut direction = 0;
         let mut current = 0;
 
-        self.list.push(Tile(cx, cy));
+        result.push(Tile(cx, cy));
         while current < total {
             match direction {
                 0 => cx += 1,
@@ -238,7 +241,7 @@ impl PendingTiles {
                 _ => unreachable!("Invalid direction"),
             }
 
-            self.list.push(Tile(cx, cy));
+            result.push(Tile(cx, cy));
 
             direction_current += 1;
             let direction_total = if direction % 2 == 0 {
@@ -258,18 +261,44 @@ impl PendingTiles {
             }
             current += 1;
         }
-        self.list.reverse();
+        result.reverse();
+        result
+    }
 
-        // Create a new list where the cached tiles go first
+    pub fn update(&mut self, tile_viewbox: &TileViewbox, surfaces: &Surfaces) {
-        let iter1 = self
+        self.list.clear();
-            .list
+
-            .iter()
+        // Generate spiral for the interest area (viewport + margin)
-            .filter(|t| surfaces.has_cached_tile_surface(**t));
+        let spiral = Self::generate_spiral(&tile_viewbox.interest_rect);
-        let iter2 = self
+
-            .list
+        // Partition tiles into 4 priority groups (highest priority = processed last due to pop()):
-            .iter()
+        // 1. visible + cached (fastest - just blit from cache)
-            .filter(|t| !surfaces.has_cached_tile_surface(**t));
+        // 2. visible + uncached (user sees these, render next)
-        self.list = iter1.chain(iter2).copied().collect();
+        // 3. interest + cached (pre-rendered area, blit from cache)
+        // 4. interest + uncached (lowest priority - background pre-render)
+        let mut visible_cached = Vec::new();
+        let mut visible_uncached = Vec::new();
+        let mut interest_cached = Vec::new();
+        let mut interest_uncached = Vec::new();
+
+        for tile in spiral {
+            let is_visible = tile_viewbox.visible_rect.contains(&tile);
+            let is_cached = surfaces.has_cached_tile_surface(tile);
+
+            match (is_visible, is_cached) {
+                (true, true) => visible_cached.push(tile),
+                (true, false) => visible_uncached.push(tile),
+                (false, true) => interest_cached.push(tile),
+                (false, false) => interest_uncached.push(tile),
+            }
+        }
+
+        // Build final list with lowest priority first (they get popped last)
+        // Order: interest_uncached, interest_cached, visible_uncached, visible_cached
+        self.list.extend(interest_uncached);
+        self.list.extend(interest_cached);
+        self.list.extend(visible_uncached);
+        self.list.extend(visible_cached);
     }
 
     pub fn pop(&mut self) -> Option<Tile> {