mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
🐛 Settle swapped placement when a parent turns
A swapped head can inherit a position frame with a different size from its content. Turning its parent then leaves the head and child away from the placement that full sync computes. Settle that derived placement in the primary transform builder for SVG, WASM and shared test operations. Preserve authored geometry and keep the correction in the same undo group. F182: portable regression fails four assertions before the fix and passes all twelve after it. The original and shrunk library programs pass. The focused suite passes 137 tests / 2570 assertions with unchanged goldens. Frontend passes 1535 tests / 9141 assertions; lint and formatting pass. Live WASM checks at 45 and 90 degrees pass undo, redo and later sync. Common JVM/JS, backend, exporter and all 68 plugin cases pass. Phase 1e benchmark ratios: mains 10/100/1000 .49/.57/.61, levels .52, chain .55, library .58, many shapes .59, variant switch .67. AI-assisted-by: GPT-6
This commit is contained in:
parent
768b12d68f
commit
8b40e6059c
@ -66,3 +66,9 @@ Intended semantics of copies, `:touched` and component sync. This is the spec: w
|
||||
|
||||
- `{:type :set-touched}` replaces `:touched` wholesale and only on copies (an empty set is dropped). The builders write only it (`pcb/update-shapes` turns a `:touched` change into it); `{:type :set :attr :touched}`, a raw assoc with no copy guard, is still applied for stored changes.
|
||||
- Builders that write to copies must emit the inverse `:set-touched` in undo (as `pcb/update-shapes` does).
|
||||
|
||||
A parent turn settles swapped descendant placement in the transform's
|
||||
change builder (F182). Its content and position reference can have different
|
||||
sizes, so rotating the content rigidly can leave its untransformed anchor
|
||||
different from full sync. Keep the swapped head/children untouched and put
|
||||
the derived correction in the same undo group. Authored geometry stays.
|
||||
|
||||
@ -17,4 +17,14 @@
|
||||
|
||||
- WASM modifier updates set plugin/local props with parsed geometry/structure modifiers rather than directly mutating file data.
|
||||
- The position-data recomputation watcher ignores commits tagged `:position-data`; keep that tag when adding derived position-data commits.
|
||||
- Rotation has separate WASM and non-WASM event paths. Check both when changing rotation modifier semantics.
|
||||
- Rotation has separate WASM and non-WASM event paths. Check both when changing rotation modifier semantics.
|
||||
|
||||
## Settling swapped placement after a parent turn
|
||||
|
||||
Both modifier commit paths set settle-component-placement? (F182). The
|
||||
update-shapes builder settles untouched swapped heads after parent bounds
|
||||
update, before the single commit. A swapped head can keep its content size
|
||||
while its position reference has another size; a rigid turn alone then
|
||||
leaves placement different from full sync. Preserve authored geometry and
|
||||
join derived placement to the transform's undo group. The common helper
|
||||
compares matrix axes as well as rotation/flip metadata for the WASM path.
|
||||
|
||||
@ -332,6 +332,34 @@
|
||||
changes)
|
||||
changes)))
|
||||
|
||||
(defn generate-sync-transformed-placement
|
||||
"Settle untouched swapped heads carried through a parent turn in the
|
||||
same change group. Their position frame can have a different size from
|
||||
their content, so a rigid turn alone does not settle their placement."
|
||||
[changes file libraries page before ids]
|
||||
(let [objects (pcb/get-objects changes)
|
||||
orientation (fn [shape]
|
||||
[(select-keys (or (:transform shape) (gmt/matrix)) [:a :b :c :d])
|
||||
(:rotation shape) (boolean (:flip-x shape)) (boolean (:flip-y shape))])
|
||||
roots (into (d/ordered-set)
|
||||
(keep (fn [id]
|
||||
(let [shape (get objects id)]
|
||||
(when (and (ctk/swapped? shape)
|
||||
(not (ctk/touched-group? shape :geometry-group))
|
||||
(not= (orientation (get before id)) (orientation shape)))
|
||||
(:id (ctn/get-component-shape objects shape))))))
|
||||
ids)]
|
||||
(reduce (fn [changes id]
|
||||
(let [objects (pcb/get-objects changes)
|
||||
page (assoc page :objects objects)
|
||||
file (assoc-in file [:data :pages-index (:id page) :objects] objects)
|
||||
container (ctn/make-container page :page)]
|
||||
(-> changes
|
||||
(pcb/with-container container)
|
||||
(generate-sync-shape-direct file (assoc libraries (:id file) file)
|
||||
container id false))))
|
||||
changes roots)))
|
||||
|
||||
(defn- sync-copy-shape
|
||||
"Sync the copy shape `shape-inst` from `shape-main`, the main shape it
|
||||
pairs with. A reset also clears its overrides and its `:remote-synced`,
|
||||
|
||||
@ -286,14 +286,18 @@
|
||||
|
||||
(defn transform-changes
|
||||
"Apply a transform through constraints and the shared placement step."
|
||||
[page shape modifiers]
|
||||
(let [objects (:objects page)
|
||||
tree (gm/set-objects-modifiers {(:id shape) {:modifiers modifiers}} objects)
|
||||
transform (clp/modifiers-transform tree)]
|
||||
(cls/generate-update-shapes
|
||||
(pcb/empty-changes nil (:id page)) (keys tree) transform objects
|
||||
{:ignore-tree (clp/calculate-ignore-tree objects (keys tree) transform #{(:id shape)})
|
||||
:attrs clp/transform-attrs})))
|
||||
([page shape modifiers]
|
||||
(transform-changes page shape modifiers nil))
|
||||
([page shape modifiers {:keys [file libraries]}]
|
||||
(let [objects (:objects page)
|
||||
tree (gm/set-objects-modifiers {(:id shape) {:modifiers modifiers}} objects)
|
||||
transform (clp/modifiers-transform tree)
|
||||
changes (cls/generate-update-shapes
|
||||
(pcb/empty-changes nil (:id page)) (keys tree) transform objects
|
||||
{:ignore-tree (clp/calculate-ignore-tree objects (keys tree) transform #{(:id shape)})
|
||||
:attrs clp/transform-attrs})]
|
||||
(cond-> changes
|
||||
file (cll/generate-sync-transformed-placement file libraries page objects (keys tree))))))
|
||||
|
||||
(defrecord Rotate [target angle]
|
||||
IComponentOperation
|
||||
@ -307,7 +311,8 @@
|
||||
objects (:objects page)
|
||||
shape (get objects shape-id)
|
||||
center (gsh/shape->center shape)
|
||||
changes (transform-changes page shape (ctm/rotation-modifiers shape center angle))
|
||||
changes (transform-changes page shape (ctm/rotation-modifiers shape center angle)
|
||||
{:file the-file :libraries (cf/all-files situation)})
|
||||
file' (thf/apply-changes the-file changes)]
|
||||
(-> situation
|
||||
(tm/with-file file')
|
||||
@ -332,7 +337,8 @@
|
||||
page (thf/current-page the-file)
|
||||
objects (:objects page)
|
||||
shape (get objects shape-id)
|
||||
changes (transform-changes page shape (ctm/change-size shape nil value))
|
||||
changes (transform-changes page shape (ctm/change-size shape nil value)
|
||||
{:file the-file :libraries (cf/all-files situation)})
|
||||
file' (thf/apply-changes the-file changes)]
|
||||
(-> situation
|
||||
(tm/with-file file')
|
||||
|
||||
@ -36,6 +36,38 @@
|
||||
|
||||
(t/use-fixtures :each thi/test-fixture)
|
||||
|
||||
(t/deftest turning-a-parent-settles-a-swapped-head-with-a-different-position-size
|
||||
(doseq [angle [45 90]]
|
||||
(let [file (-> (thf/sample-file :file)
|
||||
(tho/add-simple-component :a :main-a :rect-a
|
||||
:root-params {:width 100 :height 100})
|
||||
(tho/add-simple-component :other :main-other :rect-other
|
||||
:root-params {:width 100 :height 100})
|
||||
(tho/add-frame :main-b {:width 200 :height 200})
|
||||
(thc/instantiate-component :a :b-a :parent-label :main-b)
|
||||
(thc/make-component :b :main-b)
|
||||
(tho/add-frame :main-c {:width 300 :height 300})
|
||||
(thc/instantiate-component :b :c-b :parent-label :main-c)
|
||||
(thc/make-component :c :main-c)
|
||||
(thc/instantiate-component :c :copy-c :children-labels [:copy-b :copy-a]))
|
||||
before (-> (tm/make-situation file)
|
||||
(r/step (n/swap-shape :copy-a :other)) r/settle
|
||||
(r/step (n/change-height :b-a 80)) r/settle)
|
||||
turned (-> before (r/step (n/rotate :copy-c angle)) r/settle)
|
||||
head (tm/resolve-shape turned :copy-a)
|
||||
child (tm/shape-by-id turned (first (:shapes head)))
|
||||
file (tm/file turned)
|
||||
full (:file (r/sync-cascade file {} (keys (get-in file [:data :components]))))]
|
||||
(t/is (mth/close? 100 (:height head) 0.001))
|
||||
(t/is (= #{:swap-group} (:touched head)))
|
||||
(t/is (empty? (:touched child)))
|
||||
(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/is (= (thn/normalize-file (tm/file before))
|
||||
(thn/normalize-file (tm/file (r/step turned (n/undo)))))))))
|
||||
|
||||
(t/deftest authored-child-geometry-follows-a-parent-rotation-received-through-sync
|
||||
(doseq [swapped? [false true]
|
||||
angle [45 135]
|
||||
|
||||
@ -701,6 +701,7 @@
|
||||
(assoc :undo-group undo-group)
|
||||
(assoc :reg-objects? true)
|
||||
(assoc :ignore-tree ignore-tree)
|
||||
(assoc :settle-component-placement? true)
|
||||
(assoc :translation? translation?)
|
||||
;; Attributes that can change in the transform. This
|
||||
;; way we don't have to check all the attributes
|
||||
@ -864,6 +865,7 @@
|
||||
(assoc :undo-group undo-group)
|
||||
(assoc :reg-objects? true)
|
||||
(assoc :ignore-tree ignore-tree)
|
||||
(assoc :settle-component-placement? true)
|
||||
;; Attributes that can change in the transform. This
|
||||
;; way we don't have to check all the attributes
|
||||
(assoc :attrs clp/transform-attrs))
|
||||
|
||||
@ -11,6 +11,7 @@
|
||||
[app.common.files.changes-builder :as pcb]
|
||||
[app.common.files.helpers :as cfh]
|
||||
[app.common.files.shapes-helpers :as cfsh]
|
||||
[app.common.logic.libraries :as cll]
|
||||
[app.common.logic.shapes :as cls]
|
||||
[app.common.schema :as sm]
|
||||
[app.common.types.component :as ctc]
|
||||
@ -212,7 +213,7 @@
|
||||
{:as props
|
||||
:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id
|
||||
ignore-touched undo-group with-objects? changed-sub-attr changed-item-index translation?
|
||||
skip-grid-reassignment? skip-component-sync? resize-ids]
|
||||
skip-grid-reassignment? skip-component-sync? resize-ids settle-component-placement?]
|
||||
:or {reg-objects? false
|
||||
save-undo? true
|
||||
stack-undo? false
|
||||
@ -280,7 +281,13 @@
|
||||
(rx/empty))
|
||||
|
||||
(if (seq (:redo-changes changes))
|
||||
(let [changes (cond-> changes reg-objects? (pcb/resize-parents (or resize-ids ids)))]
|
||||
(let [changes (cond-> changes
|
||||
reg-objects? (pcb/resize-parents (or resize-ids ids))
|
||||
(and settle-component-placement? (not skip-component-sync?))
|
||||
(cll/generate-sync-transformed-placement
|
||||
(dsh/lookup-file state (:current-file-id state))
|
||||
(dsh/lookup-libraries state)
|
||||
(dsh/lookup-page state page-id) objects ids))]
|
||||
(rx/of (dch/commit-changes changes)))
|
||||
(rx/empty))
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user