From ba9d225c2b13831308ec0f59c75774dc198094e6 Mon Sep 17 00:00:00 2001 From: Alonso Torres Date: Tue, 2 Jun 2026 17:42:35 +0200 Subject: [PATCH] :bug: Fix stroke-cap-start/end stored at wrong level in SVG imports (#9982) --- common/src/app/common/files/migrations.cljc | 26 +++++++++++- .../src/app/common/files/shapes_builder.cljc | 14 +++---- .../files/shapes_builder_test.cljc | 41 +++++++++++++++++++ .../common_tests/files_migrations_test.cljc | 40 ++++++++++++++++++ 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 113634304a..aafd4cbbfd 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -1837,6 +1837,29 @@ [data _] (d/update-when data :tokens-lib ctob/fix-missing-sets-in-themes)) +;; This will fix incorrectly created strokes from SVG imports +;; that have the stroke-cap at the shape level instead of at the stroke level +(defmethod migrate-data "0024-fix-stroke-cap-placement" + [data _] + (letfn [(fix-shape [shape] + (let [cap-start (get shape :stroke-cap-start) + cap-end (get shape :stroke-cap-end)] + (if (or (some? cap-start) (some? cap-end)) + (cond-> (dissoc shape :stroke-cap-start :stroke-cap-end) + (and (some? cap-start) (seq (:strokes shape))) + (assoc-in [:strokes 0 :stroke-cap-start] cap-start) + + (and (some? cap-end) (seq (:strokes shape))) + (assoc-in [:strokes 0 :stroke-cap-end] cap-end)) + shape))) + + (update-container [container] + (d/update-when container :objects d/update-vals fix-shape))] + + (-> data + (update :pages-index d/update-vals update-container) + (d/update-when :components d/update-vals update-container)))) + (def available-migrations (into (d/ordered-set) ["legacy-2" @@ -1917,4 +1940,5 @@ "0020-sync-component-id-with-near-main" "0021-fix-shape-svg-attrs" "0022-normalize-component-root-and-resync" - "0023-repair-token-themes-with-inexistent-sets"])) + "0023-repair-token-themes-with-inexistent-sets" + "0024-fix-stroke-cap-placement"])) diff --git a/common/src/app/common/files/shapes_builder.cljc b/common/src/app/common/files/shapes_builder.cljc index dde8e152cb..d8b2c2fa64 100644 --- a/common/src/app/common/files/shapes_builder.cljc +++ b/common/src/app/common/files/shapes_builder.cljc @@ -609,17 +609,13 @@ (and (some? color) (some? width)) (assoc-in [:strokes 0 :stroke-width] width) - (and (some? linecap) (cfh/path-shape? shape) + (and (some? color) (some? linecap) (cfh/path-shape? shape) (or (= linecap :round) (= linecap :square))) + (assoc-in [:strokes 0 :stroke-cap-start] linecap) - (assoc :stroke-cap-start linecap - :stroke-cap-end linecap - :stroke-linecap linecap) - - (d/any-key? (dm/get-in shape [:strokes 0]) - :strokeColor :strokeOpacity :strokeWidth - :strokeLinecap :strokeCapStart :strokeCapEnd) - (assoc-in [:strokes 0 :stroke-style] :svg)))) + (and (some? color) (some? linecap) (cfh/path-shape? shape) + (or (= linecap :round) (= linecap :square))) + (assoc-in [:strokes 0 :stroke-cap-end] linecap)))) (defn setup-opacity [shape] (cond-> shape diff --git a/common/test/common_tests/files/shapes_builder_test.cljc b/common/test/common_tests/files/shapes_builder_test.cljc index 05956918b2..f57fe7ac07 100644 --- a/common/test/common_tests/files/shapes_builder_test.cljc +++ b/common/test/common_tests/files/shapes_builder_test.cljc @@ -50,3 +50,44 @@ (t/deftest resolve-element-name-empty-attrs-uses-tag-fallback (t/is (some? (sb/resolve-element-name :path {})))) + +;; Regression for https://tree.taiga.io/project/penpot/issue/8277 +;; stroke-linecap and stroke-linejoin on the SVG root must be inherited +;; by child path shapes, and stroke-cap-start/end must be stored inside +;; the stroke entry (not at the shape top level). +(t/deftest svg-root-stroke-linecap-inherited-to-path-shapes + (let [svg-data {:name "icon" + :tag :svg + :attrs {:xmlns "http://www.w3.org/2000/svg" + :width "24" :height "24" + :viewBox "0 0 24 24" + :fill "none" + :stroke "currentColor" + :stroke-width "2" + :stroke-linecap "round" + :stroke-linejoin "round"} + :content [{:tag :line + :attrs {:x1 "12" :y1 "8" :x2 "12" :y2 "12"} + :content []}]} + [_root children] (sb/create-svg-shapes svg-data {:x 0 :y 0} {} nil nil #{} false) + path-shapes (filter #(= :path (:type %)) children)] + + ;; At least one path shape was created from the element + (t/is (seq path-shapes)) + + (doseq [shape path-shapes] + (let [stroke (first (:strokes shape))] + ;; svg-attrs must carry stroke-linecap and stroke-linejoin for the renderers + (t/is (= "round" (get-in shape [:svg-attrs :strokeLinecap]))) + (t/is (= "round" (get-in shape [:svg-attrs :strokeLinejoin]))) + + ;; stroke-cap-start/end must be inside the stroke entry, not at shape level + (t/is (= :round (:stroke-cap-start stroke)) + "stroke-cap-start should be in the stroke entry") + (t/is (= :round (:stroke-cap-end stroke)) + "stroke-cap-end should be in the stroke entry") + (t/is (nil? (:stroke-cap-start shape)) + "stroke-cap-start must NOT be at the shape top level") + + ;; stroke-style is not set at import time (nil means default solid rendering) + (t/is (nil? (:stroke-style stroke))))))) diff --git a/common/test/common_tests/files_migrations_test.cljc b/common/test/common_tests/files_migrations_test.cljc index a48f3786d9..63dffe3e3c 100644 --- a/common/test/common_tests/files_migrations_test.cljc +++ b/common/test/common_tests/files_migrations_test.cljc @@ -10,6 +10,7 @@ [app.common.files.migrations :as cfm] [app.common.pprint :as pp] [app.common.types.file :as ctf] + [app.common.uuid :as uuid] [clojure.test :as t])) (defmethod cfm/migrate-data "test/1" [data _] (update data :sum inc)) @@ -26,3 +27,42 @@ file' (cfm/migrate file nil)] (t/is (= cfm/available-migrations (:migrations file'))) (t/is (= 3 (:sum (:data file')))))))) + +(t/deftest migration-0024-fix-stroke-cap-placement + (let [shape-id (uuid/next) + page-id (uuid/next) + data {:pages-index + {page-id + {:objects + {shape-id {:id shape-id + :type :path + :stroke-cap-start :round + :stroke-cap-end :round + :strokes [{:stroke-color "#000000" + :stroke-opacity 1 + :stroke-style :svg + :stroke-width 2}]}}}}} + data' (cfm/migrate-data data "0024-fix-stroke-cap-placement")] + + (let [shape (get-in data' [:pages-index page-id :objects shape-id])] + (t/is (nil? (:stroke-cap-start shape)) "top-level cap removed") + (t/is (nil? (:stroke-cap-end shape)) "top-level cap removed") + (t/is (= :round (get-in shape [:strokes 0 :stroke-cap-start])) "cap moved into stroke") + (t/is (= :round (get-in shape [:strokes 0 :stroke-cap-end])) "cap moved into stroke")))) + +(t/deftest migration-0024-fix-stroke-cap-no-strokes + (let [shape-id (uuid/next) + page-id (uuid/next) + data {:pages-index + {page-id + {:objects + {shape-id {:id shape-id + :type :path + :stroke-cap-start :round + :stroke-cap-end :round + :strokes []}}}}} + data' (cfm/migrate-data data "0024-fix-stroke-cap-placement")] + + (let [shape (get-in data' [:pages-index page-id :objects shape-id])] + (t/is (nil? (:stroke-cap-start shape)) "top-level cap removed even with no strokes") + (t/is (nil? (:stroke-cap-end shape)) "top-level cap removed even with no strokes"))))