Merge pull request #7664 from penpot/alotor-performance-improvements

 Improve boolean calculations
This commit is contained in:
Alejandro Alonso
2025-11-03 12:04:36 +01:00
committed by GitHub
6 changed files with 64 additions and 45 deletions

View File

@@ -420,7 +420,6 @@ pub fn bool_from_shapes(
current_path
}
#[allow(dead_code)]
pub fn update_bool_to_path(shape: &mut Shape, shapes: ShapesPoolRef) {
let children_ids = shape.children_ids(true);
@@ -431,8 +430,8 @@ pub fn update_bool_to_path(shape: &mut Shape, shapes: ShapesPoolRef) {
bool_data.path = bool_from_shapes(bool_data.bool_type, &children_ids, shapes);
}
#[allow(dead_code)]
// Debug utility for boolean shapes
#[allow(dead_code)]
pub fn debug_render_bool_paths(
render_state: &mut RenderState,
shape: &Shape,

View File

@@ -1366,7 +1366,7 @@ impl RenderState {
let children_clip_bounds =
node_render_state.get_children_clip_bounds(element, None);
let mut children_ids = element.children_ids(false);
let mut children_ids: Vec<_> = element.children_ids_iter(false).collect();
// Z-index ordering on Layouts
if element.has_layout() {
@@ -1379,7 +1379,7 @@ impl RenderState {
for child_id in children_ids.iter() {
self.pending_nodes.push(NodeRenderState {
id: *child_id,
id: **child_id,
visited_children: false,
clip_bounds: children_clip_bounds,
visited_mask: false,
@@ -1555,8 +1555,7 @@ impl RenderState {
self.update_tile_for(shape, tree);
} else {
// We only need to rebuild tiles from the first level.
let children = shape.children_ids(false);
for child_id in children.iter() {
for child_id in shape.children_ids_iter(false) {
nodes.push(*child_id);
}
}
@@ -1576,8 +1575,7 @@ impl RenderState {
self.update_tile_for(shape, tree);
}
let children = shape.children_ids(false);
for child_id in children.iter() {
for child_id in shape.children_ids_iter(false) {
nodes.push(*child_id);
}
}

View File

@@ -47,7 +47,6 @@ pub use svgraw::*;
pub use text::*;
pub use transform::*;
use crate::math::bools as math_bools;
use crate::math::{self, Bounds, Matrix, Point};
use indexmap::IndexSet;
@@ -852,8 +851,8 @@ impl Shape {
};
if include_children {
for child_id in self.children_ids(false) {
if let Some(child_shape) = shapes_pool.get(&child_id) {
for child_id in self.children_ids_iter(false) {
if let Some(child_shape) = shapes_pool.get(child_id) {
rect.join(child_shape.extrect(shapes_pool));
}
}
@@ -907,7 +906,6 @@ impl Shape {
self.children.first()
}
// TODO: Review this to use children_ids_iter instead
pub fn children_ids(&self, include_hidden: bool) -> IndexSet<Uuid> {
if include_hidden {
return self.children.clone().into_iter().rev().collect();
@@ -955,12 +953,9 @@ impl Shape {
include_hidden: bool,
include_self: bool,
) -> IndexSet<Uuid> {
let all_children = self
.children_ids(include_hidden)
.into_iter()
.flat_map(|id| {
let all_children = self.children_ids_iter(include_hidden).flat_map(|id| {
shapes
.get(&id)
.get(id)
.map(|s| s.all_children(shapes, include_hidden, true))
.unwrap_or_default()
});
@@ -972,6 +967,27 @@ impl Shape {
}
}
pub fn all_children_iter<'a>(
&'a self,
shapes: ShapesPoolRef<'a>,
include_hidden: bool,
include_self: bool,
) -> Box<dyn Iterator<Item = Uuid> + 'a> {
let all_children = self.children_ids_iter(include_hidden).flat_map(move |id| {
if let Some(shape) = shapes.get(id) {
shape.all_children_iter(shapes, include_hidden, true)
} else {
Box::new(std::iter::empty())
}
});
if include_self {
Box::new(once(self.id).chain(all_children))
} else {
Box::new(all_children)
}
}
pub fn get_matrix(&self) -> Matrix {
let mut matrix = Matrix::new_identity();
matrix.post_translate(self.left_top());
@@ -1191,7 +1207,6 @@ impl Shape {
pub fn transformed(
&self,
shapes_pool: ShapesPoolRef,
transform: Option<&Matrix>,
structure: Option<&Vec<StructureEntry>>,
) -> Self {
@@ -1202,9 +1217,7 @@ impl Shape {
if let Some(structure) = structure {
shape.to_mut().apply_structure(structure);
}
if self.is_bool() {
math_bools::update_bool_to_path(shape.to_mut(), shapes_pool)
}
shape.into_owned()
}

View File

@@ -24,15 +24,13 @@ fn propagate_children(
transform: Matrix,
bounds: &HashMap<Uuid, Bounds>,
) -> VecDeque<Modifier> {
let children_ids = shape.children_ids(true);
if children_ids.is_empty() || identitish(&transform) {
if identitish(&transform) {
return VecDeque::new();
}
let mut result = VecDeque::new();
for child_id in children_ids.iter() {
for child_id in shape.children_ids_iter(true) {
let Some(child) = shapes.get(child_id) else {
continue;
};

View File

@@ -174,10 +174,9 @@ impl ToPath for Shape {
fn to_path(&self, shapes: ShapesPoolRef) -> Path {
match &self.shape_type {
Type::Frame(ref frame) => {
let children = self.children_ids(true);
let mut result = Path::new(rect_segments(self, frame.corners));
for id in children {
let Some(shape) = shapes.get(&id) else {
for id in self.children_ids_iter(true) {
let Some(shape) = shapes.get(id) else {
continue;
};
result = join_paths(result, shape.to_path(shapes));
@@ -186,10 +185,9 @@ impl ToPath for Shape {
}
Type::Group(_) => {
let children = self.children_ids(true);
let mut result = Path::default();
for id in children {
let Some(shape) = shapes.get(&id) else {
for id in self.children_ids_iter(true) {
let Some(shape) = shapes.get(id) else {
continue;
};
result = join_paths(result, shape.to_path(shapes));

View File

@@ -11,6 +11,10 @@ use crate::skia;
use std::cell::OnceCell;
use crate::math;
use crate::math::bools as math_bools;
use crate::math::Matrix;
const SHAPES_POOL_ALLOC_MULTIPLIER: f32 = 1.3;
/// A pool allocator for `Shape` objects that attempts to minimize memory reallocations.
@@ -226,7 +230,7 @@ impl<'a> ShapesPoolImpl<'a> {
// Extend the lifetime of id to 'a - safe because it's the same Uuid stored in shapes[idx].id
let id_ref: &'a Uuid = &*(id as *const Uuid);
if self.to_update_bool(&*shape_ptr)
if (*shape_ptr).is_bool()
|| (*modifiers_ptr).contains_key(&id_ref)
|| (*structure_ptr).contains_key(&id_ref)
|| (*scale_content_ptr).contains_key(&id_ref)
@@ -234,10 +238,14 @@ impl<'a> ShapesPoolImpl<'a> {
if let Some(cell) = (*cache_ptr).get(&id_ref) {
Some(cell.get_or_init(|| {
let mut shape = (*shape_ptr).transformed(
self,
(*modifiers_ptr).get(&id_ref),
(*structure_ptr).get(&id_ref),
);
if self.to_update_bool(&shape) {
math_bools::update_bool_to_path(&mut shape, self);
}
if let Some(scale) = (*scale_content_ptr).get(&id_ref) {
shape.scale_content(*scale);
}
@@ -354,15 +362,12 @@ impl<'a> ShapesPoolImpl<'a> {
panic!("Subtree not found");
};
// TODO: Maybe create all_children_iter
let all_children = shape.all_children(self, true, true);
let mut shapes = vec![];
let mut idx = 0;
let mut shapes_uuid_to_idx = HashMap::default();
for id in all_children.iter() {
let Some(shape) = self.get(id) else {
for id in shape.all_children_iter(self, true, true) {
let Some(shape) = self.get(&id) else {
panic!("Not found");
};
shapes.push(shape.clone());
@@ -387,8 +392,16 @@ impl<'a> ShapesPoolImpl<'a> {
}
fn to_update_bool(&self, shape: &Shape) -> bool {
// TODO: Check if any of the children is in the modifiers with a
// different matrix than the current one.
shape.is_bool()
if !shape.is_bool() {
return false;
}
let default = &Matrix::default();
let parent_modifier = self.modifiers.get(&shape.id).unwrap_or(default);
// Returns true if the transform of any child is different to the parent's
shape.all_children_iter(self, true, false).any(|id| {
!math::is_close_matrix(parent_modifier, self.modifiers.get(&id).unwrap_or(default))
})
}
}