From 2045fc490064839b0582fbfa8d0538500305758f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Fri, 23 Oct 2026 13:00:53 +0200 Subject: [PATCH] :bug: Keep text image fills in place when pasting (#12552) A text image fill was always laid over the whole text box, so a pasted image span showed another part of the picture in a box of another size. Text image fills now honour the image transform, as shape fills do: the image covers the transform rect instead of the whole box. Paste gives each pasted image span a transform that keeps the image at the same pixel offset from the span, like the gradients. The SVG export emits one linked image per span transform, rather than one per image id, and the style panel keeps the transform it reads back. AI-assisted-by: claude-opus-5-5 --- .../app/main/data/workspace/clipboard.cljs | 2 +- .../src/app/main/data/workspace/texts.cljs | 24 +++--- frontend/src/app/render_wasm/text_editor.cljs | 17 ++-- frontend/src/app/render_wasm/text_paste.cljs | 25 ++++-- .../render_wasm/text_paste_test.cljs | 39 +++++++-- render-wasm/src/render/images.rs | 10 +-- render-wasm/src/render/svg/fixtures.rs | 44 +++++++--- render-wasm/src/render/svg/tests.rs | 45 ++++++++++ render-wasm/src/render/svg/text.rs | 26 ++---- render-wasm/src/shapes/fills.rs | 86 ++++++++++++++++--- render-wasm/src/shapes/text.rs | 12 +-- 11 files changed, 239 insertions(+), 91 deletions(-) diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index 4796ab969c..6b560ccd6f 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -1116,7 +1116,7 @@ (dwsh/create-and-add-shape :text x y shape (when skip-edition? {:skip-edition? true}))) (if skip-edition? - (rx/of (dwtxt/move-new-text-gradients id payload) + (rx/of (dwtxt/move-new-text-fills id payload) (dwwt/resize-wasm-text-debounce id {:undo-group id :undo-id undo-id})) (rx/of (dwu/commit-undo-transaction undo-id)))))))))) diff --git a/frontend/src/app/main/data/workspace/texts.cljs b/frontend/src/app/main/data/workspace/texts.cljs index 8ac273e1d2..c50af92813 100644 --- a/frontend/src/app/main/data/workspace/texts.cljs +++ b/frontend/src/app/main/data/workspace/texts.cljs @@ -1708,8 +1708,8 @@ :fragment fragment})))) (defn- target-geometry - "Where the gradient spans of `pasted`, inserted at `start`, begin in `shape` with - `content`, laid over the box it gets once resized to its text; nil when none." + "Where the gradient and image spans of `pasted`, inserted at `start`, begin in `shape` + with `content`, laid over the box it gets once resized to its text; nil when none." [{:keys [id grow-type]} content pasted start] (when-let [geometry (wasm.text-editor/content-geometry id pasted start)] (if (= :fixed grow-type) @@ -1728,23 +1728,23 @@ :height height}))))] (update geometry :origins (partial mapv (partial mapv #(some-> % fit)))))))) -(defn- move-pasted-gradients - "`content`, the shape's after restyling the paste, with the pasted gradients - keeping their place on their spans. Pushes the result to WASM." +(defn- move-pasted-fills + "`content`, the shape's after restyling the paste, with the pasted gradients and + images keeping their place on their spans. Pushes the result to WASM." [shape content {:keys [geometry] :as paste} start] (let [pasted (:content paste) target (when (some? geometry) (target-geometry shape content pasted start))] (if (some? target) - (let [content (text-paste/move-gradients content pasted start geometry target)] + (let [content (text-paste/move-span-fills content pasted start geometry target)] (wasm.api/set-shape-text-content (:id shape) content) content) content))) -(defn move-new-text-gradients - "Event moving the gradients of `payload`, just pasted on the canvas as the text - `id`, so they keep their place on their spans." +(defn move-new-text-fills + "Event moving the gradients and images of `payload`, just pasted on the canvas as the + text `id`, so they keep their place on their spans." [id {:keys [geometry]}] - (ptk/reify ::move-new-text-gradients + (ptk/reify ::move-new-text-fills ptk/WatchEvent (watch [_ state _] (let [{:keys [content] :as shape} (dsh/lookup-shape state id) @@ -1756,7 +1756,7 @@ (if (some? target) (rx/of (dwsh/update-shapes [id] - #(update % :content text-paste/move-gradients content start geometry target) + #(update % :content text-paste/move-span-fills content start geometry target) {:attrs #{:content}})) (rx/empty)))))) @@ -1781,7 +1781,7 @@ (wasm.text-editor/text-editor-sync-content) (if-let [{:keys [content]} (wasm.api/apply-paste-styles shape-id fragment start penpot?)] (let [shape (dsh/lookup-shape state shape-id)] - (v3-update-editor-content shape-id (move-pasted-gradients shape content paste start))) + (v3-update-editor-content shape-id (move-pasted-fills shape content paste start))) (v3-sync-editor-content))) pending? diff --git a/frontend/src/app/render_wasm/text_editor.cljs b/frontend/src/app/render_wasm/text_editor.cljs index 4348117ed8..ebfabdb07d 100644 --- a/frontend/src/app/render_wasm/text_editor.cljs +++ b/frontend/src/app/render_wasm/text_editor.cljs @@ -123,12 +123,17 @@ width (aget heap-i32 (+ fill-u32-offset 6)) height (aget heap-i32 (+ fill-u32-offset 7))] {:fill-opacity (u8->opacity alpha) - :fill-image {:id (uuid/from-unsigned-parts a b c d) - :width width - :height height - :mtype (types.fills.impl/decode-mtype mtype) - :keep-aspect-ratio (not (zero? (bit-and flags 0x01))) - :name "sample"}}) + :fill-image (cond-> {:id (uuid/from-unsigned-parts a b c d) + :width width + :height height + :mtype (types.fills.impl/decode-mtype mtype) + :keep-aspect-ratio (not (zero? (bit-and flags 0x01))) + :name "sample"} + (not (zero? (bit-and flags 0x02))) + (assoc :transform {:x (aget heap-f32 (+ fill-u32-offset 8)) + :y (aget heap-f32 (+ fill-u32-offset 9)) + :width (aget heap-f32 (+ fill-u32-offset 10)) + :height (aget heap-f32 (+ fill-u32-offset 11))}))}) nil))) diff --git a/frontend/src/app/render_wasm/text_paste.cljs b/frontend/src/app/render_wasm/text_paste.cljs index 4520327ce6..bbc5b5d512 100644 --- a/frontend/src/app/render_wasm/text_paste.cljs +++ b/frontend/src/app/render_wasm/text_paste.cljs @@ -127,7 +127,7 @@ content (rest (map-indexed vector fragment))))) -;; --- Gradients of text copied in Penpot +;; --- Gradients and images of text copied in Penpot (defn- move-gradient "`gradient`, in fractions of the `from` box, in fractions of the `to` box, with its @@ -146,22 +146,37 @@ (= :radial (:type gradient)) (update :width #(/ (* % w0 h1) (* h0 w1)))))) +(defn- move-image + "`image`, laid over its transform in the `from` box (the whole box without one), + with a transform in the `to` box at the same pixel offset from the span origin." + [image from to] + (let [{x0 :x y0 :y {w0 :width h0 :height} :box} from + {x1 :x y1 :y {w1 :width h1 :height} :box} to + {:keys [x y width height]} (get image :transform {:x 0 :y 0 :width 1 :height 1})] + (assoc image :transform {:x (/ (+ x1 (- (* x w0) x0)) w1) + :y (/ (+ y1 (- (* y h0) y0)) h1) + :width (/ (* width w0) w1) + :height (/ (* height h0) h1)}))) + (defn- move-fills [span from to] (update span :fills (fn [fills] (mapv #(cond-> % (some? (:fill-color-gradient %)) - (update :fill-color-gradient move-gradient from to)) + (update :fill-color-gradient move-gradient from to) + + (some? (:fill-image %)) + (update :fill-image move-image from to)) fills)))) (defn- sized? [{{:keys [width height]} :box}] (and (pos? width) (pos? height))) -(defn move-gradients - "`content` with the gradients of `pasted` (the content inserted at `start`) moved - from the `source` geometry to the `target` one (see `text-editor/content-geometry`)." +(defn move-span-fills + "`content` with the gradients and images of `pasted`, inserted at `start`, moved from + the `source` geometry to the `target` one (see `text-editor/content-geometry`)." [content pasted start source target] (let [entries (map vector (flatten (text-editor/span-ranges pasted start)) diff --git a/frontend/test/frontend_tests/render_wasm/text_paste_test.cljs b/frontend/test/frontend_tests/render_wasm/text_paste_test.cljs index c9bc145647..a52c1f5e2e 100644 --- a/frontend/test/frontend_tests/render_wasm/text_paste_test.cljs +++ b/frontend/test/frontend_tests/render_wasm/text_paste_test.cljs @@ -213,8 +213,8 @@ (let [linear (gradient-fill {:start-x 0.25 :start-y 0 :end-x 0.75 :end-y 1}) pasted (content [(span "HELLO" linear)]) target (content [(span "abc") (span "HELLO" linear)]) - result (text-paste/move-gradients target pasted {:para 0 :offset 3} - source-geometry target-geometry)] + result (text-paste/move-span-fills target pasted {:para 0 :offset 3} + source-geometry target-geometry)] (t/testing "points move into the target box at the same pixel offsets" (t/is (= {:start-x 0.1 :start-y 0.5 :end-x 1.1 :end-y 1} (select-keys (pasted-gradient result) [:start-x :start-y :end-x :end-y])))) @@ -227,8 +227,8 @@ (let [radial (gradient-fill {:type :radial :start-x 0.5 :start-y 0.5 :end-x 0.5 :end-y 1}) pasted (content [(span "HELLO" radial)]) target (content [(span "abc") (span "HELLO" radial)]) - result (text-paste/move-gradients target pasted {:para 0 :offset 3} - source-geometry target-geometry)] + result (text-paste/move-span-fills target pasted {:para 0 :offset 3} + source-geometry target-geometry)] ;; 200x40 is 5:1 and 100x80 is 5:4, so the width grows 4 times. (t/is (= 4 (:width (pasted-gradient result)))))) @@ -238,12 +238,12 @@ pasted (content [(span "red" red) (span "grad" linear)]) target (content [(span "red" red) (span "grad" linear)])] (t/testing "a span with no origin keeps its gradient" - (t/is (= target (text-paste/move-gradients + (t/is (= target (text-paste/move-span-fills target pasted {:para 0 :offset 0} {:origins [[nil nil]]} {:origins [[nil nil]]})))) (t/testing "an empty box leaves the gradients as they were" - (t/is (= target (text-paste/move-gradients + (t/is (= target (text-paste/move-span-fills target pasted {:para 0 :offset 0} {:origins [[nil {:x 0 :y 0 :box source-box}]]} {:origins [[nil {:x 0 :y 0 :box {:width 0 :height 80}}]]})))))) @@ -256,9 +256,34 @@ [{:x 0 :y 0 :box {:width 200 :height 40}}]]} target {:origins [[{:x 0 :y 0 :box {:width 400 :height 80}}] [{:x 0 :y 40 :box {:width 400 :height 80}}]]} - result (text-paste/move-gradients pasted pasted {:para 0 :offset 0} source target) + result (text-paste/move-span-fills pasted pasted {:para 0 :offset 0} source target) points #(select-keys (-> % :fills first :fill-color-gradient) [:start-x :start-y :end-x :end-y])] (t/is (= [{:start-x 0.125 :start-y 0 :end-x 0.125 :end-y 0.25} {:start-x 0.25 :start-y 0.5 :end-x 0.25 :end-y 1}] (map (comp points first) (spans-of result)))))) + +;; --- Images + +(defn- image-fill [image] + {:fills [{:fill-image (merge {:id #uuid "00000000-0000-0000-0000-000000000042" + :width 400 :height 200 :mtype "image/png"} + image)}]}) + +(defn- pasted-image-transform [content] + (-> (spans-of content) first second :fills first :fill-image :transform)) + +(t/deftest images-keep-their-offset-from-the-span + (let [move (fn [image] + (let [fill (image-fill image)] + (pasted-image-transform + (text-paste/move-span-fills (content [(span "abc") (span "HELLO" fill)]) + (content [(span "HELLO" fill)]) + {:para 0 :offset 3} + source-geometry target-geometry))))] + (t/testing "an image over the whole source box gets a transform over the same pixels" + (t/is (= {:x -0.4 :y 0.5 :width 2 :height 0.5} + (move {})))) + (t/testing "an image with a transform keeps its pixel rect" + (t/is (= {:x 0.1 :y 0.5 :width 1 :height 0.5} + (move {:transform {:x 0.25 :y 0 :width 0.5 :height 1}})))))) diff --git a/render-wasm/src/render/images.rs b/render-wasm/src/render/images.rs index 5b59b3620a..bcbf557d0c 100644 --- a/render-wasm/src/render/images.rs +++ b/render-wasm/src/render/images.rs @@ -21,15 +21,7 @@ pub fn get_dest_rect(container: &MathRect, delta: f32) -> MathRect { } pub fn get_image_dest_rect(container: &MathRect, image_fill: &ImageFill) -> MathRect { - match image_fill.transform() { - Some(tf) => MathRect::from_xywh( - container.left + tf.x * container.width(), - container.top + tf.y * container.height(), - tf.width * container.width(), - tf.height * container.height(), - ), - None => *container, - } + image_fill.dest_rect(container) } pub fn get_source_rect(size: ISize, container: &MathRect, image_fill: &ImageFill) -> MathRect { diff --git a/render-wasm/src/render/svg/fixtures.rs b/render-wasm/src/render/svg/fixtures.rs index db7759102b..a33d5d258e 100644 --- a/render-wasm/src/render/svg/fixtures.rs +++ b/render-wasm/src/render/svg/fixtures.rs @@ -335,23 +335,39 @@ pub(super) fn add_text_with_fills( text: &str, font_size: f32, fills: Vec, +) { + add_text_with_spans(pool, id, (l, t, r, b), font_size, vec![(text, fills)]); +} + +/// Fixed text with one paragraph of `spans`, each a text and its fills. +pub(super) fn add_text_with_spans( + pool: &mut ShapesPool, + id: Uuid, + (l, t, r, b): (f32, f32, f32, f32), + font_size: f32, + spans: Vec<(&str, Vec)>, ) { let bounds = skia::Rect::from_ltrb(l, t, r, b); let mut content = TextContent::new(bounds, GrowType::Fixed); let line_height = 1.2; - let span = TextSpan::new( - text.to_string(), - FontFamily::new(Uuid::nil(), 400, FontStyle::Normal), - font_size, - line_height, - 0.0, - None, - None, - TextDirection::LTR, - 400, - Uuid::nil(), - fills, - ); + let spans = spans + .into_iter() + .map(|(text, fills)| { + TextSpan::new( + text.to_string(), + FontFamily::new(Uuid::nil(), 400, FontStyle::Normal), + font_size, + line_height, + 0.0, + None, + None, + TextDirection::LTR, + 400, + Uuid::nil(), + fills, + ) + }) + .collect(); content.add_paragraph(Paragraph::new( TextAlign::Left, TextDirection::LTR, @@ -359,7 +375,7 @@ pub(super) fn add_text_with_fills( None, line_height, 0.0, - vec![span], + spans, )); let shape = pool.add_shape(id); diff --git a/render-wasm/src/render/svg/tests.rs b/render-wasm/src/render/svg/tests.rs index 02eda33b79..87eeb29fb4 100644 --- a/render-wasm/src/render/svg/tests.rs +++ b/render-wasm/src/render/svg/tests.rs @@ -2957,6 +2957,51 @@ fn exports_image_fill_on_text() { insta::assert_snapshot!(svg); } +#[test] +fn exports_one_image_per_text_image_transform() { + let mut pool = ShapesPool::new(); + let id = uid(1); + let image_id = uid(42); + let image = |x: f32| { + Fill::Image(ImageFill::new_with_transform( + image_id, + 255, + 200, + 100, + true, + Some(ImageFillTransform { + x, + y: 0.0, + width: 0.5, + height: 1.0, + }), + )) + }; + add_text_with_spans( + &mut pool, + id, + (0.0, 0.0, 400.0, 100.0), + 60.0, + vec![("HO", vec![image(0.0)]), ("LA", vec![image(0.5)])], + ); + + let svg = render_with(&pool, id, |resources| { + resources + .images + .set_source_url(image_id, TEST_IMAGE_URL.to_string()); + }); + + assert_eq!(svg.matches(" skia_safe::Rect { - let image_width = image_fill.width() as f32; - let image_height = image_fill.height() as f32; - let scale_x = selrect.width() / image_width; - let scale_y = selrect.height() / image_height; - let scale = scale_x.max(scale_y); - let scaled_width = image_width * scale; - let scaled_height = image_height * scale; - let pos_x = selrect.left() - (scaled_width - selrect.width()) / 2.0; - let pos_y = selrect.top() - (scaled_height - selrect.height()) / 2.0; - skia_safe::Rect::from_xywh(pos_x, pos_y, scaled_width, scaled_height) -} - fn push_glyph_clip( builder: &mut SvgLayerCanvas, element: &Shape, @@ -365,12 +351,12 @@ fn linked_image_fills_at_layer<'a>( layer: usize, shared: &RenderResources, ) -> Vec<&'a ImageFill> { - let mut out = Vec::new(); - let mut seen = HashSet::new(); + // Spans can share an image with different transforms: one element each. + let mut out: Vec<&ImageFill> = Vec::new(); for paragraph in text_content.paragraphs() { for span in paragraph.children() { if let Some(Fill::Image(img)) = span.fills_from_bottom(layer) { - if shared.images.source_url(&img.id()).is_some() && seen.insert(img.id()) { + if shared.images.source_url(&img.id()).is_some() && !out.contains(&img) { out.push(img); } } @@ -412,7 +398,7 @@ fn emit_text_image_fill( let clip_id = builder.unique("txtimgclip"); let text_content = shape.get_text_content().new_bounds(shape.selrect()); let mut paragraph_builders = - text_content.paragraph_builder_group_opaque_for_image_layer(layer, image_fill.id()); + text_content.paragraph_builder_group_opaque_for_image_layer(layer, image_fill); let canvas = builder.new_fragment(); { @@ -428,7 +414,7 @@ fn emit_text_image_fill( emit_linked_image_element( builder, image_fill, - shape.selrect(), + image_fill.dest_rect(&shape.selrect()), &href, Some(&clip_id), true, diff --git a/render-wasm/src/shapes/fills.rs b/render-wasm/src/shapes/fills.rs index c819579de0..15afbbbba7 100644 --- a/render-wasm/src/shapes/fills.rs +++ b/render-wasm/src/shapes/fills.rs @@ -212,6 +212,35 @@ impl ImageFill { pub fn transform(&self) -> Option<&ImageFillTransform> { self.transform.as_ref() } + + /// The part of `container` the image is laid over: its transform, else all of it. + pub fn dest_rect(&self, container: &Rect) -> Rect { + match &self.transform { + Some(tf) => Rect::from_xywh( + container.left + tf.x * container.width(), + container.top + tf.y * container.height(), + tf.width * container.width(), + tf.height * container.height(), + ), + None => *container, + } + } + + /// Where the image is drawn to cover its dest rect in `container`, centered. + pub fn cover_rect(&self, container: &Rect) -> Rect { + let dest = self.dest_rect(container); + let image_width = self.width as f32; + let image_height = self.height as f32; + let scale = (dest.width() / image_width).max(dest.height() / image_height); + let scaled_width = image_width * scale; + let scaled_height = image_height * scale; + Rect::from_xywh( + dest.left - (scaled_width - dest.width()) / 2.0, + dest.top - (scaled_height - dest.height()) / 2.0, + scaled_width, + scaled_height, + ) + } } #[derive(Debug, Clone, PartialEq, Copy)] @@ -310,20 +339,14 @@ pub fn get_fill_shader(fill: &Fill, bounding_box: &Rect) -> Option let sampling_options = skia::SamplingOptions::new(skia::FilterMode::Linear, skia::MipmapMode::Nearest); - // FIXME no image ratio applied, centered to the current rect + // Covers the dest rect (transform, else the whole box), centered. + // keep_aspect_ratio is not applied: the image always covers. let tile_modes = (skia::TileMode::Clamp, skia::TileMode::Clamp); - let image_width = image_fill.width as f32; - let image_height = image_fill.height as f32; - let scale_x = bounding_box.width() / image_width; - let scale_y = bounding_box.height() / image_height; - let scale = scale_x.max(scale_y); - let scaled_width = image_width * scale; - let scaled_height = image_height * scale; - let pos_x = bounding_box.left() - (scaled_width - bounding_box.width()) / 2.0; - let pos_y = bounding_box.top() - (scaled_height - bounding_box.height()) / 2.0; + let cover = image_fill.cover_rect(bounding_box); + let scale = cover.width() / image_fill.width as f32; let mut matrix = skia::Matrix::new_identity(); - matrix.pre_translate((pos_x, pos_y)); + matrix.pre_translate((cover.left, cover.top)); matrix.pre_scale((scale, scale), None); let opacity = image_fill.opacity(); @@ -394,3 +417,44 @@ pub fn set_paint_fill(paint: &mut Paint, fill: &Fill, bounding_box: &Rect, remov paint.set_shader(shader); } } + +#[cfg(test)] +mod tests { + use super::*; + + fn image_fill(transform: Option) -> ImageFill { + ImageFill::new_with_transform(Uuid::nil(), 255, 100, 50, true, transform) + } + + #[test] + fn an_image_without_transform_covers_the_whole_container() { + let container = Rect::from_xywh(10.0, 20.0, 100.0, 100.0); + + assert_eq!(image_fill(None).dest_rect(&container), container); + // 100x50 scaled by 2 to fill the height, centered horizontally. + assert_eq!( + image_fill(None).cover_rect(&container), + Rect::from_xywh(-40.0, 20.0, 200.0, 100.0) + ); + } + + #[test] + fn an_image_with_transform_covers_its_part_of_the_container() { + let container = Rect::from_xywh(10.0, 20.0, 200.0, 100.0); + let fill = image_fill(Some(ImageFillTransform { + x: 0.5, + y: -0.5, + width: 0.5, + height: 1.0, + })); + + assert_eq!( + fill.dest_rect(&container), + Rect::from_xywh(110.0, -30.0, 100.0, 100.0) + ); + assert_eq!( + fill.cover_rect(&container), + Rect::from_xywh(60.0, -30.0, 200.0, 100.0) + ); + } +} diff --git a/render-wasm/src/shapes/text.rs b/render-wasm/src/shapes/text.rs index 49b8fc4861..a8328be0d1 100644 --- a/render-wasm/src/shapes/text.rs +++ b/render-wasm/src/shapes/text.rs @@ -883,18 +883,18 @@ impl TextContent { } /// Opaque black glyphs only for spans whose fill at `layer_from_bottom` is - /// the given image — used as an SVG `` for linked image fills. + /// `image_fill` — used as an SVG `` for linked image fills. pub fn paragraph_builder_group_opaque_for_image_layer( &self, layer_from_bottom: usize, - image_id: Uuid, + image_fill: &shapes::ImageFill, ) -> Vec { self.paragraph_builders( None, false, None, None, - Some((layer_from_bottom, image_id)), + Some((layer_from_bottom, image_fill)), None, ) } @@ -905,7 +905,7 @@ impl TextContent { opaque: bool, align_override: Option, fill_layer: Option, - opaque_image_layer: Option<(usize, Uuid)>, + opaque_image_layer: Option<(usize, &shapes::ImageFill)>, skip_image_ids: Option<&HashSet>, ) -> Vec { let fonts = get_font_collection(); @@ -920,7 +920,7 @@ impl TextContent { let mut builder = ParagraphBuilder::new(¶graph_style, fonts); let mut has_text = false; for span in paragraph.children() { - let text_style = if let Some((layer, image_id)) = opaque_image_layer { + let text_style = if let Some((layer, image_fill)) = opaque_image_layer { let mut style = span.to_style( &self.bounds(), fallback_fonts, @@ -929,7 +929,7 @@ impl TextContent { ); let mut paint = paint::Paint::default(); match span.fills_from_bottom(layer) { - Some(shapes::Fill::Image(img)) if img.id() == image_id => { + Some(shapes::Fill::Image(img)) if img == image_fill => { paint.set_color(skia::Color::BLACK); paint.set_alpha(255); }