mirror of
https://github.com/penpot/penpot.git
synced 2026-08-28 07:38:42 +00:00
🎉 Reduce heap allocations
This commit is contained in:
parent
25ee8dee78
commit
0956becd12
@ -121,9 +121,11 @@ pub extern "C" fn render(timestamp: i32) -> Result<()> {
|
|||||||
// modifier set, so the cost is paid once per rAF rather than
|
// modifier set, so the cost is paid once per rAF rather than
|
||||||
// once per pointer move.
|
// once per pointer move.
|
||||||
if get_render_state().options.is_interactive_transform() {
|
if get_render_state().options.is_interactive_transform() {
|
||||||
let ids = state.shapes.modifier_ids();
|
// Collect into an owned Vec to release the immutable borrow on
|
||||||
|
// `state.shapes` before the mutable `rebuild_modifier_tiles` call.
|
||||||
|
let ids = state.shapes.modifier_ids().to_vec();
|
||||||
if !ids.is_empty() {
|
if !ids.is_empty() {
|
||||||
state.rebuild_modifier_tiles(ids)?;
|
state.rebuild_modifier_tiles(&ids)?;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
state
|
state
|
||||||
@ -856,9 +858,8 @@ pub extern "C" fn set_modifiers() -> Result<()> {
|
|||||||
|
|
||||||
with_state!(state, {
|
with_state!(state, {
|
||||||
state.set_modifiers(modifiers);
|
state.set_modifiers(modifiers);
|
||||||
// TO CHECK
|
|
||||||
if !get_render_state().options.is_interactive_transform() {
|
if !get_render_state().options.is_interactive_transform() {
|
||||||
state.rebuild_modifier_tiles(ids)?;
|
state.rebuild_modifier_tiles(&ids)?;
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|||||||
@ -3040,24 +3040,27 @@ impl RenderState {
|
|||||||
// modified shapes (doc-space @ 100% zoom, scale=1.0). This is used as a cheap overlap
|
// modified shapes (doc-space @ 100% zoom, scale=1.0). This is used as a cheap overlap
|
||||||
// guard to decide when cached top-level crops are unsafe to reuse (something is moving
|
// guard to decide when cached top-level crops are unsafe to reuse (something is moving
|
||||||
// over/inside them), without doing expensive ancestor walks per node.
|
// over/inside them), without doing expensive ancestor walks per node.
|
||||||
let moved_bounds =
|
//
|
||||||
if self.options.is_interactive_transform() && !tree.modifier_ids().is_empty() {
|
// `modifier_ids` is pre-computed once here and reused throughout the loop to avoid
|
||||||
let mut acc: Option<Rect> = None;
|
// repeated allocations (formerly O(N_shapes) HashMap builds) per node.
|
||||||
for id in tree.modifier_ids().iter() {
|
let modifier_ids = tree.modifier_ids();
|
||||||
let Some(s) = tree.get(id) else { continue };
|
let moved_bounds = if self.options.is_interactive_transform() && !modifier_ids.is_empty() {
|
||||||
let r = self.get_cached_extrect(s, tree, 1.0);
|
let mut acc: Option<Rect> = None;
|
||||||
acc = Some(match acc {
|
for id in modifier_ids.iter() {
|
||||||
None => r,
|
let Some(s) = tree.get(id) else { continue };
|
||||||
Some(mut prev) => {
|
let r = self.get_cached_extrect(s, tree, 1.0);
|
||||||
prev.join(r);
|
acc = Some(match acc {
|
||||||
prev
|
None => r,
|
||||||
}
|
Some(mut prev) => {
|
||||||
});
|
prev.join(r);
|
||||||
}
|
prev
|
||||||
acc
|
}
|
||||||
} else {
|
});
|
||||||
None
|
}
|
||||||
};
|
acc
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
|
||||||
while let Some(node_render_state) = self.pending_nodes.pop() {
|
while let Some(node_render_state) = self.pending_nodes.pop() {
|
||||||
let node_id = node_render_state.id;
|
let node_id = node_render_state.id;
|
||||||
@ -3136,7 +3139,7 @@ impl RenderState {
|
|||||||
let use_cached = self.should_use_cached_top_level_during_interactive(
|
let use_cached = self.should_use_cached_top_level_during_interactive(
|
||||||
node_id,
|
node_id,
|
||||||
tree,
|
tree,
|
||||||
&tree.modifier_ids(),
|
modifier_ids,
|
||||||
moved_bounds,
|
moved_bounds,
|
||||||
);
|
);
|
||||||
|
|
||||||
@ -3857,7 +3860,7 @@ impl RenderState {
|
|||||||
pub fn rebuild_modifier_tiles(
|
pub fn rebuild_modifier_tiles(
|
||||||
&mut self,
|
&mut self,
|
||||||
tree: ShapesPoolMutRef<'_>,
|
tree: ShapesPoolMutRef<'_>,
|
||||||
ids: Vec<Uuid>,
|
ids: &[Uuid],
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
// During interactive transform, skip ancestor invalidation: walking up to the
|
// During interactive transform, skip ancestor invalidation: walking up to the
|
||||||
// parent frame evicts every tile the frame covers, including dense tiles with
|
// parent frame evicts every tile the frame covers, including dense tiles with
|
||||||
@ -3865,9 +3868,9 @@ impl RenderState {
|
|||||||
// `ShapesPool::set_modifiers`; the tile index is reconciled post-gesture by
|
// `ShapesPool::set_modifiers`; the tile index is reconciled post-gesture by
|
||||||
// the committing code path (rebuild_touched_tiles).
|
// the committing code path (rebuild_touched_tiles).
|
||||||
if self.options.is_interactive_transform() {
|
if self.options.is_interactive_transform() {
|
||||||
self.update_tiles_shapes(&ids, tree)?;
|
self.update_tiles_shapes(ids, tree)?;
|
||||||
} else {
|
} else {
|
||||||
let ancestors = all_with_ancestors(&ids, tree, false);
|
let ancestors = all_with_ancestors(ids, tree, false);
|
||||||
self.update_tiles_shapes(&ancestors, tree)?;
|
self.update_tiles_shapes(&ancestors, tree)?;
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|||||||
@ -225,8 +225,7 @@ impl State {
|
|||||||
let _ = get_render_state().render_preview(&self.shapes, timestamp);
|
let _ = get_render_state().render_preview(&self.shapes, timestamp);
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn rebuild_modifier_tiles(&mut self, ids: Vec<Uuid>) -> Result<()> {
|
pub fn rebuild_modifier_tiles(&mut self, ids: &[Uuid]) -> Result<()> {
|
||||||
// Index-based storage is safe
|
|
||||||
get_render_state().rebuild_modifier_tiles(&mut self.shapes, ids)
|
get_render_state().rebuild_modifier_tiles(&mut self.shapes, ids)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -49,6 +49,11 @@ pub struct ShapesPoolImpl {
|
|||||||
modified_shape_cache: HashMap<usize, OnceCell<Shape>>,
|
modified_shape_cache: HashMap<usize, OnceCell<Shape>>,
|
||||||
/// Transform modifiers, keyed by index
|
/// Transform modifiers, keyed by index
|
||||||
modifiers: HashMap<usize, skia::Matrix>,
|
modifiers: HashMap<usize, skia::Matrix>,
|
||||||
|
/// UUIDs of shapes that have an active transform modifier, kept in sync
|
||||||
|
/// with `modifiers`. Stored explicitly so that `modifier_ids()` is O(K)
|
||||||
|
/// (K = number of modified shapes) instead of O(N_shapes) — avoids
|
||||||
|
/// building a full reverse-index HashMap on every call.
|
||||||
|
modifier_uuids: Vec<Uuid>,
|
||||||
/// Structure entries, keyed by index
|
/// Structure entries, keyed by index
|
||||||
structure: HashMap<usize, Vec<StructureEntry>>,
|
structure: HashMap<usize, Vec<StructureEntry>>,
|
||||||
/// Scale content values, keyed by index
|
/// Scale content values, keyed by index
|
||||||
@ -69,6 +74,7 @@ impl ShapesPoolImpl {
|
|||||||
|
|
||||||
modified_shape_cache: HashMap::default(),
|
modified_shape_cache: HashMap::default(),
|
||||||
modifiers: HashMap::default(),
|
modifiers: HashMap::default(),
|
||||||
|
modifier_uuids: Vec::new(),
|
||||||
structure: HashMap::default(),
|
structure: HashMap::default(),
|
||||||
scale_content: HashMap::default(),
|
scale_content: HashMap::default(),
|
||||||
}
|
}
|
||||||
@ -238,7 +244,11 @@ impl ShapesPoolImpl {
|
|||||||
}
|
}
|
||||||
self.modifiers = modifiers_with_idx;
|
self.modifiers = modifiers_with_idx;
|
||||||
|
|
||||||
|
// Compute ancestors before consuming `ids` so we can move it into
|
||||||
|
// `modifier_uuids` without a clone.
|
||||||
let all_ids = shapes::all_with_ancestors(&ids, self, true);
|
let all_ids = shapes::all_with_ancestors(&ids, self, true);
|
||||||
|
// Keep modifier_uuids in sync so modifier_ids() is O(K) not O(N_shapes).
|
||||||
|
self.modifier_uuids = ids;
|
||||||
for uuid in all_ids {
|
for uuid in all_ids {
|
||||||
if let Some(idx) = self.uuid_to_idx.get(&uuid).copied() {
|
if let Some(idx) = self.uuid_to_idx.get(&uuid).copied() {
|
||||||
self.modified_shape_cache.insert(idx, OnceCell::new());
|
self.modified_shape_cache.insert(idx, OnceCell::new());
|
||||||
@ -300,19 +310,9 @@ impl ShapesPoolImpl {
|
|||||||
pub fn clean_all(&mut self) -> Vec<Uuid> {
|
pub fn clean_all(&mut self) -> Vec<Uuid> {
|
||||||
self.clean_shape_cache();
|
self.clean_shape_cache();
|
||||||
|
|
||||||
let modified_uuids: Vec<Uuid> = if self.modifiers.is_empty() {
|
// `modifier_uuids` is kept in sync with `modifiers` by `set_modifiers`,
|
||||||
Vec::new()
|
// so we can take it directly — no need to rebuild a reverse index.
|
||||||
} else {
|
let modified_uuids = std::mem::take(&mut self.modifier_uuids);
|
||||||
let mut idx_to_uuid: HashMap<usize, Uuid> =
|
|
||||||
HashMap::with_capacity(self.uuid_to_idx.len());
|
|
||||||
for (uuid, idx) in self.uuid_to_idx.iter() {
|
|
||||||
idx_to_uuid.insert(*idx, *uuid);
|
|
||||||
}
|
|
||||||
self.modifiers
|
|
||||||
.keys()
|
|
||||||
.filter_map(|idx| idx_to_uuid.get(idx).copied())
|
|
||||||
.collect()
|
|
||||||
};
|
|
||||||
|
|
||||||
self.modifiers = HashMap::default();
|
self.modifiers = HashMap::default();
|
||||||
self.structure = HashMap::default();
|
self.structure = HashMap::default();
|
||||||
@ -325,18 +325,12 @@ impl ShapesPoolImpl {
|
|||||||
/// Used by the throttled drag path so per-rAF tile invalidation can
|
/// Used by the throttled drag path so per-rAF tile invalidation can
|
||||||
/// be done once with the current modifier set instead of once per
|
/// be done once with the current modifier set instead of once per
|
||||||
/// pointer move.
|
/// pointer move.
|
||||||
pub fn modifier_ids(&self) -> Vec<Uuid> {
|
///
|
||||||
if self.modifiers.is_empty() {
|
/// Returns a reference to avoid allocation on every call — callers
|
||||||
return Vec::new();
|
/// inside hot render loops should hold this reference rather than
|
||||||
}
|
/// calling `modifier_ids()` repeatedly.
|
||||||
let mut idx_to_uuid: HashMap<usize, Uuid> = HashMap::with_capacity(self.uuid_to_idx.len());
|
pub fn modifier_ids(&self) -> &[Uuid] {
|
||||||
for (uuid, idx) in self.uuid_to_idx.iter() {
|
&self.modifier_uuids
|
||||||
idx_to_uuid.insert(*idx, *uuid);
|
|
||||||
}
|
|
||||||
self.modifiers
|
|
||||||
.keys()
|
|
||||||
.filter_map(|idx| idx_to_uuid.get(idx).copied())
|
|
||||||
.collect()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn subtree(&self, id: &Uuid) -> ShapesPoolImpl {
|
pub fn subtree(&self, id: &Uuid) -> ShapesPoolImpl {
|
||||||
@ -363,6 +357,7 @@ impl ShapesPoolImpl {
|
|||||||
uuid_to_idx,
|
uuid_to_idx,
|
||||||
modified_shape_cache: HashMap::default(),
|
modified_shape_cache: HashMap::default(),
|
||||||
modifiers: HashMap::default(),
|
modifiers: HashMap::default(),
|
||||||
|
modifier_uuids: Vec::new(),
|
||||||
structure: HashMap::default(),
|
structure: HashMap::default(),
|
||||||
scale_content: HashMap::default(),
|
scale_content: HashMap::default(),
|
||||||
}
|
}
|
||||||
@ -409,6 +404,7 @@ impl Clone for ShapesPoolImpl {
|
|||||||
// so it gets lazily rebuilt on demand rather than cloning OnceCell state.
|
// so it gets lazily rebuilt on demand rather than cloning OnceCell state.
|
||||||
modified_shape_cache: HashMap::default(),
|
modified_shape_cache: HashMap::default(),
|
||||||
modifiers: self.modifiers.clone(),
|
modifiers: self.modifiers.clone(),
|
||||||
|
modifier_uuids: self.modifier_uuids.clone(),
|
||||||
structure: self.structure.clone(),
|
structure: self.structure.clone(),
|
||||||
scale_content: self.scale_content.clone(),
|
scale_content: self.scale_content.clone(),
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user