From 708c4065b3208e276ffc5d1cdae875a5579ff1a7 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Wed, 6 May 2026 07:33:40 +0200 Subject: [PATCH] :bug: Fix drag and drop cache eligibility rules --- render-wasm/src/render.rs | 68 ++++++++++++++++-------- render-wasm/src/shapes.rs | 106 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 22 deletions(-) diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index 1b384e235d..a6822c991e 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -391,6 +391,9 @@ pub(crate) struct RenderState { pub struct InteractiveDragCrop { pub src_doc_bounds: Rect, pub src_selrect: Rect, + /// True if the captured crop bounds were fully inside the viewport at capture time. + /// Used to avoid serving partial/offscreen crops during interactive drag. + pub fits_viewport_at_capture: bool, pub image: skia::Image, } @@ -444,13 +447,30 @@ impl RenderState { let Some(m) = tree.get_modifier(&node_id) else { return false; }; - return crate::math::is_move_only_matrix(m); + // Only allow using the cached pixels for pure translations. + // For non-translation transforms (scale/rotate/skew), cached pixels won't match. + if !crate::math::is_move_only_matrix(m) { + return false; + } + + let Some(crop) = self.backbuffer_crop_cache.get(&node_id) else { + return false; + }; + if !crop.fits_viewport_at_capture { + return false; + } + + // Additionally require this node to be safe to serve from a rectangular backbuffer + // crop while moving; otherwise it must be rendered live (e.g. text, overflow frames). + return tree + .get(&node_id) + .is_some_and(|s| s.is_safe_for_drag_crop_cache(tree)); } - // Invalidate cached top-level pixels whenever the moving content overlaps - // the cached pixel area. Use `src_doc_bounds` because it's the exact bounds - // captured from the Backbuffer crop (more reliable than extents derived - // from layout/layout-less container heuristics). + // If the moving content overlaps this cached crop, do not use the cached pixels + // for this frame. We intentionally keep the cache entry: overlap is typically + // transient during drag, and once the moving content leaves the area the crop + // becomes valid again (stationary shape unchanged). if let Some(moved) = moved_bounds { let intersects = self .backbuffer_crop_cache @@ -458,25 +478,12 @@ impl RenderState { .is_some_and(|crop| moved.intersects(crop.src_doc_bounds)); if intersects { - // Simplest "automatic invalidation": once something moves over this cached - // area, drop the cached crop so it won't be reused again until the next - // full-frame rebuild. - self.backbuffer_crop_cache.remove(&node_id); return false; } } true } - fn is_recortable_for_drag_crop(&self, shape: &Shape) -> bool { - // "Recortable" (happy path): the shape is fully represented by the pixels - // already in Backbuffer and can be moved as a texture during drag. - shape.blur.is_none() - && shape.shadows.is_empty() - && (shape.opacity - 1.0).abs() <= 1e-4 - && shape.blend_mode().0 == skia::BlendMode::SrcOver - } - pub fn try_new(width: i32, height: i32) -> Result { // This needs to be done once per WebGL context. let mut gpu_state = GpuState::try_new()?; @@ -1649,9 +1656,6 @@ impl RenderState { if shape.hidden { continue; } - if !self.is_recortable_for_drag_crop(shape) { - continue; - } let doc_bounds = self.get_cached_extrect(shape, tree, 1.0); if !doc_bounds.intersects(viewport) { @@ -1707,11 +1711,16 @@ impl RenderState { src_irect.right as f32 / scale + vb_left, src_irect.bottom as f32 / scale + vb_top, ); + let fits_viewport_at_capture = doc_bounds.left >= viewport.left + && doc_bounds.top >= viewport.top + && doc_bounds.right <= viewport.right + && doc_bounds.bottom <= viewport.bottom; self.backbuffer_crop_cache.insert( id, InteractiveDragCrop { src_doc_bounds, src_selrect: selrect, + fits_viewport_at_capture, image, }, ); @@ -3024,16 +3033,31 @@ impl RenderState { dst_doc_rect.height() * scale, ); - // let canvas = self.surfaces.canvas_and_mark_dirty(target_surface); let canvas = self.surfaces.canvas(target_surface); canvas.save(); canvas.reset_matrix(); + // If the crop includes shadows/blur (extrect pixels outside the fill/stroke + // silhouette), do NOT apply the silhouette clip or we'd cut those pixels. + let should_clip_crop = element.shadows.is_empty() && element.blur.is_none(); + if should_clip_crop { + if let Some(clip_path) = element.drag_crop_clip_path() { + let mut doc_to_tile = Matrix::new_identity(); + // Map document-space coordinates into tile pixels. + // Rendering surfaces apply: scale(scale) then translate(translation) in doc units. + // Equivalent point mapping: (doc + translation) * scale. + doc_to_tile.post_translate((translation.0, translation.1)); + doc_to_tile.post_scale((scale, scale), None); + let clip_path = clip_path.make_transform(&doc_to_tile); + canvas.clip_path(&clip_path, skia::ClipOp::Intersect, true); + } + } canvas.draw_image_rect( crop_image, None, dst_tile_rect, &skia::Paint::default(), ); + canvas.restore(); } continue; diff --git a/render-wasm/src/shapes.rs b/render-wasm/src/shapes.rs index ec99e7fef0..22e9e1e9d8 100644 --- a/render-wasm/src/shapes.rs +++ b/render-wasm/src/shapes.rs @@ -1358,6 +1358,112 @@ impl Shape { } } + /// Same `concat` applied around [`center`](Self::center) as in `render_shape` (non-text branch). + fn shape_document_transform(&self) -> Matrix { + let c = self.center(); + let mut m = self.transform; + m.post_translate(c); + m.pre_translate(-c); + m + } + + /// Fill silhouette only, document space (matches fill rendering). + fn drag_crop_fill_clip_path_skia(&self) -> Option { + match &self.shape_type { + Type::Rect(r) => { + let p = Path::new(shape_to_path::rect_segments(self, r.corners)); + Some(p.to_skia_path(self.svg_attrs.as_ref())) + } + Type::Circle => { + let p = Path::new(shape_to_path::circle_segments(self)); + Some(p.to_skia_path(self.svg_attrs.as_ref())) + } + Type::Path(_) | Type::Bool(_) => { + let sk = self.get_skia_path()?; + Some(sk.make_transform(&self.shape_document_transform())) + } + _ => None, + } + } + + /// Whether this shape may use the backbuffer crop fast path during interactive drag. + /// + /// Conservative: only effects and fills that match what we snapshot and clip in + /// [`drag_crop_clip_path`](Self::drag_crop_clip_path). Text is never safe (glyph layout, + /// no `drag_crop_clip_path`). + pub fn is_safe_for_drag_crop_cache(&self, shapes_pool: ShapesPoolRef) -> bool { + if matches!(self.shape_type, Type::Text(_)) { + return false; + } + + // If a frame shows overflow (clip_content=false) and its visible content exceeds the + // frame bounds, a cached crop anchored to the frame can easily become incorrect while + // moving (children can extend beyond selrect). Be conservative and render live. + if matches!(self.shape_type, Type::Frame(_)) && !self.clip_content { + let extrect = self.extrect(shapes_pool, 1.0); + let sr = self.selrect; + let exceeds = extrect.left < sr.left + || extrect.top < sr.top + || extrect.right > sr.right + || extrect.bottom > sr.bottom; + if exceeds { + return false; + } + } + + let has_opaque_fill = self + .fills + .iter() + .any(|f| math::is_close_to(f.opacity(), 1.0)); + + self.blur.is_none() + && self.shadows.is_empty() + && (self.opacity - 1.0).abs() <= 1e-4 + && self.blend_mode().0 == skia::BlendMode::SrcOver + && has_opaque_fill + } + + /// Fill + visible strokes in **document space** for clipping interactive drag textures. + /// + /// The backbuffer crop uses an axis-aligned `extrect`; we clip the blit so backdrop pixels + /// outside the real silhouette (fill and stroke regions) are not smeared. Strokes use + /// [`stroke_to_path`](stroke_to_path) like the main renderer, then union with the fill path. + pub fn drag_crop_clip_path(&self) -> Option { + let mut acc = self.drag_crop_fill_clip_path_skia()?; + if !self.has_visible_strokes() { + return Some(acc); + } + + let shape_path = match &self.shape_type { + Type::Rect(r) => Path::new(shape_to_path::rect_segments(self, r.corners)), + Type::Circle => Path::new(shape_to_path::circle_segments(self)), + Type::Path(_) | Type::Bool(_) => self.shape_type.path()?.clone(), + _ => return Some(acc), + }; + + let path_transform = self.to_path_transform(); + let apply_doc_transform = path_transform.is_some(); + + for stroke in self.visible_strokes() { + let Some(stroke_region) = stroke_to_path( + stroke, + &shape_path, + path_transform.as_ref(), + &self.selrect, + self.svg_attrs.as_ref(), + ) else { + continue; + }; + let mut sk = stroke_region.to_skia_path(self.svg_attrs.as_ref()); + if apply_doc_transform { + sk = sk.make_transform(&self.shape_document_transform()); + } + acc = acc.op(&sk, skia::PathOp::Union).unwrap_or(acc); + } + + Some(acc) + } + fn transform_selrect(&mut self, transform: &Matrix) { if math::is_move_only_matrix(transform) { let tx = transform.translate_x();