From 281251ff87b65a927400fac52b4ed606ccefec8f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 12 Sep 2023 11:48:03 +0200 Subject: [PATCH 1/7] :zap: Add minor optimizations to rect shape --- frontend/src/app/main/ui/shapes/rect.cljs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/frontend/src/app/main/ui/shapes/rect.cljs b/frontend/src/app/main/ui/shapes/rect.cljs index 722ad5573b..1ce1dd0452 100644 --- a/frontend/src/app/main/ui/shapes/rect.cljs +++ b/frontend/src/app/main/ui/shapes/rect.cljs @@ -6,7 +6,9 @@ (ns app.main.ui.shapes.rect (:require + [app.common.data.macros :as dm] [app.common.geom.shapes :as gsh] + [app.main.ui.context :as muc] [app.main.ui.shapes.attrs :as attrs] [app.main.ui.shapes.custom-stroke :refer [shape-custom-strokes]] [app.util.object :as obj] @@ -16,16 +18,18 @@ {::mf/wrap-props false} [props] (let [shape (unchecked-get props "shape") - {:keys [x y width height]} shape - transform (gsh/transform-str shape) - props (-> (attrs/extract-style-attrs shape) - (obj/merge! - #js {:x x - :y y - :transform transform - :width width - :height height})) + x (dm/get-prop shape :x) + y (dm/get-prop shape :y) + w (dm/get-prop shape :width) + h (dm/get-prop shape :height) + + t (gsh/transform-str shape) + rid (mf/use-ctx muc/render-id) + + props (mf/with-memo [shape rid] + (-> (attrs/extract-style-attrs shape rid) + (obj/merge! #js {:x x :y y :transform t :width w :height h}))) path? (some? (.-d props))] From 85a1f7d69e4d00ac1147ebab985b69096142f178 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 12 Sep 2023 12:02:25 +0200 Subject: [PATCH 2/7] :zap: Add minor optimizations to fills component (shapes) --- frontend/src/app/main/ui/shapes/fills.cljs | 74 +++++++++++++--------- 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/frontend/src/app/main/ui/shapes/fills.cljs b/frontend/src/app/main/ui/shapes/fills.cljs index 9ba40a95a3..c0a946df4c 100644 --- a/frontend/src/app/main/ui/shapes/fills.cljs +++ b/frontend/src/app/main/ui/shapes/fills.cljs @@ -22,45 +22,57 @@ {::mf/wrap-props false} [props] - (let [shape (obj/get props "shape") - render-id (obj/get props "render-id")] + (let [shape (unchecked-get props "shape") + render-id (unchecked-get props "render-id") - (when (or (some? (:fill-image shape)) - (#{:image :text} (:type shape)) - (> (count (:fills shape)) 1) - (some :fill-color-gradient (:fills shape))) + shape-type (dm/get-prop shape :type) + fill-image (:fill-image shape) + shape-fills (:fills shape [])] - (let [{:keys [x y width height]} (:selrect shape) - {:keys [metadata]} shape + (when (or (some? fill-image) + (or (= shape-type :image) + (= shape-type :text)) + (> (count shape-fills) 1) + (some :fill-color-gradient shape-fills)) - has-image? (or metadata (:fill-image shape)) + (let [selrect (dm/get-prop shape :selrect) + metadata (get shape :metadata) + x (dm/get-prop selrect :x) + y (dm/get-prop selrect :y) + width (dm/get-prop selrect :width) + height (dm/get-prop selrect :height) - uri (cond - metadata - (cfg/resolve-file-media metadata) + has-image? (or (some? metadata) + (some? fill-image)) - (:fill-image shape) - (cfg/resolve-file-media (:fill-image shape))) + uri (cond + (some? metadata) + (cfg/resolve-file-media metadata) - embed (embed/use-data-uris [uri]) - transform (gsh/transform-str shape) + (some? fill-image) + (cfg/resolve-file-media fill-image)) + + embed (embed/use-data-uris [uri]) + transform (gsh/transform-str shape) ;; When true the image has not loaded yet - loading? (and (some? uri) (not (contains? embed uri))) + loading? (and (some? uri) + (not (contains? embed uri))) - pattern-attrs (cond-> #js {:patternUnits "userSpaceOnUse" - :x x - :y y - :height height - :width width - :data-loading loading?} - (= :path (:type shape)) - (obj/set! "patternTransform" transform)) - type (:type shape)] + pat-props #js {:patternUnits "userSpaceOnUse" + :x x + :y y + :width width + :height height + :data-loading loading?} + + pat-props (if (= :path shape-type) + (obj/set! pat-props "patternTransform" transform) + pat-props)] (for [[shape-index shape] (d/enumerate (or (:position-data shape) [shape]))] [:* {:key (dm/str shape-index)} - (for [[fill-index value] (-> (d/enumerate (:fills shape [])) reverse)] + (for [[fill-index value] (reverse (d/enumerate shape-fills))] (when (some? (:fill-color-gradient value)) (let [gradient (:fill-color-gradient value) props #js {:id (dm/str "fill-color-gradient_" render-id "_" fill-index) @@ -73,21 +85,21 @@ (let [fill-id (dm/str "fill-" shape-index "-" render-id)] - [:> :pattern (-> (obj/clone pattern-attrs) + [:> :pattern (-> (obj/clone pat-props) (obj/set! "id" fill-id) (cond-> has-image? (-> (obj/set! "width" (* width no-repeat-padding)) (obj/set! "height" (* height no-repeat-padding))))) [:g - (for [[fill-index value] (-> (d/enumerate (:fills shape [])) reverse)] - (let [style (attrs/get-fill-style value fill-index render-id type) + (for [[fill-index value] (reverse (d/enumerate shape-fills))] + (let [style (attrs/get-fill-style value fill-index render-id shape-type) props #js {:key (dm/str fill-index) :width width :height height :style style}] [:> :rect props])) - (when has-image? + (when ^boolean has-image? [:g ;; We add this shape to add a padding so the patter won't repeat ;; Issue: https://tree.taiga.io/project/penpot/issue/5583 From b1e54a97148aacba72b142b64e46543caf06a222 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 12 Sep 2023 13:59:32 +0200 Subject: [PATCH 3/7] :sparkles: Pass explicitly the render-id on props handling in path and svg-raw shapes --- frontend/src/app/main/ui/shapes/path.cljs | 4 +++- frontend/src/app/main/ui/shapes/svg_raw.cljs | 11 +++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/main/ui/shapes/path.cljs b/frontend/src/app/main/ui/shapes/path.cljs index 031b0458b9..20fbcde0d2 100644 --- a/frontend/src/app/main/ui/shapes/path.cljs +++ b/frontend/src/app/main/ui/shapes/path.cljs @@ -7,6 +7,7 @@ (ns app.main.ui.shapes.path (:require [app.common.logging :as log] + [app.main.ui.context :as muc] [app.main.ui.shapes.attrs :as attrs] [app.main.ui.shapes.custom-stroke :refer [shape-custom-strokes]] [app.util.object :as obj] @@ -28,7 +29,8 @@ :cause e) ""))) - props (-> (attrs/extract-style-attrs shape) + render-id (mf/use-ctx muc/render-id) + props (-> (attrs/extract-style-attrs shape render-id) (obj/set! "d" pdata))] [:& shape-custom-strokes {:shape shape} diff --git a/frontend/src/app/main/ui/shapes/svg_raw.cljs b/frontend/src/app/main/ui/shapes/svg_raw.cljs index 6a744745be..ad482efed9 100644 --- a/frontend/src/app/main/ui/shapes/svg_raw.cljs +++ b/frontend/src/app/main/ui/shapes/svg_raw.cljs @@ -8,6 +8,7 @@ (:require [app.common.data.macros :as dm] [app.common.geom.shapes :as gsh] + [app.main.ui.context :as muc] [app.main.ui.shapes.attrs :as usa] [app.util.object :as obj] [app.util.svg :as usvg] @@ -20,8 +21,8 @@ ;; Context to store a re-mapping of the ids (def svg-ids-ctx (mf/create-context nil)) -(defn set-styles [attrs shape] - (let [custom-attrs (-> (usa/extract-style-attrs shape) +(defn set-styles [attrs shape render-id] + (let [custom-attrs (-> (usa/extract-style-attrs shape render-id) (obj/without ["transform"])) attrs (or attrs {}) @@ -51,8 +52,9 @@ {:keys [attrs] :as content} (:content shape) ids-mapping (mf/use-memo #(usvg/generate-id-mapping content)) + render-id (mf/use-ctx muc/render-id) - attrs (-> (set-styles attrs shape) + attrs (-> (set-styles attrs shape render-id) (obj/set! "x" x) (obj/set! "y" y) (obj/set! "width" width) @@ -73,12 +75,13 @@ {:keys [attrs tag]} content ids-mapping (mf/use-ctx svg-ids-ctx) + render-id (mf/use-ctx muc/render-id) attrs (mf/use-memo #(usvg/replace-attrs-ids attrs ids-mapping)) attrs (translate-shape attrs shape) element-id (get-in content [:attrs :id]) - attrs (cond-> (set-styles attrs shape) + attrs (cond-> (set-styles attrs shape render-id) (and element-id (contains? ids-mapping element-id)) (obj/set! "id" (get ids-mapping element-id)))] [:> (name tag) attrs children])) From e6f8022de0cf03fe076145ab960aaea658e931b9 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 12 Sep 2023 14:00:18 +0200 Subject: [PATCH 4/7] :sparkles: Add obj/array? helper --- frontend/src/app/main/ui/shapes/shape.cljs | 2 +- frontend/src/app/util/object.cljs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/main/ui/shapes/shape.cljs b/frontend/src/app/main/ui/shapes/shape.cljs index 659ce9f238..0cb212a8ab 100644 --- a/frontend/src/app/main/ui/shapes/shape.cljs +++ b/frontend/src/app/main/ui/shapes/shape.cljs @@ -42,7 +42,7 @@ (defn propagate-wrapper-styles ([children wrapper-props] - (if (.isArray js/Array children) + (if ^boolean (obj/array? children) (->> children (map #(propagate-wrapper-styles-child % wrapper-props))) (-> children (propagate-wrapper-styles-child wrapper-props))))) diff --git a/frontend/src/app/util/object.cljs b/frontend/src/app/util/object.cljs index 4c6d319814..c9caf8f2b9 100644 --- a/frontend/src/app/util/object.cljs +++ b/frontend/src/app/util/object.cljs @@ -6,11 +6,15 @@ (ns app.util.object "A collection of helpers for work with javascript objects." - (:refer-clojure :exclude [set! new get get-in merge clone contains?]) + (:refer-clojure :exclude [set! new get get-in merge clone contains? array?]) (:require ["lodash/omit" :as omit] [cuerdas.core :as str])) +(defn array? + [o] + (.isArray js/Array o)) + (defn create [] #js {}) (defn get From 385fd9c4e677c3ee0de2a527e29c3d1784128eeb Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 12 Sep 2023 16:01:10 +0200 Subject: [PATCH 5/7] :recycle: Refactor shape attrs extraction helpers --- frontend/src/app/main/ui/shapes/attrs.cljs | 327 +++++++++--------- frontend/src/app/main/ui/shapes/circle.cljs | 2 +- .../src/app/main/ui/shapes/custom_stroke.cljs | 20 +- frontend/src/app/main/ui/shapes/fills.cljs | 2 + frontend/src/app/main/ui/shapes/frame.cljs | 4 +- frontend/src/app/main/ui/shapes/image.cljs | 28 +- frontend/src/app/main/ui/shapes/path.cljs | 2 +- frontend/src/app/main/ui/shapes/rect.cljs | 2 +- frontend/src/app/main/ui/shapes/shape.cljs | 5 +- frontend/src/app/main/ui/shapes/svg_raw.cljs | 4 +- .../src/app/main/ui/shapes/text/fo_text.cljs | 2 +- .../src/app/main/ui/shapes/text/svg_text.cljs | 3 +- .../src/app/main/ui/workspace/shapes.cljs | 4 +- .../main/ui/workspace/viewport/outline.cljs | 2 +- frontend/src/app/util/object.cljs | 35 +- 15 files changed, 231 insertions(+), 211 deletions(-) diff --git a/frontend/src/app/main/ui/shapes/attrs.cljs b/frontend/src/app/main/ui/shapes/attrs.cljs index cbd5e81fd1..c44d6a9152 100644 --- a/frontend/src/app/main/ui/shapes/attrs.cljs +++ b/frontend/src/app/main/ui/shapes/attrs.cljs @@ -12,11 +12,9 @@ [app.common.geom.shapes :as gsh] [app.common.types.shape :refer [stroke-caps-line stroke-caps-marker]] [app.common.types.shape.radius :as ctsr] - [app.main.ui.context :as muc] [app.util.object :as obj] [app.util.svg :as usvg] - [cuerdas.core :as str] - [rumext.v2 :as mf])) + [cuerdas.core :as str])) (defn- stroke-type->dasharray [width style] @@ -29,7 +27,8 @@ (->> values (map #(+ % width)) (str/join ",")))) -(defn extract-border-radius [{:keys [x y width height] :as shape}] +(defn get-border-radius + [shape] (case (ctsr/radius-mode shape) :radius-1 (let [radius (gsh/shape-corners-1 shape)] @@ -37,6 +36,10 @@ :radius-4 (let [[r1 r2 r3 r4] (gsh/shape-corners-4 shape) + x (dm/get-prop shape :x) + y (dm/get-prop shape :y) + width (dm/get-prop shape :width) + height (dm/get-prop shape :height) top (- width r1 r2) right (- height r2 r3) bottom (- width r3 r4) @@ -53,208 +56,198 @@ "a" r1 "," r1 " 0 0 1 " r1 "," (- r1) " " "z")}))) +(defn add-border-props! + [props shape] + (obj/merge! props (get-border-radius shape))) -(defn add-border-radius [attrs shape] - (obj/merge! attrs (extract-border-radius shape))) +(defn add-fill! + [attrs fill-data render-id index type] + (let [index (if (some? index) (dm/str "_" index) "")] + (cond + (contains? fill-data :fill-image) + (let [id (dm/str "fill-image-" render-id)] + (obj/set! attrs "fill" (dm/str "url(#" id ")"))) -(defn add-fill - ([attrs fill-data render-id type] - (add-fill attrs fill-data render-id nil type)) + (some? (:fill-color-gradient fill-data)) + (let [id (dm/str "fill-color-gradient_" render-id index)] + (obj/set! attrs "fill" (dm/str "url(#" id ")"))) - ([attrs fill-data render-id index type] - (let [fill-attrs - (cond - (contains? fill-data :fill-image) - (let [fill-image-id (str "fill-image-" render-id)] - {:fill (str "url(#" fill-image-id ")")}) + (contains? fill-data :fill-color) + (obj/set! attrs "fill" (:fill-color fill-data)) - (and (contains? fill-data :fill-color-gradient) (some? (:fill-color-gradient fill-data))) - (let [fill-color-gradient-id (str "fill-color-gradient_" render-id (if index (str "_" index) ""))] - {:fill (str "url(#" fill-color-gradient-id ")")}) + :else + (obj/set! attrs "fill" "none")) - (contains? fill-data :fill-color) - {:fill (:fill-color fill-data)} + (when (contains? fill-data :fill-opacity) + (obj/set! attrs "fillOpacity" (:fill-opacity fill-data))) - :else - {:fill "none"}) + (when (and (= :text type) + (nil? (:fill-color-gradient fill-data)) + (nil? (:fill-color fill-data))) + (obj/set! attrs "fill" "black")) - fill-attrs (cond-> fill-attrs - (contains? fill-data :fill-opacity) - (assoc :fillOpacity (:fill-opacity fill-data)) + attrs)) - ;; Old texts with only an opacity set are black by default - (and (= type :text) (nil? (:fill-color-gradient fill-data)) (nil? (:fill-color fill-data))) - (assoc :fill "black"))] +(defn add-stroke! + [attrs data render-id index] + (let [style (:stroke-style data :solid)] + (when-not (= style :none) + (let [width (:stroke-width data 1) + gradient (:stroke-color-gradient data) + color (:stroke-color data) + opacity (:stroke-opacity data)] - (obj/merge! attrs (clj->js fill-attrs))))) + (obj/set! attrs "strokeWidth" width) -(defn add-stroke [attrs stroke-data render-id index] - (let [stroke-style (:stroke-style stroke-data :solid) - stroke-color-gradient-id (str "stroke-color-gradient_" render-id "_" index) - stroke-width (:stroke-width stroke-data 1)] - (if (not= stroke-style :none) - (let [stroke-attrs - (cond-> {:strokeWidth stroke-width} - (:stroke-color-gradient stroke-data) - (assoc :stroke (str/format "url(#%s)" stroke-color-gradient-id)) + (when (some? gradient) + (let [gradient-id (dm/str "stroke-color-gradient_" render-id "_" index)] + (obj/set! attrs "stroke" (str/ffmt "url(#%)" gradient-id)))) - (and (not (:stroke-color-gradient stroke-data)) - (:stroke-color stroke-data nil)) - (assoc :stroke (:stroke-color stroke-data nil)) + (when-not (some? gradient) + (when (some? color) + (obj/set! attrs "stroke" color)) + (when (some? opacity) + (obj/set! attrs "strokeOpacity" opacity))) - (and (not (:stroke-color-gradient stroke-data)) - (:stroke-opacity stroke-data nil)) - (assoc :strokeOpacity (:stroke-opacity stroke-data nil)) + (when (not= style :svg) + (obj/set! attrs "strokeDasharray" (stroke-type->dasharray width style))) - (not= stroke-style :svg) - (assoc :strokeDasharray (stroke-type->dasharray stroke-width stroke-style)) + ;; For simple line caps we use svg stroke-line-cap attribute. This + ;; only works if all caps are the same and we are not using the tricks + ;; for inner or outer strokes. + (let [caps-start (:stroke-cap-start data) + caps-end (:stroke-cap-end data) + alignment (:stroke-alignment data)] + (cond + (and (contains? stroke-caps-line caps-start) + (= caps-start caps-end) + (not= :inner alignment) + (not= :outer alignment) + (not= :dotted style)) + (obj/set! attrs "strokeLinecap" caps-start) - ;; For simple line caps we use svg stroke-line-cap attribute. This - ;; only works if all caps are the same and we are not using the tricks - ;; for inner or outer strokes. - (and (stroke-caps-line (:stroke-cap-start stroke-data)) - (= (:stroke-cap-start stroke-data) (:stroke-cap-end stroke-data)) - (not (#{:inner :outer} (:stroke-alignment stroke-data))) - (not= :dotted stroke-style)) - (assoc :strokeLinecap (:stroke-cap-start stroke-data)) + (= :dotted style) + (obj/set! attrs "strokeLinecap" "round")) - (= :dotted stroke-style) - (assoc :strokeLinecap "round") + (when (and (not= :inner alignment) + (not= :outer alignment)) - ;; For other cap types we use markers. - (and (or (stroke-caps-marker (:stroke-cap-start stroke-data)) - (and (stroke-caps-line (:stroke-cap-start stroke-data)) - (not= (:stroke-cap-start stroke-data) (:stroke-cap-end stroke-data)))) - (not (#{:inner :outer} (:stroke-alignment stroke-data)))) - (assoc :markerStart - (str/format "url(#marker-%s-%s)" render-id (name (:stroke-cap-start stroke-data)))) + ;; For other cap types we use markers. + (when (or (contains? stroke-caps-marker caps-start) + (and (contains? stroke-caps-line caps-start) + (not= caps-start caps-end))) + (obj/set! attrs "markerStart" (str/ffmt "url(#marker-%-%)" render-id (name caps-start)))) - (and (or (stroke-caps-marker (:stroke-cap-end stroke-data)) - (and (stroke-caps-line (:stroke-cap-end stroke-data)) - (not= (:stroke-cap-start stroke-data) (:stroke-cap-end stroke-data)))) - (not (#{:inner :outer} (:stroke-alignment stroke-data)))) - (assoc :markerEnd - (str/format "url(#marker-%s-%s)" render-id (name (:stroke-cap-end stroke-data)))))] + (when (or (contains? stroke-caps-marker caps-end) + (and (contains? stroke-caps-line caps-end) + (not= caps-start caps-end))) + (obj/set! attrs "markerEnd" (str/ffmt "url(#marker-%-%)" render-id (name caps-end)))))))) - (obj/merge! attrs (clj->js stroke-attrs))) - attrs))) + attrs)) -(defn add-layer-props [attrs shape] - (cond-> attrs - (:opacity shape) - (obj/set! "opacity" (:opacity shape)))) +(defn add-layer-props! + [props shape] + (let [opacity (:opacity shape)] + (if (some? opacity) + (obj/set! props "opacity" opacity) + props))) -;; FIXME: DEPRECATED -(defn extract-svg-attrs - [render-id svg-defs svg-attrs] - (if (and (empty? svg-defs) (empty? svg-attrs)) - [#js {} #js {}] - (let [replace-id (fn [id] - (if (contains? svg-defs id) - (str render-id "-" id) - id)) - svg-attrs (-> svg-attrs - (usvg/clean-attrs) - (usvg/update-attr-ids replace-id) - (dissoc :id)) - - attrs (-> svg-attrs (dissoc :style) (clj->js)) - styles (-> svg-attrs (get :style {}) (clj->js))] - - [attrs styles]))) - -(defn get-svg-attrs +(defn get-svg-props [shape render-id] - (let [svg-attrs (get shape :svg-attrs {}) - svg-defs (get shape :svg-defs {})] - (if (and (empty? svg-defs) - (empty? svg-attrs)) - {} - (let [replace-id (fn [id] - (if (contains? svg-defs id) - (str render-id "-" id) - id))] - (-> svg-attrs - (usvg/clean-attrs) - (usvg/update-attr-ids replace-id) - (dissoc :id)))))) + (let [attrs (get shape :svg-attrs {}) + defs (get shape :svg-defs {})] + (if (and (empty? defs) + (empty? attrs)) + #js {} + (-> attrs + ;; TODO: revisit, why we need to execute it each render? Can + ;; we do this operation on importation and avoid unnecesary + ;; work on render? + (usvg/clean-attrs) + (usvg/update-attr-ids + (fn [id] + (if (contains? defs id) + (str render-id "-" id) + id))) + (dissoc :id) + (obj/map->obj))))) -(defn add-style-attrs - ([props shape] - (let [render-id (mf/use-ctx muc/render-id)] - (add-style-attrs props shape render-id))) +(defn add-fill-props! + [props shape render-id] + (let [svg-attrs (get-svg-props shape render-id) + svg-style (obj/get svg-attrs "style") - ([props shape render-id] - (let [svg-defs (:svg-defs shape {}) - svg-attrs (:svg-attrs shape {}) + shape-type (dm/get-prop shape :type) - [svg-attrs svg-styles] - (extract-svg-attrs render-id svg-defs svg-attrs) + shape-fills (get shape :fills []) + fill-image (get shape :fill-image) - styles (-> (obj/get props "style" (obj/create)) - (obj/merge! svg-styles) - (add-layer-props shape)) + style (-> (obj/get props "style" #js {}) + (obj/merge! svg-style) + (add-layer-props! shape))] - styles (cond (or (some? (:fill-image shape)) - (= :image (:type shape)) - (> (count (:fills shape)) 1) - (some #(some? (:fill-color-gradient %)) (:fills shape))) - (obj/set! styles "fill" (str "url(#fill-0-" render-id ")")) + (cond + (or (some? fill-image) + (= :image shape-type) + (> (count shape-fills) 1) + (some #(some? (:fill-color-gradient %)) shape-fills)) + (obj/set! style "fill" (dm/str "url(#fill-0-" render-id ")")) - ;; imported svgs can have fill and fill-opacity attributes - (and (some? svg-styles) (obj/contains? svg-styles "fill")) - (-> styles - (obj/set! "fill" (obj/get svg-styles "fill")) - (obj/set! "fillOpacity" (obj/get svg-styles "fillOpacity"))) + ;; imported svgs can have fill and fill-opacity attributes + (contains? svg-style "fill") + (-> style + (obj/set! "fill" (obj/get svg-style "fill")) + (obj/set! "fillOpacity" (obj/get svg-style "fillOpacity"))) - (and (some? svg-attrs) (obj/contains? svg-attrs "fill")) - (-> styles - (obj/set! "fill" (obj/get svg-attrs "fill")) - (obj/set! "fillOpacity" (obj/get svg-attrs "fillOpacity"))) + (obj/contains? svg-attrs "fill") + (-> style + (obj/set! "fill" (obj/get svg-attrs "fill")) + (obj/set! "fillOpacity" (obj/get svg-attrs "fillOpacity"))) - ;; If the shape comes from an imported SVG (we know because it has - ;; the :svg-attrs atribute), and it does not have an own fill, we - ;; set a default black fill. This will be inherited by child nodes, - ;; and is for emulating the behavior of standard SVG, in that a node - ;; that has no explicit fill has a default fill of black. - ;; This may be reset to normal if a Penpot frame shape appears below - ;; (see main.ui.shapes.frame/frame-container). - (and (contains? shape :svg-attrs) - (#{:svg-raw :group} (:type shape)) - (empty? (:fills shape))) - (-> styles - (obj/set! "fill" (or (obj/get (:wrapper-styles shape) "fill") clr/black))) + ;; If the shape comes from an imported SVG (we know because + ;; it has the :svg-attrs atribute), and it does not have an + ;; own fill, we set a default black fill. This will be + ;; inherited by child nodes, and is for emulating the + ;; behavior of standard SVG, in that a node that has no + ;; explicit fill has a default fill of black. This may be + ;; reset to normal if a Penpot frame shape appears below + ;; (see main.ui.shapes.frame/frame-container). + (and (contains? shape :svg-attrs) + (or (= :svg-raw shape-type) + (= :group shape-type)) + (empty? shape-fills)) + (let [wstyle (get shape :wrapper-styles) + fill (obj/get wstyle "fill") + fill (d/nilv fill clr/black)] + (obj/set! style "fill" fill)) - (d/not-empty? (:fills shape)) - (add-fill styles (d/without-nils (get-in shape [:fills 0])) render-id 0 (:type shape)) + (d/not-empty? shape-fills) + (let [fill (d/without-nils (nth shape-fills 0))] + (add-fill! style fill render-id 0 shape-type))) - :else - styles)] + (-> props + (obj/merge! svg-attrs) + (obj/set! "style" style)))) - (-> props - (obj/merge! svg-attrs) - (add-border-radius shape) - (obj/set! "style" styles))))) - -(defn extract-style-attrs - ([shape] - (-> (obj/create) - (add-style-attrs shape))) - ([shape render-id] - (-> (obj/create) - (add-style-attrs shape render-id)))) +(defn get-style-props + [shape render-id] + (-> #js {} + (add-fill-props! shape render-id) + (add-border-props! shape))) (defn get-stroke-style [stroke-data index render-id] - ;; FIXME: optimize - (add-stroke #js {} stroke-data render-id index)) + (add-stroke! #js {} stroke-data render-id index)) (defn get-fill-style [fill-data index render-id type] - ;; FIXME: optimize - (add-fill #js {} fill-data render-id index type)) + (add-fill! #js {} fill-data render-id index type)) (defn extract-border-radius-attrs [shape] (-> (obj/create) - (add-border-radius shape))) + (add-border-props! shape))) + +(defn get-border-radius-props + [shape] + (add-border-props! #js {} shape)) diff --git a/frontend/src/app/main/ui/shapes/circle.cljs b/frontend/src/app/main/ui/shapes/circle.cljs index a1cd362f36..367e0854d4 100644 --- a/frontend/src/app/main/ui/shapes/circle.cljs +++ b/frontend/src/app/main/ui/shapes/circle.cljs @@ -34,7 +34,7 @@ rid (mf/use-ctx muc/render-id) props (mf/with-memo [shape] - (-> (attrs/extract-style-attrs shape rid) + (-> (attrs/get-style-props shape rid) (obj/merge! #js {:cx cx :cy cy :rx rx :ry ry :transform t})))] [:& shape-custom-strokes {:shape shape} diff --git a/frontend/src/app/main/ui/shapes/custom_stroke.cljs b/frontend/src/app/main/ui/shapes/custom_stroke.cljs index fa40851587..f7d6efdeb0 100644 --- a/frontend/src/app/main/ui/shapes/custom_stroke.cljs +++ b/frontend/src/app/main/ui/shapes/custom_stroke.cljs @@ -394,8 +394,8 @@ (obj/set! props "filter" (dm/fmt "url(#filter_%)" render-id)) props)) - svg-attrs (attrs/get-svg-attrs shape render-id) - svg-styles (get svg-attrs :style {})] + svg-attrs (attrs/get-svg-props shape render-id) + svg-styles (obj/get svg-attrs "style")] (cond ^boolean url-fill? @@ -404,22 +404,22 @@ (obj/unset! style "fillOpacity") (obj/set! props "fill" (dm/fmt "url(#fill-%-%)" position render-id))) - (and ^boolean (or (contains? svg-styles :fill) - (contains? svg-styles :fillOpacity)) + (and ^boolean (or (obj/contains? svg-styles "fill") + (obj/contains? svg-styles "fillOpacity")) ^boolean (obj/contains? svg-styles "fill")) - (let [fill (get svg-styles :fill) - opacity (get svg-styles :fillOpacity)] + (let [fill (obj/get svg-styles "fill") + opacity (obj/get svg-styles "fillOpacity")] (when (some? fill) (obj/set! style "fill" fill)) (when (some? opacity) (obj/set! style "fillOpacity" opacity))) - (and ^boolean (or (contains? svg-attrs :fill) - (contains? svg-attrs :fillOpacity)) + (and ^boolean (or (obj/contains? svg-attrs "fill") + (obj/contains? svg-attrs "fillOpacity")) ^boolean (empty? shape-fills)) - (let [fill (get svg-attrs :fill) - opacity (get svg-attrs :fillOpacity)] + (let [fill (obj/get svg-attrs "fill") + opacity (obj/get svg-attrs "fillOpacity")] (when (some? fill) (obj/set! style "fill" fill)) (when (some? opacity) diff --git a/frontend/src/app/main/ui/shapes/fills.cljs b/frontend/src/app/main/ui/shapes/fills.cljs index c0a946df4c..eb46446434 100644 --- a/frontend/src/app/main/ui/shapes/fills.cljs +++ b/frontend/src/app/main/ui/shapes/fills.cljs @@ -18,6 +18,8 @@ (def no-repeat-padding 1.05) +;; FIXME: this component breaks hooks rules + (mf/defc fills {::mf/wrap-props false} [props] diff --git a/frontend/src/app/main/ui/shapes/frame.cljs b/frontend/src/app/main/ui/shapes/frame.cljs index 2ad080c4fe..77a223bc40 100644 --- a/frontend/src/app/main/ui/shapes/frame.cljs +++ b/frontend/src/app/main/ui/shapes/frame.cljs @@ -43,7 +43,7 @@ t (gsh/transform-str shape) props (mf/with-memo [shape render-id] - (-> (attrs/extract-style-attrs shape render-id) + (-> (attrs/get-style-props shape render-id) (obj/merge! #js {:x x :y y :width w :height h :transform t}))) path? (some? (.-d props))] @@ -74,7 +74,7 @@ show-content? (get shape :show-content) props (mf/with-memo [shape render-id] - (-> (attrs/extract-style-attrs shape render-id) + (-> (attrs/get-style-props shape render-id) (obj/merge! #js {:x x :y y diff --git a/frontend/src/app/main/ui/shapes/image.cljs b/frontend/src/app/main/ui/shapes/image.cljs index cb201846ce..0288b11ebe 100644 --- a/frontend/src/app/main/ui/shapes/image.cljs +++ b/frontend/src/app/main/ui/shapes/image.cljs @@ -6,6 +6,7 @@ (ns app.main.ui.shapes.image (:require + [app.common.data.macros :as dm] [app.common.geom.shapes :as gsh] [app.main.ui.context :as muc] [app.main.ui.shapes.attrs :as attrs] @@ -17,22 +18,25 @@ {::mf/wrap-props false} [props] - (let [shape (unchecked-get props "shape") - {:keys [x y width height]} shape + (let [shape (unchecked-get props "shape") + + x (dm/get-prop shape :x) + y (dm/get-prop shape :y) + w (dm/get-prop shape :width) + h (dm/get-prop shape :height) render-id (mf/use-ctx muc/render-id) transform (gsh/transform-str shape) - props (-> (attrs/extract-style-attrs shape render-id) - (obj/merge! (attrs/extract-border-radius-attrs shape)) - (obj/merge! - #js {:x x - :y y - :transform transform - :width width - :height height})) - path? (some? (.-d props))] + + props (mf/with-memo [shape render-id] + (-> #js {} + (attrs/add-fill-props! shape render-id) + (attrs/add-border-props! shape) + (obj/merge! #js {:x x :y y :width w :height h :transform transform}))) + + path? (some? (.-d props))] [:& shape-custom-strokes {:shape shape} - (if path? + (if ^boolean path? [:> :path props] [:> :rect props])])) diff --git a/frontend/src/app/main/ui/shapes/path.cljs b/frontend/src/app/main/ui/shapes/path.cljs index 20fbcde0d2..ad4a7fd4b4 100644 --- a/frontend/src/app/main/ui/shapes/path.cljs +++ b/frontend/src/app/main/ui/shapes/path.cljs @@ -30,7 +30,7 @@ ""))) render-id (mf/use-ctx muc/render-id) - props (-> (attrs/extract-style-attrs shape render-id) + props (-> (attrs/get-style-props shape render-id) (obj/set! "d" pdata))] [:& shape-custom-strokes {:shape shape} diff --git a/frontend/src/app/main/ui/shapes/rect.cljs b/frontend/src/app/main/ui/shapes/rect.cljs index 1ce1dd0452..f2bf5c03d9 100644 --- a/frontend/src/app/main/ui/shapes/rect.cljs +++ b/frontend/src/app/main/ui/shapes/rect.cljs @@ -28,7 +28,7 @@ rid (mf/use-ctx muc/render-id) props (mf/with-memo [shape rid] - (-> (attrs/extract-style-attrs shape rid) + (-> (attrs/get-style-props shape rid) (obj/merge! #js {:x x :y y :transform t :width w :height h}))) path? (some? (.-d props))] diff --git a/frontend/src/app/main/ui/shapes/shape.cljs b/frontend/src/app/main/ui/shapes/shape.cljs index 0cb212a8ab..67fa2785c5 100644 --- a/frontend/src/app/main/ui/shapes/shape.cljs +++ b/frontend/src/app/main/ui/shapes/shape.cljs @@ -21,6 +21,8 @@ [app.util.object :as obj] [rumext.v2 :as mf])) +;; FIXME: revisit this: breaks all memoization because of this new +;; property added to shapes (defn propagate-wrapper-styles-child [child wrapper-props] (let [child-props-childs @@ -93,7 +95,8 @@ wrapper-props (cond-> wrapper-props (= :group type) - (attrs/add-style-attrs shape render-id) + (-> (attrs/add-fill-props! shape render-id) + (attrs/add-border-props! shape)) (some? filter-str) (obj/set! "filter" filter-str)) diff --git a/frontend/src/app/main/ui/shapes/svg_raw.cljs b/frontend/src/app/main/ui/shapes/svg_raw.cljs index ad482efed9..bb13fc1090 100644 --- a/frontend/src/app/main/ui/shapes/svg_raw.cljs +++ b/frontend/src/app/main/ui/shapes/svg_raw.cljs @@ -22,8 +22,8 @@ (def svg-ids-ctx (mf/create-context nil)) (defn set-styles [attrs shape render-id] - (let [custom-attrs (-> (usa/extract-style-attrs shape render-id) - (obj/without ["transform"])) + (let [custom-attrs (-> (usa/get-style-props shape render-id) + (obj/unset! "transform")) attrs (or attrs {}) attrs (cond-> attrs diff --git a/frontend/src/app/main/ui/shapes/text/fo_text.cljs b/frontend/src/app/main/ui/shapes/text/fo_text.cljs index 6a8d21eb26..20742fd121 100644 --- a/frontend/src/app/main/ui/shapes/text/fo_text.cljs +++ b/frontend/src/app/main/ui/shapes/text/fo_text.cljs @@ -192,7 +192,7 @@ :transform transform :width (if (#{:auto-width} grow-type) 100000 width) :height (if (#{:auto-height :auto-width} grow-type) 100000 height) - :style (-> (obj/create) (attrs/add-layer-props shape)) + :style (attrs/add-layer-props! #js {} shape) :ref ref} ;; We use a class here because react has a bug that won't use the appropriate selector for ;; `background-clip` diff --git a/frontend/src/app/main/ui/shapes/text/svg_text.cljs b/frontend/src/app/main/ui/shapes/text/svg_text.cljs index e508f0bbfc..dfaaa560fc 100644 --- a/frontend/src/app/main/ui/shapes/text/svg_text.cljs +++ b/frontend/src/app/main/ui/shapes/text/svg_text.cljs @@ -50,7 +50,8 @@ :y y :width width :height height} - (attrs/add-style-attrs shape render-id)) + (attrs/add-fill-props! shape render-id) + (attrs/add-border-props! shape)) get-gradient-id (fn [index] (str render-id "_" (:id shape) "_" index))] diff --git a/frontend/src/app/main/ui/workspace/shapes.cljs b/frontend/src/app/main/ui/workspace/shapes.cljs index fb4563e709..3d4d42f784 100644 --- a/frontend/src/app/main/ui/workspace/shapes.cljs +++ b/frontend/src/app/main/ui/workspace/shapes.cljs @@ -99,8 +99,8 @@ rawsvg? (= :svg-raw shape-type) wrapper-elem (if ^boolean rawsvg? mf/Fragment "g") wrapper-props (if ^boolean rawsvg? - #js {:className "workspace-shape-wrapper"} - #js {})] + #js {} + #js {:className "workspace-shape-wrapper"})] (when (and (some? shape) (not ^boolean (:hidden shape))) diff --git a/frontend/src/app/main/ui/workspace/viewport/outline.cljs b/frontend/src/app/main/ui/workspace/viewport/outline.cljs index 49b893a1df..8af4792b9e 100644 --- a/frontend/src/app/main/ui/workspace/viewport/outline.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/outline.cljs @@ -51,7 +51,7 @@ (d/nilv (ex/ignoring (upf/format-path content)) ""))) border-attrs - (attrs/extract-border-radius shape) + (attrs/get-border-radius shape) outline-type (case type diff --git a/frontend/src/app/util/object.cljs b/frontend/src/app/util/object.cljs index c9caf8f2b9..a70728760d 100644 --- a/frontend/src/app/util/object.cljs +++ b/frontend/src/app/util/object.cljs @@ -19,7 +19,7 @@ (defn get ([obj k] - (when-not (nil? obj) + (when (some? obj) (unchecked-get obj k))) ([obj k default] (let [result (get obj k)] @@ -27,7 +27,8 @@ (defn contains? [obj k] - (some? (unchecked-get obj k))) + (when (some? obj) + (js/Object.hasOwn obj k))) (defn get-keys [obj] @@ -105,11 +106,27 @@ (js* "~{} in ~{}" prop obj)) (defn map->obj - [o] - (reduce-kv (fn [result k v] - (let [k (if (keyword? k) (name k) k) - v (if (keyword? v) (name v) v)] - (unchecked-set result k v) - result)) + [x] + (cond + (nil? x) + nil + + (keyword? x) + (name x) + + (map? x) + (reduce-kv (fn [m k v] + (let [k (if (keyword? k) (name k) k)] + (unchecked-set m k (^function map->obj v)) + m)) #js {} - o)) + x) + + (coll? x) + (reduce (fn [arr v] + (.push arr v) + arr) + (array) + x) + + :else x)) From 393863b29f6beaf83f486c9f46410d1f9c5a5a69 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 12 Sep 2023 16:06:53 +0200 Subject: [PATCH 6/7] :bug: Fix broken hooks rule on shapes fills component --- frontend/src/app/main/ui/shapes/fills.cljs | 191 +++++++++++---------- 1 file changed, 99 insertions(+), 92 deletions(-) diff --git a/frontend/src/app/main/ui/shapes/fills.cljs b/frontend/src/app/main/ui/shapes/fills.cljs index eb46446434..30a6556ac9 100644 --- a/frontend/src/app/main/ui/shapes/fills.cljs +++ b/frontend/src/app/main/ui/shapes/fills.cljs @@ -18,101 +18,108 @@ (def no-repeat-padding 1.05) -;; FIXME: this component breaks hooks rules +(mf/defc fills* + {::mf/wrap-props false} + [props] + (let [shape (unchecked-get props "shape") + render-id (unchecked-get props "render-id") + + type (dm/get-prop shape :type) + image (get shape :fill-image) + fills (get shape :fills []) + + selrect (dm/get-prop shape :selrect) + metadata (get shape :metadata) + x (dm/get-prop selrect :x) + y (dm/get-prop selrect :y) + width (dm/get-prop selrect :width) + height (dm/get-prop selrect :height) + + has-image? (or (some? metadata) + (some? image)) + + uri (cond + (some? metadata) + (cfg/resolve-file-media metadata) + + (some? image) + (cfg/resolve-file-media image)) + + embed (embed/use-data-uris [uri]) + transform (gsh/transform-str shape) + + ;; When tru e the image has not loaded yet + loading? (and (some? uri) + (not (contains? embed uri))) + + pat-props #js {:patternUnits "userSpaceOnUse" + :x x + :y y + :width width + :height height + :data-loading loading?} + + pat-props (if (= :path type) + (obj/set! pat-props "patternTransform" transform) + pat-props)] + + (for [[shape-index shape] (d/enumerate (or (:position-data shape) [shape]))] + [:* {:key (dm/str shape-index)} + (for [[fill-index value] (reverse (d/enumerate fills))] + (when (some? (:fill-color-gradient value)) + (let [gradient (:fill-color-gradient value) + props #js {:id (dm/str "fill-color-gradient_" render-id "_" fill-index) + :key (dm/str fill-index) + :gradient gradient + :shape shape}] + (case (:type gradient) + :linear [:> grad/linear-gradient props] + :radial [:> grad/radial-gradient props])))) + + + (let [fill-id (dm/str "fill-" shape-index "-" render-id)] + [:> :pattern (-> (obj/clone pat-props) + (obj/set! "id" fill-id) + (cond-> has-image? + (-> (obj/set! "width" (* width no-repeat-padding)) + (obj/set! "height" (* height no-repeat-padding))))) + [:g + (for [[fill-index value] (reverse (d/enumerate fills))] + (let [style (attrs/get-fill-style value fill-index render-id type) + props #js {:key (dm/str fill-index) + :width width + :height height + :style style}] + [:> :rect props])) + + (when ^boolean has-image? + [:g + ;; We add this shape to add a padding so the patter won't repeat + ;; Issue: https://tree.taiga.io/project/penpot/issue/5583 + [:rect {:x 0 + :y 0 + :width (* width no-repeat-padding) + :height (* height no-repeat-padding) + :fill "none"}] + [:image {:href (or (:data-uri shape) (get embed uri uri)) + :preserveAspectRatio "none" + :x 0 + :y 0 + :width width + :height height}]])]])]))) (mf/defc fills {::mf/wrap-props false} [props] - (let [shape (unchecked-get props "shape") - render-id (unchecked-get props "render-id") + (let [shape (unchecked-get props "shape") + type (dm/get-prop shape :type) + image (:fill-image shape) + fills (:fills shape [])] - shape-type (dm/get-prop shape :type) - fill-image (:fill-image shape) - shape-fills (:fills shape [])] - - (when (or (some? fill-image) - (or (= shape-type :image) - (= shape-type :text)) - (> (count shape-fills) 1) - (some :fill-color-gradient shape-fills)) - - (let [selrect (dm/get-prop shape :selrect) - metadata (get shape :metadata) - x (dm/get-prop selrect :x) - y (dm/get-prop selrect :y) - width (dm/get-prop selrect :width) - height (dm/get-prop selrect :height) - - has-image? (or (some? metadata) - (some? fill-image)) - - uri (cond - (some? metadata) - (cfg/resolve-file-media metadata) - - (some? fill-image) - (cfg/resolve-file-media fill-image)) - - embed (embed/use-data-uris [uri]) - transform (gsh/transform-str shape) - - ;; When true the image has not loaded yet - loading? (and (some? uri) - (not (contains? embed uri))) - - pat-props #js {:patternUnits "userSpaceOnUse" - :x x - :y y - :width width - :height height - :data-loading loading?} - - pat-props (if (= :path shape-type) - (obj/set! pat-props "patternTransform" transform) - pat-props)] - - (for [[shape-index shape] (d/enumerate (or (:position-data shape) [shape]))] - [:* {:key (dm/str shape-index)} - (for [[fill-index value] (reverse (d/enumerate shape-fills))] - (when (some? (:fill-color-gradient value)) - (let [gradient (:fill-color-gradient value) - props #js {:id (dm/str "fill-color-gradient_" render-id "_" fill-index) - :key (dm/str fill-index) - :gradient gradient - :shape shape}] - (case (:type gradient) - :linear [:> grad/linear-gradient props] - :radial [:> grad/radial-gradient props])))) - - - (let [fill-id (dm/str "fill-" shape-index "-" render-id)] - [:> :pattern (-> (obj/clone pat-props) - (obj/set! "id" fill-id) - (cond-> has-image? - (-> (obj/set! "width" (* width no-repeat-padding)) - (obj/set! "height" (* height no-repeat-padding))))) - [:g - (for [[fill-index value] (reverse (d/enumerate shape-fills))] - (let [style (attrs/get-fill-style value fill-index render-id shape-type) - props #js {:key (dm/str fill-index) - :width width - :height height - :style style}] - [:> :rect props])) - - (when ^boolean has-image? - [:g - ;; We add this shape to add a padding so the patter won't repeat - ;; Issue: https://tree.taiga.io/project/penpot/issue/5583 - [:rect {:x 0 - :y 0 - :width (* width no-repeat-padding) - :height (* height no-repeat-padding) - :fill "none"}] - [:image {:href (or (:data-uri shape) (get embed uri uri)) - :preserveAspectRatio "none" - :x 0 - :y 0 - :width width - :height height}]])]])]))))) + (when (or (some? image) + (or (= type :image) + (= type :text)) + (> (count fills) 1) + (some :fill-color-gradient fills)) + [:> fills* props]))) From 307cfa287f1963d58b52d7426f5d800e7abccbc8 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 12 Sep 2023 16:30:15 +0200 Subject: [PATCH 7/7] :fire: Remove inneficient obj/without helper --- frontend/src/app/main/ui/shapes/shape.cljs | 4 +++- frontend/src/app/util/object.cljs | 10 ---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/frontend/src/app/main/ui/shapes/shape.cljs b/frontend/src/app/main/ui/shapes/shape.cljs index 67fa2785c5..f369b1afba 100644 --- a/frontend/src/app/main/ui/shapes/shape.cljs +++ b/frontend/src/app/main/ui/shapes/shape.cljs @@ -87,7 +87,9 @@ wrapper-props (-> (obj/clone props) - (obj/without ["shape" "children" "disable-shadows?"]) + (obj/unset! "shape") + (obj/unset! "children") + (obj/unset! "disable-shadows?") (obj/set! "ref" ref) (obj/set! "id" (dm/fmt "shape-%" shape-id)) (obj/set! "style" styles)) diff --git a/frontend/src/app/util/object.cljs b/frontend/src/app/util/object.cljs index a70728760d..bb5d24fcc7 100644 --- a/frontend/src/app/util/object.cljs +++ b/frontend/src/app/util/object.cljs @@ -8,7 +8,6 @@ "A collection of helpers for work with javascript objects." (:refer-clojure :exclude [set! new get get-in merge clone contains? array?]) (:require - ["lodash/omit" :as omit] [cuerdas.core :as str])) (defn array? @@ -48,15 +47,6 @@ (rest keys) (unchecked-get res key)))))) -#_:clj-kondo/ignore -(defn without - [obj keys] - (let [keys (cond - (vector? keys) (into-array keys) - (array? keys) keys - :else (throw (js/Error. "unexpected input")))] - (omit obj keys))) - (defn clone [a] (js/Object.assign #js {} a))