From 77b9eee6bd54a6a4e1794d375de523512a0d84b4 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Fri, 21 Nov 2025 11:21:52 +0100 Subject: [PATCH] :bug: Fix svg fills defined in svg-attrs with url or color format --- CHANGES.md | 1 + .../get-file-large-blur-shadow.json | 2 +- frontend/src/app/render_wasm/api.cljs | 15 +- frontend/src/app/render_wasm/shape.cljs | 6 +- frontend/src/app/render_wasm/svg_fills.cljs | 344 ++++++++++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + .../test/frontend_tests/svg_fills_test.cljs | 66 ++++ 7 files changed, 423 insertions(+), 13 deletions(-) create mode 100644 frontend/src/app/render_wasm/svg_fills.cljs create mode 100644 frontend/test/frontend_tests/svg_fills_test.cljs diff --git a/CHANGES.md b/CHANGES.md index 25c4693a72..a4a4e15ea4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -70,6 +70,7 @@ penpot on-premise you will need to apply the same changes on your own ### :bug: Bugs fixed +- Ensure imported SVG shapes keep default fill when updating renders - Fix text line-height values are wrong [Taiga #12252](https://tree.taiga.io/project/penpot/issue/12252) - Fix an error translation [Taiga #12402](https://tree.taiga.io/project/penpot/issue/12402) - Fix pan cursor not disabling viewport guides [Github #6985](https://github.com/penpot/penpot/issues/6985) diff --git a/frontend/playwright/data/render-wasm/get-file-large-blur-shadow.json b/frontend/playwright/data/render-wasm/get-file-large-blur-shadow.json index a51c634fbe..745e35700d 100644 --- a/frontend/playwright/data/render-wasm/get-file-large-blur-shadow.json +++ b/frontend/playwright/data/render-wasm/get-file-large-blur-shadow.json @@ -887,4 +887,4 @@ "~:base-font-size": "16px" } } - } \ No newline at end of file + } diff --git a/frontend/src/app/render_wasm/api.cljs b/frontend/src/app/render_wasm/api.cljs index ed66b69d9a..dcae37fd02 100644 --- a/frontend/src/app/render_wasm/api.cljs +++ b/frontend/src/app/render_wasm/api.cljs @@ -34,6 +34,7 @@ [app.render-wasm.performance :as perf] [app.render-wasm.serializers :as sr] [app.render-wasm.serializers.color :as sr-clr] + [app.render-wasm.svg-fills :as svg-fills] ;; FIXME: rename; confunsing name [app.render-wasm.wasm :as wasm] [app.util.debug :as dbg] @@ -463,7 +464,7 @@ (h/call wasm/internal-module "_add_shape_stroke_fill"))))) strokes)) -(defn set-shape-path-attrs +(defn set-shape-svg-attrs [attrs] (let [style (:style attrs) ;; Filter to only supported attributes @@ -902,13 +903,7 @@ ;; inherited by child nodes and emulates the behavior of ;; standard SVG, where a node without an explicit fill ;; defaults to black. - fills (let [base-fills (get shape :fills)] - (if (and ^boolean (contains? shape :svg-attrs) - ^boolean (or ^boolean (= :svg-raw type) - ^boolean (= :group type)) - ^boolean (empty? base-fills)) - [{:fill-color "#000000" :fill-opacity 1}] - base-fills)) + fills (svg-fills/resolve-shape-fills shape) strokes (if (= type :group) [] (get shape :strokes)) @@ -948,9 +943,9 @@ (when (and (some? content) (or (= type :path) (= type :bool))) - (when (some? svg-attrs) - (set-shape-path-attrs svg-attrs)) (set-shape-path-content content)) + (when (some? svg-attrs) + (set-shape-svg-attrs svg-attrs)) (when (and (some? content) (= type :svg-raw)) (set-shape-svg-raw-content (get-static-markup shape))) (when (some? shadows) (set-shape-shadows shadows)) diff --git a/frontend/src/app/render_wasm/shape.cljs b/frontend/src/app/render_wasm/shape.cljs index 6ce1caf9c5..fbb45cd4a3 100644 --- a/frontend/src/app/render_wasm/shape.cljs +++ b/frontend/src/app/render_wasm/shape.cljs @@ -14,6 +14,7 @@ [app.common.types.shape.layout :as ctl] [app.main.refs :as refs] [app.render-wasm.api :as api] + [app.render-wasm.svg-fills :as svg-fills] [app.render-wasm.wasm :as wasm] [beicon.v2.core :as rx] [cljs.core :as c] @@ -162,7 +163,8 @@ (api/set-shape-transform v) :fills - (into [] (api/set-shape-fills id v false)) + (let [fills (svg-fills/resolve-shape-fills shape)] + (into [] (api/set-shape-fills id fills false))) :strokes (into [] (api/set-shape-strokes id v false)) @@ -221,7 +223,7 @@ :svg-attrs (when (cfh/path-shape? shape) - (api/set-shape-path-attrs v)) + (api/set-shape-svg-attrs v)) :masked-group (when (cfh/mask-shape? shape) diff --git a/frontend/src/app/render_wasm/svg_fills.cljs b/frontend/src/app/render_wasm/svg_fills.cljs new file mode 100644 index 0000000000..ba9b40d3a9 --- /dev/null +++ b/frontend/src/app/render_wasm/svg_fills.cljs @@ -0,0 +1,344 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns app.render-wasm.svg-fills + (:require + [app.common.data :as d] + [app.common.data.macros :as dm] + [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] + [app.common.svg :as csvg] + [app.common.types.color :as clr] + [clojure.string :as str])) + +(def ^:private url-fill-pattern + #"url\(\s*['\"]?#([^)'\"]+)['\"]?\s*\)") + +(defn- trim-fill-value + [value] + (let [string-value (cond + (string? value) value + (keyword? value) (name value) + (symbol? value) (name value) + (number? value) (str value) + (some? value) (str value) + :else nil)] + (when (some? string-value) + (let [trimmed (str/trim string-value)] + (when (seq trimmed) + trimmed))))) + +(defn- parse-length + [value] + (cond + (nil? value) nil + (number? value) (double value) + :else + (let [value (trim-fill-value value)] + (when (seq value) + (let [percent? (str/ends-with? value "%") + px? (str/ends-with? value "px") + numeric (cond + percent? (subs value 0 (dec (count value))) + px? (subs value 0 (- (count value) 2)) + :else value) + parsed (d/parse-double numeric)] + (when parsed + (if percent? + (/ parsed 100.0) + parsed))))))) + +(defn- parse-offset + [value] + (let [length (parse-length (or value 0))] + (-> (or length 0.0) + (max 0.0) + (min 1.0)))) + +(defn- parse-opacity + [value] + (let [parsed (parse-length value)] + (if (some? parsed) parsed 1.0))) + +(defn- shape->selrect + [shape] + (let [selrect (dm/get-prop shape :selrect)] + (cond + (grc/rect? selrect) selrect + (map? selrect) (grc/make-rect selrect) + :else (grc/make-rect {:x (or (dm/get-prop shape :x) 0) + :y (or (dm/get-prop shape :y) 0) + :width (max 0.01 (or (dm/get-prop shape :width) 1)) + :height (max 0.01 (or (dm/get-prop shape :height) 1))})))) + +(defn- normalize-point + [pt units shape] + (if (= units "userspaceonuse") + (let [rect (shape->selrect shape) + width (max 0.01 (dm/get-prop rect :width)) + height (max 0.01 (dm/get-prop rect :height)) + origin-x (or (dm/get-prop rect :x) (dm/get-prop rect :x1) 0) + origin-y (or (dm/get-prop rect :y) (dm/get-prop rect :y1) 0)] + (gpt/point (/ (- (dm/get-prop pt :x) origin-x) width) + (/ (- (dm/get-prop pt :y) origin-y) height))) + pt)) + +(defn- normalize-attrs + [attrs] + (into {} + (map (fn [[k v]] + (let [key (cond + (keyword? k) (name k) + (symbol? k) (name k) + :else (str k))] + [(keyword (str/lower-case key)) v]))) + (or attrs {}))) + +(defn- id-candidates + [id] + (let [base (cond + (string? id) id + (keyword? id) (name id) + (symbol? id) (name id) + (some? id) (str id) + :else nil) + lower (some-> base str/lower-case) + kebab (some-> base + (str/replace #"([a-z0-9])([A-Z])" "$1-$2") + str/lower-case)] + (->> [(when (string? id) id) + (when (keyword? id) id) + (when (symbol? id) id) + base + (when base (keyword base)) + (when base (symbol base)) + lower + (when lower (keyword lower)) + (when lower (symbol lower)) + kebab + (when kebab (keyword kebab)) + (when kebab (symbol kebab))] + (remove #(or (nil? %) (and (string? %) (str/blank? %)))) + distinct))) + +(defn- svg-def-by-id + [defs id] + (some #(get defs %) + (id-candidates id))) + +(defn- normalize-gradient-id + [value] + (when-let [clean (trim-fill-value value)] + (let [without (str/replace clean #"^#" "")] + (when (seq without) without)))) + +(defn- attr + "Returns the first matching value for the provided attribute keys." + [attrs & keys] + (some #(get attrs %) keys)) + +(defn- resolve-gradient-node + [shape gradient-id] + (let [defs (dm/get-prop shape :svg-defs)] + (when (and defs gradient-id) + (let [chain (loop [gid gradient-id + seen #{} + acc []] + (let [normalized (normalize-gradient-id gid)] + (if (or (nil? normalized) + (contains? seen normalized)) + acc + (if-let [node (svg-def-by-id defs normalized)] + (let [attrs (normalize-attrs (:attrs node)) + tag (let [raw (:tag node)] + (cond + (keyword? raw) raw + (string? raw) (keyword raw) + :else raw)) + content (:content node) + href (or (get attrs :xlinkhref) + (get attrs :xlink-href) + (get attrs :xlink:href) + (get attrs :href))] + (recur href + (conj seen normalized) + (conj acc {:tag tag + :attrs attrs + :content content}))) + acc))))] + (when (seq chain) + (let [combined + (reduce + (fn [result node] + (let [tag (or (:tag node) (:tag result)) + attrs (merge (:attrs result) (:attrs node)) + content (let [own (:content node)] + (if (seq own) + own + (:content result)))] + {:tag tag + :attrs attrs + :content content})) + {} (reverse chain))] + (when (contains? #{:linearGradient :radialGradient} (:tag combined)) + (let [result (update combined :content #(or % []))] + result)))))))) + +(defn- parse-gradient-stop + [stop-node] + (let [attrs (normalize-attrs (:attrs stop-node)) + style (some-> (get attrs :style) csvg/parse-style) + color-value (or (get attrs :stop-color) + (get attrs :stopcolor) + (get style :stop-color) + (get style :stopColor)) + color-value (trim-fill-value color-value) + color-value (if (= color-value "currentcolor") clr/black color-value) + color (when (clr/color-string? color-value) + (clr/parse color-value)) + opacity (or (get attrs :stop-opacity) + (get attrs :stopopacity) + (get style :stop-opacity) + (get style :stopOpacity)) + offset (or (get attrs :offset) "0")] + (when color + (d/without-nils {:color color + :opacity (some-> opacity parse-opacity) + :offset (parse-offset offset)})))) + +(defn- apply-gradient-transform + [points transform] + (if transform + (let [matrix (csvg/parse-transform transform)] + (mapv #(gpt/transform % matrix) points)) + points)) + +(defn- build-linear-gradient + [shape {:keys [attrs content]}] + (let [units (-> (or (attr attrs :gradientunits :gradient-units) "objectBoundingBox") + (str/lower-case)) + transform (attr attrs :gradienttransform :gradient-transform) + x1 (or (parse-length (or (get attrs :x1) (get attrs :x))) 0.0) + y1 (or (parse-length (or (get attrs :y1) (get attrs :y))) 0.0) + x2 (or (parse-length (or (get attrs :x2) (get attrs :x))) 1.0) + y2 (or (parse-length (or (get attrs :y2) (get attrs :y))) 0.0) + stops (->> content + (keep (fn [node] + (when (= (keyword (:tag node)) :stop) + (parse-gradient-stop node)))) + vec)] + (when (seq stops) + (let [points (apply-gradient-transform [(gpt/point x1 y1) + (gpt/point x2 y2)] + transform) + [start end] (map #(normalize-point % units shape) points)] + {:type :linear + :start-x (dm/get-prop start :x) + :start-y (dm/get-prop start :y) + :end-x (dm/get-prop end :x) + :end-y (dm/get-prop end :y) + :width 1 + :stops stops})))) + +(defn- build-radial-gradient + [shape {:keys [attrs content]}] + (let [units (-> (or (attr attrs :gradientunits :gradient-units) "objectBoundingBox") + (str/lower-case)) + transform (attr attrs :gradienttransform :gradient-transform) + cx (or (parse-length (or (get attrs :cx) (get attrs :fx))) 0.5) + cy (or (parse-length (or (get attrs :cy) (get attrs :fy))) 0.5) + r (or (parse-length (get attrs :r)) 0.5) + stops (->> content + (keep (fn [node] + (when (= (keyword (:tag node)) :stop) + (parse-gradient-stop node)))) + vec)] + (when (seq stops) + (let [[center radius-point] + (let [points (apply-gradient-transform [(gpt/point cx cy) + (gpt/point (+ cx r) cy)] + transform)] + (map #(normalize-point % units shape) points)) + radius (gpt/distance center radius-point)] + {:type :radial + :start-x (dm/get-prop center :x) + :start-y (dm/get-prop center :y) + :end-x (dm/get-prop radius-point :x) + :end-y (dm/get-prop radius-point :y) + :width radius + :stops stops})))) + +(defn- svg-gradient->fill + [shape value] + (let [trimmed (trim-fill-value value) + fill-str (cond + (string? trimmed) trimmed + (some? trimmed) (str trimmed) + :else nil)] + (when-let [gradient-id + (when (string? fill-str) + (some-> (re-matches url-fill-pattern fill-str) + (nth 1 nil)))] + (when-let [node (resolve-gradient-node shape gradient-id)] + (case (:tag node) + :linearGradient (build-linear-gradient shape node) + :radialGradient (build-radial-gradient shape node) + nil))))) + +(defn- parse-svg-fill + [shape value] + (when-let [trimmed (trim-fill-value value)] + (let [normalized (if (= (str/lower-case trimmed) "currentcolor") clr/black trimmed)] + (cond + (= normalized "none") nil + + (or (clr/color-string? normalized) + (keyword? value)) + {:type :color + :value (clr/parse normalized)} + + (str/starts-with? normalized "url(") + (when-let [gradient (svg-gradient->fill shape normalized)] + {:type :gradient + :value gradient}) + + :else nil)))) + +(defn svg-fill->fills + "Returns a sequence with a single fill derived from the shape SVG attrs/defs, + or nil when no SVG fill information can be inferred." + [shape] + (let [style-fill (parse-svg-fill shape (dm/get-in shape [:svg-attrs :style :fill])) + attr-fill (parse-svg-fill shape (dm/get-in shape [:svg-attrs :fill])) + {:keys [type value]} (or style-fill attr-fill)] + (when (some? type) + (let [opacity (or (some-> (dm/get-in shape [:svg-attrs :style :fillOpacity]) + (d/parse-double 1)) + (some-> (dm/get-in shape [:svg-attrs :fillOpacity]) + (d/parse-double 1))) + base-fill (case type + :color {:fill-color value} + :gradient {:fill-color-gradient value})] + [(cond-> base-fill + (some? opacity) (assoc :fill-opacity opacity))])))) + +(defn resolve-shape-fills + "Returns the fills that should be sent to WASM for the provided shape, + reusing existing fills when present, falling back to SVG derived fills, + and finally defaulting to the standard SVG black fill when needed." + [shape] + (let [base-fills (dm/get-prop shape :fills) + fallback (svg-fill->fills shape) + type (dm/get-prop shape :type)] + (cond + (seq base-fills) base-fills + (seq fallback) fallback + (and (contains? shape :svg-attrs) + (or (= :svg-raw type) + (= :group type))) + [{:fill-color "#000000" :fill-opacity 1}] + :else []))) + diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 85a7f310cb..704600a35f 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -11,6 +11,7 @@ [frontend-tests.logic.groups-test] [frontend-tests.logic.pasting-in-containers-test] [frontend-tests.plugins.context-shapes-test] + [frontend-tests.svg-fills-test] [frontend-tests.tokens.import-export-test] [frontend-tests.tokens.logic.token-actions-test] [frontend-tests.tokens.logic.token-data-test] @@ -39,6 +40,7 @@ 'frontend-tests.logic.groups-test 'frontend-tests.logic.pasting-in-containers-test 'frontend-tests.plugins.context-shapes-test + 'frontend-tests.svg-fills-test 'frontend-tests.tokens.import-export-test 'frontend-tests.tokens.logic.token-actions-test 'frontend-tests.tokens.logic.token-data-test diff --git a/frontend/test/frontend_tests/svg_fills_test.cljs b/frontend/test/frontend_tests/svg_fills_test.cljs new file mode 100644 index 0000000000..a2f202c943 --- /dev/null +++ b/frontend/test/frontend_tests/svg_fills_test.cljs @@ -0,0 +1,66 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.svg-fills-test + (:require + [app.render-wasm.svg-fills :as svg-fills] + [cljs.test :refer [deftest is testing]])) + +(def sample-shape + {:selrect {:x 100 :y 200 :width 200 :height 100} + :svg-attrs {:fillOpacity "0.5" + :style {:fill "url(#grad)"}} + :svg-defs {"grad" + {:tag :radialGradient + :attrs {:id "grad" + :gradientUnits "userSpaceOnUse" + :cx "150" + :cy "250" + :r "50"} + :content [{:tag :stop + :attrs {:offset "0" + :style "stop-color:#ff0000;stop-opacity:1"}} + {:tag :stop + :attrs {:offset "1" + :style "stop-color:#00ff00;stop-opacity:0"}}]}}}) + +(deftest builds-gradient-fill-from-svg-defs + (let [fills (svg-fills/svg-fill->fills sample-shape) + gradient (get-in (first fills) [:fill-color-gradient])] + (testing "fallback fill is generated" + (is (= 1 (count fills)))) + (testing "gradient metadata" + (is (= :radial (:type gradient))) + (is (= "#ff0000" (get-in gradient [:stops 0 :color]))) + (is (= "#00ff00" (get-in gradient [:stops 1 :color])))) + (testing "opacity preserved" + (is (= 0.5 (:fill-opacity (first fills))))))) + +(deftest skips-when-no-svg-fill + (is (nil? (svg-fills/svg-fill->fills {:svg-attrs {:fill "none"}})))) + +(deftest resolve-shape-fills-prefers-existing-fills + (let [fills [{:fill-color "#ff00ff" :fill-opacity 0.75}] + resolved (svg-fills/resolve-shape-fills {:fills fills})] + (is (= fills resolved)))) + +(deftest resolve-shape-fills-falls-back-to-svg-fill + (let [resolved (svg-fills/resolve-shape-fills (assoc sample-shape :fills []))] + (is (= (svg-fills/svg-fill->fills sample-shape) resolved)))) + +(deftest resolve-shape-fills-defaults-to-black + (is (= [{:fill-color "#000000" :fill-opacity 1}] + (svg-fills/resolve-shape-fills {:type :group + :svg-attrs {}})))) + +(deftest resolve-shape-fills-accepts-hex-fill + (let [fills (svg-fills/resolve-shape-fills {:fills [] + :type :svg-raw + :svg-attrs {:fill "#fabada"}})] + (is (= 1 (count fills))) + (is (= "#fabada" (:fill-color (first fills)))))) + +