johanels/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-12-11 22:14:05 +01:00
Code Issues Actions 9 Packages Projects Releases Wiki Activity

Merge pull request #7067 from penpot/superalex-fix-frames-extrect-calculation

Browse Source 
🐛 Fix frames extrect calculation
This commit is contained in:
Aitor Moreno
2025-08-11 13:57:29 +02:00
committed by GitHub
parent 6176027263 98522a390e
commit 132069472c
6 changed files with 209 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
render-wasm/src/main.rs
View File

@@ -226,9 +226,9 @@ pub extern "C" fn use_shape(a: u32, b: u32, c: u32, d: u32) {
#[no_mangle]
pub extern "C" fn set_parent(a: u32, b: u32, c: u32, d: u32) {
with_current_shape_mut!(state, |shape: &mut Shape| {
with_state_mut!(state, {
let id = uuid_from_u32_quartet(a, b, c, d);
shape.set_parent(id);
state.set_parent_for_current_shape(id);
});
}

94
render-wasm/src/render.rs
View File

@@ -896,7 +896,7 @@ impl RenderState {
// If the shape is not in the tile set, then we update
// it.
if self.tiles.get_tiles_of(node_id).is_none() {
self.update_tile_for(element);
self.update_tile_for(element, tree, modifiers);
}
if visited_children {
@@ -916,12 +916,24 @@ impl RenderState {
transformed_element.to_mut().apply_transform(modifier);
}
let is_visible = transformed_element.extrect().intersects(self.render_area)
let is_visible = transformed_element
.extrect(tree, modifiers)
.intersects(self.render_area)
&& !transformed_element.hidden
&& !transformed_element.visually_insignificant(self.get_scale());
&& !transformed_element.visually_insignificant(
self.get_scale(),
tree,
modifiers,
);
if self.options.is_debug_visible() {
debug::render_debug_shape(self, &transformed_element, is_visible);
debug::render_debug_shape(
self,
&transformed_element,
is_visible,
tree,
modifiers,
);
}
if !is_visible {
@@ -1040,7 +1052,6 @@ impl RenderState {
}
}
// println!("clear current {:?}", self.current_tile);
self.surfaces
.canvas(SurfaceId::Current)
.clear(self.background_color);
@@ -1097,13 +1108,23 @@ impl RenderState {
Ok(())
}
pub fn get_tiles_for_shape(&mut self, shape: &Shape) -> TileRect {
pub fn get_tiles_for_shape(
&mut self,
shape: &Shape,
tree: &ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) -> TileRect {
let tile_size = tiles::get_tile_size(self.get_scale());
tiles::get_tiles_for_rect(shape.extrect(), tile_size)
tiles::get_tiles_for_rect(shape.extrect(tree, modifiers), tile_size)
}
pub fn update_tile_for(&mut self, shape: &Shape) {
let TileRect(rsx, rsy, rex, rey) = self.get_tiles_for_shape(shape);
pub fn update_tile_for(
&mut self,
shape: &Shape,
tree: &ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) {
let TileRect(rsx, rsy, rex, rey) = self.get_tiles_for_shape(shape, tree, modifiers);
let new_tiles: HashSet<tiles::Tile> = (rsx..=rex)
.flat_map(|x| (rsy..=rey).map(move |y| tiles::Tile(x, y)))
.collect();
@@ -1144,7 +1165,7 @@ impl RenderState {
if let Some(modifier) = modifiers.get(&shape_id) {
shape.to_mut().apply_transform(modifier);
}
self.update_tile_for(&shape);
self.update_tile_for(&shape, tree, modifiers);
} else {
// We only need to rebuild tiles from the first level.
let children = shape.modified_children_ids(structure.get(&shape.id), false);
@@ -1174,7 +1195,7 @@ impl RenderState {
if let Some(modifier) = modifiers.get(&shape_id) {
shape.to_mut().apply_transform(modifier);
}
self.update_tile_for(&shape);
self.update_tile_for(&shape, tree, modifiers);
}
let children = shape.modified_children_ids(structure.get(&shape.id), false);
@@ -1186,13 +1207,52 @@ impl RenderState {
performance::end_measure!("rebuild_tiles");
}
pub fn rebuild_modifier_tiles(&mut self, tree: &ShapesPool, modifiers: &HashMap<Uuid, Matrix>) {
for (uuid, matrix) in modifiers {
if let Some(shape) = tree.get(uuid) {
let mut shape: Cow<Shape> = Cow::Borrowed(shape);
shape.to_mut().apply_transform(matrix);
self.update_tile_for(&shape);
/// Processes all ancestors of a shape, invalidating their extended rectangles and updating their tiles
///
/// When a shape changes, all its ancestors need to have their extended rectangles recalculated
/// because they may contain the changed shape. This function:
/// 1. Invalidates the extrect cache for each ancestor
/// 2. Updates the tiles for each ancestor to ensure proper rendering
pub fn process_shape_ancestors(
&mut self,
shape: &Shape,
tree: &mut ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) {
for ancestor in shape.all_ancestors(tree, false) {
if let Some(ancestor) = tree.get_mut(&ancestor) {
ancestor.invalidate_extrect();
}
if let Some(ancestor) = tree.get(&ancestor) {
if !ancestor.id.is_nil() {
self.update_tile_for(ancestor, tree, modifiers);
}
}
}
}
/// Rebuilds tiles for shapes with modifiers and processes their ancestors
///
/// This function applies transformation modifiers to shapes and updates their tiles.
/// Additionally, it processes all ancestors of modified shapes to ensure their
/// extended rectangles are properly recalculated and their tiles are updated.
/// This is crucial for frames and groups that contain transformed children.
pub fn rebuild_modifier_tiles(
&mut self,
tree: &mut ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) {
for (uuid, matrix) in modifiers {
let mut shape = {
let Some(shape) = tree.get(uuid) else {
panic!("Invalid current shape")
};
shape.clone()
};
shape.apply_transform(matrix);
self.update_tile_for(&shape, tree, modifiers);
self.process_shape_ancestors(&shape, tree, modifiers);
}
}

16
render-wasm/src/render/debug.rs
View File

@@ -1,6 +1,12 @@
use crate::shapes::Shape;
use crate::state::ShapesPool;
use crate::uuid::Uuid;
use crate::math::Matrix;
use skia_safe::{self as skia, Rect};
use std::collections::HashMap;
use super::{tiles, RenderState, SurfaceId};
#[cfg(target_arch = "wasm32")]
@@ -58,7 +64,13 @@ pub fn render_wasm_label(render_state: &mut RenderState) {
canvas.draw_str(str, p, debug_font, &paint);
}
pub fn render_debug_shape(render_state: &mut RenderState, element: &Shape, intersected: bool) {
pub fn render_debug_shape(
render_state: &mut RenderState,
element: &Shape,
intersected: bool,
shapes_pool: &ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) {
let mut paint = skia::Paint::default();
paint.set_style(skia::PaintStyle::Stroke);
paint.set_color(if intersected {
@@ -68,7 +80,7 @@ pub fn render_debug_shape(render_state: &mut RenderState, element: &Shape, inter
});
paint.set_stroke_width(1.);
let rect = get_debug_rect(element.extrect());
let rect = get_debug_rect(element.extrect(shapes_pool, modifiers));
render_state
.surfaces
.canvas(SurfaceId::Debug)

86
render-wasm/src/shapes.rs
View File

@@ -2,6 +2,7 @@ use skia_safe::{self as skia};
use crate::render::BlendMode;
use crate::uuid::Uuid;
use std::borrow::Cow;
use std::cell::OnceCell;
use std::collections::{HashMap, HashSet};
use std::iter::once;
@@ -274,7 +275,7 @@ impl Shape {
result
}
fn invalidate_extrect(&mut self) {
pub fn invalidate_extrect(&mut self) {
self.extrect = OnceCell::new();
}
@@ -652,8 +653,13 @@ impl Shape {
self.selrect.width()
}
pub fn visually_insignificant(&self, scale: f32) -> bool {
let extrect = self.extrect();
pub fn visually_insignificant(
&self,
scale: f32,
shapes_pool: &ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) -> bool {
let extrect = self.extrect(shapes_pool, modifiers);
extrect.width() * scale < MIN_VISIBLE_SIZE && extrect.height() * scale < MIN_VISIBLE_SIZE
}
@@ -688,11 +694,21 @@ impl Shape {
self.selrect
}
pub fn extrect(&self) -> math::Rect {
*self.extrect.get_or_init(|| self.calculate_extrect())
pub fn extrect(
&self,
shapes_pool: &ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) -> math::Rect {
*self
.extrect
.get_or_init(|| self.calculate_extrect(shapes_pool, modifiers))
}
pub fn calculate_extrect(&self) -> math::Rect {
pub fn calculate_extrect(
&self,
shapes_pool: &ShapesPool,
modifiers: &HashMap<Uuid, Matrix>,
) -> math::Rect {
let mut max_stroke: f32 = 0.;
let is_open = if let Type::Path(p) = &self.shape_type {
p.is_open()
@@ -753,6 +769,26 @@ impl Shape {
rect.bottom += self.blur.value;
}
// For frames without clipping, extend the bounding rectangle to include all nested shapes
// This ensures that frames properly encompass their content when clip_content is false
if let Type::Frame(_) = &self.shape_type {
if !self.clip_content {
for child_id in self.children_ids(false) {
if let Some(child_shape) = shapes_pool.get(&child_id) {
// Create a copy of the child shape to apply any transformations
let mut transformed_element: Cow<Shape> = Cow::Borrowed(child_shape);
if let Some(modifier) = modifiers.get(&child_id) {
transformed_element.to_mut().apply_transform(modifier);
}
// Get the child's extended rectangle and join it with the frame's rectangle
let child_extrect = transformed_element.extrect(shapes_pool, modifiers);
rect.join(child_extrect);
}
}
}
}
rect
}
@@ -810,6 +846,44 @@ impl Shape {
.collect()
}
/// Returns all ancestor shapes of this shape, traversing up the parent hierarchy
///
/// This function walks up the parent chain starting from this shape's parent,
/// collecting all ancestor IDs. It stops when it reaches a nil UUID or when
/// an ancestor is hidden (unless include_hidden is true).
///
/// # Arguments
/// * `shapes` - The shapes pool containing all shapes
/// * `include_hidden` - Whether to include hidden ancestors in the result
///
/// # Returns
/// A set of ancestor UUIDs in traversal order (closest ancestor first)
pub fn all_ancestors(&self, shapes: &ShapesPool, include_hidden: bool) -> IndexSet<Uuid> {
let mut ancestors = IndexSet::new();
let mut current_id = self.id;
// Traverse upwards using parent_id
while let Some(parent_id) = shapes.get(&current_id).and_then(|s| s.parent_id) {
// If the parent_id is the zero UUID, there are no more ancestors
if parent_id == Uuid::nil() {
break;
}
// Check if the ancestor is hidden
if let Some(parent) = shapes.get(&parent_id) {
if !include_hidden && parent.hidden() {
break;
}
ancestors.insert(parent_id);
current_id = parent_id;
} else {
break;
}
}
ancestors
}
pub fn image_filter(&self, scale: f32) -> Option<skia::ImageFilter> {
if !self.blur.hidden {
match self.blur.blur_type {

44
render-wasm/src/state.rs
View File

@@ -99,7 +99,9 @@ impl State {
pub fn delete_shape(&mut self, id: Uuid) {
// We don't really do a self.shapes.remove so that redo/undo keep working
if let Some(shape) = self.shapes.get(&id) {
let tiles::TileRect(rsx, rsy, rex, rey) = self.render_state.get_tiles_for_shape(shape);
let tiles::TileRect(rsx, rsy, rex, rey) =
self.render_state
.get_tiles_for_shape(shape, &self.shapes, &self.modifiers);
for x in rsx..=rex {
for y in rsy..=rey {
let tile = tiles::Tile(x, y);
@@ -122,6 +124,31 @@ impl State {
self.render_state.set_background_color(color);
}
/// Sets the parent for the current shape and updates the parent's extended rectangle
///
/// When a shape is assigned a new parent, the parent's extended rectangle needs to be
/// invalidated and recalculated to include the new child. This ensures that frames
/// and groups properly encompass their children.
pub fn set_parent_for_current_shape(&mut self, id: Uuid) {
let shape = {
let Some(shape) = self.current_shape_mut() else {
panic!("Invalid current shape")
};
shape.set_parent(id);
shape.clone()
};
if let Some(parent) = shape.parent_id.and_then(|id| self.shapes.get_mut(&id)) {
parent.invalidate_extrect();
parent.add_child(shape.id);
}
}
/// Sets the selection rectangle for the current shape and processes its ancestors
///
/// When a shape's selection rectangle changes, all its ancestors need to have their
/// extended rectangles recalculated because the shape's bounds may have changed.
/// This ensures proper rendering of frames and groups containing the modified shape.
pub fn set_selrect_for_current_shape(&mut self, left: f32, top: f32, right: f32, bottom: f32) {
let shape = {
let Some(shape) = self.current_shape_mut() else {
@@ -130,16 +157,14 @@ impl State {
shape.set_selrect(left, top, right, bottom);
shape.clone()
};
// We don't need to update the tile for the root shape.
if !shape.id.is_nil() {
self.render_state.update_tile_for(&shape);
}
self.render_state
.process_shape_ancestors(&shape, &mut self.shapes, &self.modifiers);
}
pub fn update_tile_for_shape(&mut self, shape_id: Uuid) {
if let Some(shape) = self.shapes.get(&shape_id) {
self.render_state.update_tile_for(shape);
self.render_state
.update_tile_for(shape, &self.shapes, &self.modifiers);
}
}
@@ -148,7 +173,8 @@ impl State {
panic!("Invalid current shape")
};
if !shape.id.is_nil() {
self.render_state.update_tile_for(&shape.clone());
self.render_state
.update_tile_for(&shape.clone(), &self.shapes, &self.modifiers);
}
}
@@ -164,7 +190,7 @@ impl State {
pub fn rebuild_modifier_tiles(&mut self) {
self.render_state
.rebuild_modifier_tiles(&self.shapes, &self.modifiers);
.rebuild_modifier_tiles(&mut self.shapes, &self.modifiers);
}
pub fn font_collection(&self) -> &FontCollection {

2
render-wasm/src/tiles.rs
View File

@@ -7,7 +7,7 @@ use std::collections::{HashMap, HashSet};
#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)]
pub struct Tile(pub i32, pub i32);
#[derive(PartialEq, Eq, Hash, Clone, Copy)]
#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)]
pub struct TileRect(pub i32, pub i32, pub i32, pub i32);
impl TileRect {