github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-30 14:30:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Skip numerically equal component sync writes

Browse Source 
Compare integer and floating forms exactly for scalar values, points and
selection rectangles. A library update that only changes their numeric
representation must not create changes or an undo entry. Preserve every
nonzero difference, including real subpixel moves.

Add the reduced resize/library-sync regression and a subpixel control,
plus async frontend undo/redo replay. Review and update eight golden
states in case N: only equal integer/float representations changed.
The recreated seed-712 audit leaves all 61 syncs and undo depths unchanged.

Common JVM 1,914 tests/31,146 assertions and JS 1,845/29,810 pass.
Frontend 1,563/10,880, its added replay, backend 1,610/8,104 and exporter
14/32 pass. Lint and formatting pass. Exporter needs its test-only secret.

Benchmark ratios against the unchanged baseline: mains 10/100/1000
0.54/0.61/0.61, levels 0.53, chain 0.55, library 0.55, many shapes 0.62,
variant switch 0.65. No regression flag.

AI-assisted-by: gpt-6
This commit is contained in:
alonso.torres 2026-10-27 13:24:06 +01:00
parent 7665b80018
commit ea4bf5d56f
6 changed files with 54 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
.serena/memories/common/component-sync-contract.md
View File

@ -37,6 +37,10 @@ Intended semantics of copies, `:touched` and component sync. This is the spec: w
- Variant switch with keep-touched: an attr is carried from the old copy only when its group is touched and the old and new mains hold the same value for it. Resulting touched = groups of carried attrs (+ `:swap-group` when the head is inside a copy). The root also keeps a retained rotation or flip as a geometry override when it differs from the new expected placement, including orientation supplied by the old component (F146, F175). Placement still supplied by an enclosing copy remains inherited when no root geometry override was carried. A value equal to the new main is not carried and not touched. Texts: the same per text group (letters carried only when both mains have the same letters; a style group when both mains agree on it once placed on each other's letters).
- Delete component: copies keep their data. The deleted main cannot be edited, so it starts no sync, but changes from components nested in it still pass through: sync updates its stored shapes (`:objects` of the deleted component), its copies, and the components that nest it. Restore brings back an up-to-date main.
- Undo/redo: restores the exact prior state, `:touched` and `:remote-synced` included, of the op and the sync it caused together (same undo group, one step). Undo never needs a re-sync.
- Sync skips exactly equal numeric values, including integer/float forms
inside points and selection rectangles (F187). Representation alone must
not create changes or undo entries. This comparison has no tolerance:
a real subpixel difference still syncs.
## Text overrides

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

@ -1249,6 +1249,24 @@
(or (automatic-name content) origin-name))
origin-name)))
(defn- same-sync-value?
"Compare numeric values exactly, including integer/float representations
inside geometry. Every nonzero difference still requires a sync."
[attr value other]
(or (= value other)
(let [same-number? (fn [a b] (and (number? a) (number? b) (== a b)))]
(case attr
:selrect (and (map? value) (map? other)
(= (set (keys value)) (set (keys other)))
(every? #(same-number? (get value %) (get other %)) (keys value)))
:points (and (sequential? value) (sequential? other)
(= (count value) (count other))
(every? (fn [[a b]]
(and (same-number? (:x a) (:x b))
(same-number? (:y a) (:y b))))
(map vector value other)))
(same-number? value other)))))
(defn- sync-attr-operations
"The `[attr value]` pairs that sync `attr` from `origin-shape`, a main
shape placed like `dest-shape`, into `dest-shape`, its copy. With
@ -1289,7 +1307,7 @@
skip?
(or (and (not text-name?)
(= (get origin-shape attr) (get dest-shape attr)))
(same-sync-value? attr (get origin-shape attr) (get dest-shape attr)))
(and (touched sync-group)
omit-touched?
;; When it is a text-partial-change, we should generate operations
@ -1318,7 +1336,7 @@
(cond
;; If the final attr-value is the actual value, skip
(or skip? (= attr-val (get dest-shape attr)))
(or skip? (same-sync-value? attr attr-val (get dest-shape attr)))
[]
;; On a text-partial-change, we want to force a position-data reset

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

@ -33,6 +33,21 @@
(t/use-fixtures :each thi/test-fixture)
(t/deftest sync-keeps-a-real-subpixel-position-change
(let [initial (setup/simple-component-with-labeled-copy)
original (tm/resolve-shape initial :copy-child)
file (tm/file initial)
page (thf/current-page file)
changes (cls/generate-update-shapes
(pcb/empty-changes nil (:id page)) [(thi/id :main-child)]
#(gsh/move % (gpt/point 0.000001 0)) (:objects page) {})
source (thf/apply-changes file changes)
changed (tm/with-file initial (:file (r/sync-cascade source {} (keys (get-in file [:data :components])))))
actual (tm/resolve-shape changed :copy-child)
expected (gsh/move original (gpt/point 0.000001 0))]
(t/is (not= (:points original) (:points actual)))
(t/is (gsh/close-attrs? :points (:points expected) (:points actual) 1e-10))))
(t/deftest grid-sync-keeps-new-main-cells
(let [file (-> (thf/sample-file :file1)
(tho/add-grid-frame :main-root :width 100 :height 100)

7
common/test/common_tests/logic/composable_gen_test.cljc
View File

@ -427,6 +427,13 @@
:ops [[:text-style 4 8 1] [:switch 1 0 0 0]
[:undo] [:undo] [:redo] [:redo]]}))))
(t/deftest accepting-a-library-does-not-record-equal-numeric-geometry
(let [before (-> (cg/initial-state [2 1 {:library true :variants true}])
(cg/step [:resize 10 2 2 10]) cg/settle-state)
after (-> before (cg/step [:library-sync 3 1 2 8]) cg/settle-state)]
(t/is (= (r/undo-depth (:situation before)) (r/undo-depth (:situation after))))
(t/is (= (oracle/snapshot (:situation before)) (oracle/snapshot (:situation after))))))
(t/deftest source-library-edits-wait-for-explicit-consumer-acceptance
(let [initial (cg/initial-state [2 1 {:library true :stale-libraries true}])
situation (:situation initial)

6
common/test/common_tests/logic/golden_master.edn
View File

@ -149,10 +149,10 @@
{:transcript "rotate {:target :copy-root, :angle 45}\n rotate {:target :main-root, :angle 30}\n change-height {:target :main-instance, :value 80}", :states ["8208735a98e4c105" "f848dd262a0a07aa" "88df05dbfc4217c8" "6f3c4b7eb0e89f8a"]}
{:transcript "rotate {:target :copy-root, :angle 45}\n one-of -> skip\n change-property {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["8208735a98e4c105" "aa9f6c1b6a87ac0f" "38265afaa05640e8"]}
{:transcript "rotate {:target :copy-root, :angle 45}\n one-of -> skip\n change-height {:target :main-instance, :value 80}", :states ["8208735a98e4c105" "e92b0483129edb9c" "afd213908fee2f38"]}
{:transcript "one-of -> skip\n rotate {:target :main-root, :angle 30}\n change-property {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["39e2dadef951dca2" "0cc17d2b4ff02bb9" "b675e496913b617e"]}
{:transcript "one-of -> skip\n rotate {:target :main-root, :angle 30}\n change-height {:target :main-instance, :value 80}", :states ["39e2dadef951dca2" "789103afba436783" "78667a8353113f40"]}
{:transcript "one-of -> skip\n rotate {:target :main-root, :angle 30}\n change-property {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["144339de3f79f0c9" "dfbdf08b9dabae5a" "f1bd4da0fd31db22"]}
{:transcript "one-of -> skip\n rotate {:target :main-root, :angle 30}\n change-height {:target :main-instance, :value 80}", :states ["144339de3f79f0c9" "3afe54402183c41f" "1b64e8013665d73a"]}
{:transcript "one-of -> skip\n one-of -> skip\n change-property {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["17b5ce28aeffa62e" "8f31b06c3ab1921b"]}
{:transcript "one-of -> skip\n one-of -> skip\n change-height {:target :main-instance, :value 80}", :states ["928074db7743f484" "d54aee2c77882202"]}]
{:transcript "one-of -> skip\n one-of -> skip\n change-height {:target :main-instance, :value 80}", :states ["c30da30e06aa190f" "4bdc1cd2f25762e2"]}]
:o
[{:transcript "create-component {:component \"main\", :id sync-main-component}\n make-variant-container {:variant-set \"vset\", :variant-id sync-vset-vcontainer, :count 2}\n make-nested-component-with-variant {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n instantiate-copy {:component \"main\", :copy sync-main-copyhead-0}\n change-property {:target #fn, :property :fills, :value \"#00ff00\"}\n switch-variant {:target-component sync-vset-v1-component}", :states ["9a1a3735f1009290" "b00ea3774a618f8e" "d15895a5c5a7e993"]}
{:transcript "create-component {:component \"main\", :id sync-main-component}\n make-variant-container {:variant-set \"vset\", :variant-id sync-vset-vcontainer, :count 2}\n make-nested-component-with-variant {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n instantiate-copy {:component \"main\", :copy sync-main-copyhead-0}\n change-property {:target #fn, :property :fills, :value \"#00ff00\"}\n switch-variant {:target-component sync-vset-v1-component}", :states ["1247c1fd53a5de3f" "45d0fe0a073fbada" "9a8e3d09e48af9fe"]}]

5
frontend/test/frontend_tests/composable_tests/generated_test.cljs
View File

@ -88,6 +88,11 @@
rx/timer mock/timer-mock}
(await (replay-program program)))))
(t/deftest ^:async accepting-a-library-does-not-record-equal-numeric-geometry
(await (replay {:world [2 1 {:library true :variants true}]
:ops [[:resize 10 2 2 10] [:library-sync 3 1 2 8]
[:undo] [:redo]]})))
(t/deftest ^:async nesting-a-swapped-copy-in-a-blocked-main-authors-its-lock
(await (replay {:world [2 0 {:library true :variants true :rich-variants true
:text true :text-styles true :layout nil :layout-copies true}]