github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 05:50:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

♻️ Simplify the component sync walk and its changes

Browse Source 
- The sync walk took its direction as a map of seven closures
  (direct-sync, inverse-sync). The two directions are fixed, so each
  is now a plain recursive function: sync-copy-recursive (main to
  copy, a sync or a reset) and update-main-recursive (copy to main),
  with sync-copy-shape and update-main-shape for the pair itself.
  Both share compare-children, which takes the children, a pair?
  predicate and four callbacks instead of 13 arguments; the inverse
  branch's call of the swapped callback, which could never run, goes.
- The container of the main shapes is found once, at the start of the
  walk. The walk holds the :remote-synced a reset or an update main
  gives the copy shapes, instead of initial-root? and two branches.
  add-shape-to-instance and add-shape-to-main take the parent from the
  pair being walked instead of searching the whole subtree for it.
- pcb/add-changes takes a container and the redo and undo changes, and
  applies them locally. Eight sync functions and the library
  typography sync each wrote that block; make-change goes from both
  namespaces. One list holds the attributes that link a copy shape
  with its main, and one shape-label prints shapes in the log lines.
- change-touched chose what to write from three flags. It is now
  set-touched, which takes the value each caller computes. The swap's
  restore of the kept properties passes ignore-touched instead of
  calling it afterwards to put the groups back.
- generate-sync-shape was a multimethod over the three asset types; it
  is a case in generate-sync-container. generate-sync-file loses its
  unused arity. The 100-line comment that opened the sync helpers
  described an older model; a short note points at the component sync
  contract. text-content-with-overrides only called
  txt/merge-overrides, which sync and switch now call.
- update-flex-child-copy-attrs and update-flex-child-main-attrs are one
  function, update-flex-children, and the omit-touched? argument that
  was always false goes.
- In the variant switch, sized-geometry-value builds the resized
  :selrect and :points that two geometry policies each built, and a
  binding of skip? shadowed by a second one is skip-attr?.

add-shape-to-main keeps two changes that look wrong, now marked F102:
its :reg-objects names the component, so it only reaches a deleted
component's objects, and the undo of its new shapes deletes them from
the copy's page.

No behavior change: the golden masters and the suites are unchanged.
libraries.cljc: 1816 -> 1284 lines.

Benchmark (lower quantile, ms; baseline / now / ratio):
  deepest-edit-through-three-levels    35.2 /  15.2 / 0.43
  edit-through-a-chain-of-dependents   68.9 /  35.7 / 0.52
  library-update-into-copies           83.1 /  40.5 / 0.49
  main-edit-among-many-shapes           4.8 /   2.4 / 0.51
  main-edit-into-10-copies              1.8 /   0.8 / 0.46
  main-edit-into-100-copies            15.4 /   7.8 / 0.51
  main-edit-into-1000-copies          153.1 /  97.4 / 0.64
  variant-switch-into-copies           32.1 /  17.6 / 0.55
(The machine was busy for the 1000-copies world: its mean was 119 ms,
against about 83 in the runs of the earlier steps, at 0.49-0.52.)

AI-assisted-by: claude-opus-5-5
This commit is contained in:
alonso.torres 2026-10-14 14:49:47 +02:00
parent d1104c608b
commit bc5d6bc4fe
10 changed files with 523 additions and 1118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
common/src/app/common/files/changes_builder.cljc
View File

@ -210,6 +210,19 @@
::applied-changes-count (count redo-changes)))
changes))
(defn add-changes
"Add the changes `redo` and their inverse `undo`, on the objects of
`container` (a page or a component), and apply them locally. Each list is
added in order, as one `conj` after another."
[changes container redo undo]
(let [in-container (if (cfh/page? container)
#(assoc % :page-id (:id container))
#(assoc % :component-id (:id container)))]
(-> changes
(update :redo-changes into (map in-container) redo)
(update :undo-changes into (map in-container) undo)
(apply-changes-local))))
(defn skip-local
"Stop applying changes to the builder objects."
[changes skip?]

1407
common/src/app/common/logic/libraries.cljc
View File

File diff suppressed because it is too large Load Diff

51
common/src/app/common/logic/library_assets.cljc
View File

@ -10,16 +10,9 @@
override on a copy."
(:require
[app.common.files.changes-builder :as pcb]
[app.common.files.helpers :as cfh]
[app.common.types.library :as ctl]
[app.common.types.text :as txt]))
(defn- make-change
[container change]
(if (cfh/page? container)
(assoc change :page-id (:id container))
(assoc change :component-id (:id container))))
(defn generate-sync-colors
"Copy into `shape` the value of each color of the library `library-id` that
it uses."
@ -34,41 +27,19 @@
[changes shape container update-node]
(let [old-content (:content shape)
new-content (txt/transform-nodes update-node old-content)
redo-change
(make-change
container
{:type :mod-obj
:id (:id shape)
:operations [{:type :set
:attr :content
:val new-content
:ignore-touched true}
{:type :set
:attr :position-data
:val nil}]})
undo-change
(make-change
container
{:type :mod-obj
:id (:id shape)
:operations [{:type :set
:attr :content
:val old-content
:ignore-touched true}
{:type :set
:attr :position-data
:val nil}]})
changes' (-> changes
(update :redo-changes conj redo-change)
(update :undo-changes conj undo-change))]
set-content (fn [content]
{:type :mod-obj
:id (:id shape)
:operations [{:type :set
:attr :content
:val content
:ignore-touched true}
{:type :set
:attr :position-data
:val nil}]})]
(if (= new-content old-content)
changes
(-> changes'
(pcb/apply-changes-local)))))
(pcb/add-changes changes container [(set-content new-content)] [(set-content old-content)]))))
(defn generate-sync-typographies
"Copy into the text `shape` the text styles of each typography of the

151
common/src/app/common/logic/swap.cljc
View File

@ -21,7 +21,6 @@
[app.common.logic.shapes :as cls]
[app.common.math :as mth]
[app.common.types.component :as ctk]
[app.common.types.container :as ctn]
[app.common.types.file :as ctf]
[app.common.types.path.segment :as segment]
[app.common.types.shape :as cts]
@ -67,6 +66,29 @@
(get shape attr)))
(defn- sized-geometry-value
"The value of `attr`, `:selrect` or `:points`, of `shape` resized to
`width` and `height`, at the same position and with the same transform."
[shape width height attr]
(let [{:keys [x y] :as selrect} (:selrect shape)
selrect (assoc selrect
:width width
:height height
:x1 x
:y1 y
:x2 (+ x width)
:y2 (+ y height))]
(case attr
:selrect
selrect
:points
(-> selrect
(grc/rect->points)
(gsh/transform-points
(grc/rect->center selrect)
(or (:transform shape) (gmt/matrix)))))))
(defn- switch-fixed-layout-geom-change-value
[prev-shape ; The shape before the switch
current-shape ; The shape after the switch (a clean copy)
@ -74,59 +96,22 @@
attr]
;; When there is a layout with fixed h or v sizing, we need
;; to keep the width/height (and recalculate selrect and points)
(let [prev-width (-> prev-shape :selrect :width)
current-width (-> current-shape :selrect :width)
origin-width (-> origin-shape :selrect :width)
prev-height (-> prev-shape :selrect :height)
current-height (-> current-shape :selrect :height)
origin-height (-> origin-shape :selrect :height)
x (-> current-shape :selrect :x)
y (-> current-shape :selrect :y)
h-sizing (:layout-item-h-sizing prev-shape)
v-sizing (:layout-item-v-sizing prev-shape)
final-width (if (= :fix h-sizing)
current-width
(if (= origin-width current-width)
prev-width ;; same-size: preserve override
current-width)) ;; different-size: use new component's
final-height (if (= :fix v-sizing)
current-height
(if (= origin-height current-height)
prev-height ;; same-size: preserve override
current-height)) ;; different-size: use new component's
selrect (assoc (:selrect current-shape)
:width final-width
:height final-height
:x x
:y y
:x1 x
:y1 y
:x2 (+ x final-width)
:y2 (+ y final-height))]
(let [size (fn [dimension sizing]
(let [prev (-> prev-shape :selrect dimension)
current (-> current-shape :selrect dimension)
origin (-> origin-shape :selrect dimension)]
(cond
(= :fix (sizing prev-shape)) current
;; same-size: preserve override
(= origin current) prev
;; different-size: use new component's
:else current)))
width (size :width :layout-item-h-sizing)
height (size :height :layout-item-v-sizing)]
(case attr
:width
final-width
:height
final-height
:selrect
selrect
:points
(-> selrect
(grc/rect->points)
(gsh/transform-points
(grc/rect->center selrect)
(or (:transform current-shape) (gmt/matrix)))))))
:width width
:height height
(sized-geometry-value current-shape width height attr))))
(defn- switch-geom-change-value
@ -134,31 +119,10 @@
;; Composite geometry stores absolute coordinates. When preserving a size
;; override across variants, keep the target variant's position and only carry
;; the previous dimensions; otherwise :x/:y can disagree with :selrect/:points.
(let [prev-selrect (:selrect prev-shape)
current-selrect (:selrect current-shape)
final-width (:width prev-selrect)
final-height (:height prev-selrect)
x (:x current-selrect)
y (:y current-selrect)
selrect (assoc current-selrect
:width final-width
:height final-height
:x x
:y y
:x1 x
:y1 y
:x2 (+ x final-width)
:y2 (+ y final-height))]
(case attr
:selrect
selrect
:points
(-> selrect
(grc/rect->points)
(gsh/transform-points
(grc/rect->center selrect)
(or (:transform current-shape) (gmt/matrix)))))))
(sized-geometry-value current-shape
(-> prev-shape :selrect :width)
(-> prev-shape :selrect :height)
attr))
(defn- equal-geometry?
@ -214,7 +178,7 @@
(let [sync-group
(ctk/resolve-sync-group (:type previous-shape) attr)
skip?
skip-attr?
(or
;; For auto text, avoid copying geometry-driven attrs on switch.
(and text-auto?
@ -255,7 +219,7 @@
;; If only one of them is touched, we want to address this case and
;; only update the untouched one
text-change?
(and (not skip?)
(and (not skip-attr?)
(cfh/text-shape? current-shape)
(cfh/text-shape? previous-shape)
(= :content attr)
@ -269,14 +233,14 @@
;; so, if the position-data changes but the geometry is touched we need to reset the position-data
;; so it's calculated again
reset-pos-data?
(and (not skip?)
(and (not skip-attr?)
(cfh/text-shape? previous-shape)
(or (= attr :position-data) (= attr :selrect))
(not= (:position-data previous-shape) (:position-data current-shape))
(touched :geometry-group))
skip?
(or skip?
(or skip-attr?
;; If we are going to reset the position data, skip the selrect attr
(and reset-pos-data? (= attr :selrect))
;; Avoid copying composite geometry attrs (:selrect/:points) when the
@ -305,11 +269,11 @@
;; Each text override that the old and the new main agree on is
;; carried over the new main's content
text-change?
(cll/text-content-with-overrides (:content previous-shape)
(:content current-shape)
(set/difference (txt/text-overrides touched)
(txt/override-groups (:content origin-ref-shape)
(:content current-shape))))
(txt/merge-overrides (:content previous-shape)
(:content current-shape)
(set/difference (txt/text-overrides touched)
(txt/override-groups (:content origin-ref-shape)
(:content current-shape))))
path-change?
(switch-path-change-value previous-shape
@ -369,9 +333,7 @@
#(switch-attr-operations current-shape previous-shape origin-ref-shape
touched text-auto? %))
updated-attrs (into #{} (comp (filter #(= :set (:type %)))
(map :attr))
roperations)
updated-attrs (into #{} (map :attr) roperations)
updated-sync-groups (into #{}
(keep #(ctk/resolve-sync-group (:type previous-shape) %))
updated-attrs)
@ -452,15 +414,12 @@
[new-shape (-> changes
;; Restore the properties
(pcb/update-shapes [(:id new-shape)] #(d/patch-object % keep-props-values))
(pcb/update-shapes [(:id new-shape)] #(d/patch-object % keep-props-values)
{:ignore-touched true})
;; We need to set the same index as the original shape
(pcb/change-parent (:parent-id shape) [new-shape] index {:allow-altering-copies true
:ignore-touched true})
(cll/change-touched new-shape
shape
(ctn/make-container page :page)
{}))]))
:ignore-touched true}))]))
(defn generate-component-swap
[changes objects shape file page libraries id-new-component

3
common/src/app/common/test_helpers/composable/comp/nodes.cljc
View File

@ -851,8 +851,7 @@
container (ctn/make-container page :page)
changes (-> (pcb/empty-changes)
(pcb/with-container container)
(cll/generate-sync-shape-inverse (:data the-file)
(libraries situation)
(cll/generate-sync-shape-inverse (libraries situation)
container
shape-id)
(strip-local-change))

5
common/src/app/common/types/file.cljc
View File

@ -499,8 +499,9 @@
(d/update-when shape :shape-ref #(get ref-map % %))))))
(defn swapped-subhead?
"Whether `shape` references outside its near main parent and needs a swap slot.
Same-parent positional differences are synchronized as reorders."
"Whether `shape` refers to a main shape that is not a child of the main
shape its parent refers to (`find-parent-ref-shape`). Same-parent
positional differences are synchronized as reorders."
[shape container find-parent-ref-shape]
(let [parent-shape (ctst/get-shape container (:parent-id shape))
parent-ref-shape (when parent-shape

3
common/test/common_tests/logic/comp_flex_interactions_test.cljc
View File

@ -90,8 +90,7 @@
container (ctn/make-container page :page)
changes (-> (pcb/empty-changes)
(pcb/with-container container)
(cll/generate-sync-shape-inverse (:data updated-file)
{(:id updated-file) updated-file}
(cll/generate-sync-shape-inverse {(:id updated-file) updated-file}
container
(thi/id :copy-root)))
file' (thf/apply-changes updated-file changes)

3
common/test/common_tests/logic/comp_sync_test.cljc
View File

@ -568,8 +568,7 @@
container (ctn/make-container page :page)
changes (-> (pcb/empty-changes)
(pcb/with-container container)
(cll/generate-sync-shape-inverse (:data file)
{(:id file) file}
(cll/generate-sync-shape-inverse {(:id file) file}
container
(thi/id :copy-root)))
file' (thf/apply-changes file changes)

3
common/test/common_tests/logic/swap_group_test.cljc
View File

@ -412,8 +412,7 @@
container (ctn/make-container page :page)
changes (-> (pcb/empty-changes)
(pcb/with-container container)
(cll/generate-sync-shape-inverse (:data file)
{(:id file) file}
(cll/generate-sync-shape-inverse {(:id file) file}
container
(thi/id label)))]
(-> file

2
frontend/src/app/main/data/workspace/libraries.cljs
View File

@ -865,7 +865,7 @@
(-> (pcb/empty-changes it)
(pcb/set-undo-group undo-group)
(pcb/with-container container)
(cll/generate-sync-shape-inverse fdata libraries container id))
(cll/generate-sync-shape-inverse libraries container id))
ldata (->> (:component-file shape)
(dsh/lookup-file-data state))