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

🐛 Keep orientation inherited from the old variant on switch

Browse Source 
A switch keeps the old root orientation even when its main supplied it.
Record that retained choice as geometry when it differs from the new
expected placement. Continue inheriting placement from enclosing copies
when the switch carries no root geometry override.

Portable regression: three failures before the fix; now passes through
full sync. Focused JVM checks: 182 tests, 2137 assertions. Existing goldens
match. Full frontend: 1528 tests, 8877 assertions. Live WASM passes.

Component benchmark ratios: mains 0.58/0.57/0.60; levels 0.49;
chain 0.50; library 0.60; many shapes 0.46; variant switch 0.65.
All worlds pass with task stress JVMs paused. An earlier run during
concurrent JVM startup exceeded the timing gate; the baseline is unchanged.

AI-assisted-by: GPT-6
This commit is contained in:
alonso.torres 2026-10-24 14:33:57 +02:00
parent 337b1230b6
commit 9f42e20a15
3 changed files with 45 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -31,7 +31,7 @@ Intended semantics of copies, `:touched` and component sync. This is the spec: w
- Duplicate a copy: the new subtree keeps the source's values and touched; if its nesting level changes, touched is inherited as in detach. Paste and move out of a copy follow the same rule.
- Swap without keep-touched: the new subtree keeps `swap-keep-attrs` values; the new head keeps the old shape's id. A retained rotation or flip that differs from the new component's expected placement becomes a `:geometry-group` override so later sync preserves it, including orientation inherited from the old component. The swap authors that choice. Size-only overrides clear. Orientation inherited from the position source or
the parent stays untouched, including references into deleted mains (F154). Other content overrides clear. Inside a copy it keeps the old `:shape-ref` and gets `:swap-group` (none when the new component is the near main's); inside a main it is a plain nested copy.
- 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 its authored rotation or flip as a geometry override when swap placement retains it (F146). 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).
- 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.

28
common/src/app/common/logic/variants.cljc
View File

@ -425,19 +425,25 @@
(clsw/update-attrs-on-switch changes new old new-shape original-shape old-ref container)))
changes
pairs)
changes (cond-> changes
(some #(and (= (:id %) (:id original-shape))
(ctk/touched-group? % :geometry-group)) orig-touched)
(keep-orientation-override original-shape new-shape page
(or (get-in libraries [(:component-file new-shape) :data]) ldata)
(:component-id new-shape) nil))
root-geometry? (some #(and (= (:id %) (:id original-shape))
(ctk/touched-group? % :geometry-group)) orig-touched)
parent (get page-objects (:parent-id original-shape))
;; A switch keeps orientation even when the old main supplied it.
;; If no geometry was carried, placement from an enclosing copy is
;; still inherited rather than authored by this switch.
position (when (and (not root-geometry?) (:shape-ref parent))
(ctf/find-ref-shape nil page libraries original-shape
:include-deleted? true :with-context? true))
changes (keep-orientation-override changes original-shape new-shape page
(or (get-in libraries [(:component-file new-shape) :data]) ldata)
(:component-id new-shape) position)
;; A carried size changes the frame its untouched children follow.
;; Place them through the same constraints as an ordinary copy sync.
changes (cond-> changes
(some #(ctk/touched-group? % :geometry-group) orig-touched)
(cll/generate-sync-shape-direct
nil libraries (assoc container :objects (pcb/get-objects changes))
(:id new-shape) false))]
changes (cond-> changes
(some #(ctk/touched-group? % :geometry-group) orig-touched)
(cll/generate-sync-shape-direct
nil libraries (assoc container :objects (pcb/get-objects changes))
(:id new-shape) false))]
[changes (mapv (comp :parent-id :new) pairs)]))
(defn- shape-index

27
common/test/common_tests/logic/variants_switch_test.cljc
View File

@ -16,6 +16,8 @@
[app.common.math :as mth]
[app.common.test-helpers.components :as thc]
[app.common.test-helpers.composable.comp.nodes :as n]
[app.common.test-helpers.composable.comp.runner :as r]
[app.common.test-helpers.composable.core :as tm]
[app.common.test-helpers.compositions :as tho]
[app.common.test-helpers.files :as thf]
[app.common.test-helpers.ids-map :as thi]
@ -29,6 +31,31 @@
(t/use-fixtures :each thi/test-fixture)
(t/deftest switching-an-untouched-rotated-copy-authors-its-retained-orientation
(let [file (-> (thf/sample-file :file)
(thv/add-variant :variant :c01 :m01 :c02 :m02
:variant1-params {:width 100 :height 100}
:variant2-params {:width 100 :height 100})
(ths/add-sample-shape :child1 :parent-label :m01 :width 50 :height 50)
(ths/add-sample-shape :child2 :parent-label :m02 :width 50 :height 50)
(thc/instantiate-component :c01 :copy01 :children-labels [:copy-child]))
initial (-> (tm/make-situation file)
(r/step (n/rotate :m01 45)) r/settle)
before (tm/resolve-shape initial :copy01)
switched (-> initial (r/step (n/switch-variant :copy01 "Value2")) r/settle)
head (tm/resolve-shape switched :copy01)
child (tm/shape-by-id switched (first (:shapes head)))
file (tm/file switched)
full (:file (r/sync-cascade file {} (keys (get-in file [:data :components]))))]
(t/is (empty? (:touched before)))
(t/is (mth/close? 45 (:rotation before) 0.001))
(t/is (= #{:geometry-group} (:touched head)))
(t/is (mth/close? 45 (:rotation head) 0.001))
(t/is (gsh/close-attrs? :points (:points head)
(:points (ths/get-shape-by-id full (:id head))) 0.001))
(t/is (gsh/close-attrs? :points (:points child)
(:points (ths/get-shape-by-id full (:id child))) 0.001))))
(t/deftest switch-keeps-a-resize-override-after-rotating-the-copy
(doseq [sizing [nil :fix]]
(let [params {:width 100 :height 100 :layout-item-v-sizing sizing}