diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index d18f7019ec..a89aa633ab 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -2505,15 +2505,49 @@ (pcb/concat-changes changes new-changes))) (defn- reposition-shape - [shape origin-root dest-root] - (let [shape-pos (fn [shape] - (gpt/point (get-in shape [:selrect :x]) - (get-in shape [:selrect :y]))) + "Expresses the shape (belonging to the origin-root instance) in the frame of the + dest-root instance, making the geometry of both instances directly comparable — + and copyable. - origin-root-pos (shape-pos origin-root) - dest-root-pos (shape-pos dest-root) - delta (gpt/subtract dest-root-pos origin-root-pos)] - (gsh/move shape delta))) + If the dest root's geometry is NOT overridden (touched), the instance follows + the origin's transformation verbatim (including rotation and flips), so a + translation by the roots' (untransformed) position delta suffices — position is + free per-instance placement. + + If the dest root's geometry IS overridden (e.g. the user rotated the copy as a + whole), the instance keeps its own placement transform, so the origin shape is + additionally transformed by the roots' relative transformation (rotation / + flips) around the dest root center — geometric changes then land expressed in + the dest instance's own frame instead of wiping its placement." + [shape origin-root dest-root] + (let [shape-pos (fn [shape] + (gpt/point (dm/get-in shape [:selrect :x]) + (dm/get-in shape [:selrect :y]))) + + origin-root-pos (shape-pos origin-root) + dest-root-pos (shape-pos dest-root) + delta (gpt/subtract dest-root-pos origin-root-pos) + + shape (gsh/move shape delta)] + (if-not (ctk/touched-group? dest-root :geometry-group) + shape + (let [origin-transform (d/nilv (:transform origin-root) (gmt/matrix)) + dest-transform (d/nilv (:transform dest-root) (gmt/matrix)) + rel-transform (gmt/multiply dest-transform (gmt/inverse origin-transform))] + (if ^boolean (gmt/unit? rel-transform) + shape + ;; The roots differ in rotation/flips: rotate the whole (already moved) + ;; shape around the dest root center by the roots' relative transform. + ;; The :rotation attribute delta is fed through the :modifiers path so + ;; apply-transform keeps it consistent with the resulting matrix. + (let [center (grc/rect->center (:selrect dest-root)) + rel-rotation (mod (- (d/nilv (:rotation dest-root) 0) + (d/nilv (:rotation origin-root) 0)) + 360)] + (-> shape + (assoc-in [:modifiers :rotation] rel-rotation) + (gsh/apply-transform (gmt/transform-in center rel-transform)) + (dissoc :modifiers)))))))) (defn- make-change [container change] diff --git a/frontend/src/app/main/data/workspace/modifiers.cljs b/frontend/src/app/main/data/workspace/modifiers.cljs index 41f14c0eea..91268a8550 100644 --- a/frontend/src/app/main/data/workspace/modifiers.cljs +++ b/frontend/src/app/main/data/workspace/modifiers.cljs @@ -117,28 +117,54 @@ ;; geometric attributes of the shapes. (defn- check-delta - "If the shape is a component instance, check its relative position and rotation respect - the root of the component, and see if it changes after applying a transformation." - [shape root transformed-shape transformed-root] - (let [shape-delta - (when root - (gpt/point (- (gsh/left-bound shape) (gsh/left-bound root)) - (- (gsh/top-bound shape) (gsh/top-bound root)))) + "If the shape is a component instance, check whether the transformation is a + free change that must not mark the shape as touched. - transformed-shape-delta - (when transformed-root - (gpt/point (- (gsh/left-bound transformed-shape) (gsh/left-bound transformed-root)) - (- (gsh/top-bound transformed-shape) (gsh/top-bound transformed-root)))) + For the instance ROOT, position is free placement, but its rotation and flips + are inherited content: transforming the copy as a whole overrides them, so a + change there marks the root as touched. + + For DESCENDANTS, position, size, rotation and flips are compared RELATIVE TO + THE ROOT: a transformation of the whole instance preserves them all and does + not mark the descendants as touched; editing an individual shape does." + [shape root transformed-shape transformed-root] + (let [is-root? (and (some? root) + (= (dm/get-prop shape :id) (dm/get-prop root :id))) + + center (fn [shape] + (grc/rect->center (:selrect shape))) + + rel-pos (fn [shape root] + (when root + ;; vector from the root center to the shape center, + ;; expressed in the root's local (untransformed) axes + (-> (gpt/subtract (center shape) (center root)) + (gpt/transform (:transform-inverse root (gmt/matrix)))))) + + orientation (fn [shape root] + ;; the shape's rotation/flip orientation taken from its + ;; transform matrix, so it is reliable regardless of how the + ;; transform was applied (the WASM apply-transform path does + ;; not refresh the :rotation attribute). For the ROOT the + ;; absolute orientation; for a DESCENDANT the orientation + ;; relative to the root — invariant when the whole instance + ;; is rotated or flipped as a unit. + (let [t (:transform shape (gmt/matrix))] + (if is-root? + t + (gmt/multiply (:transform-inverse root (gmt/matrix)) t)))) + + pos-before (rel-pos shape root) + pos-after (rel-pos transformed-shape transformed-root) distance - (if (and shape-delta transformed-shape-delta) - (gpt/distance-vector shape-delta transformed-shape-delta) + (if (and pos-before pos-after) + (gpt/distance-vector pos-before pos-after) (gpt/point 0 0)) - rotation-delta - (if (and (some? (:rotation shape)) (some? (:rotation shape))) - (- (:rotation transformed-shape) (:rotation shape)) - 0) + orientation-unchanged? + (gmt/close? (orientation shape root) + (orientation transformed-shape transformed-root)) selrect (:selrect shape) transformed-selrect (:selrect transformed-shape)] @@ -152,7 +178,7 @@ (and (and (< (:x distance) 1) (< (:y distance) 1)) (mth/close? (:width selrect) (:width transformed-selrect)) (mth/close? (:height selrect) (:height transformed-selrect)) - (mth/close? rotation-delta 0)))) + orientation-unchanged?))) (defn calculate-ignore-tree "Retrieves a map with the flag `ignore-geometry?` given a tree of modifiers" diff --git a/frontend/test/frontend_tests/composable_tests/comp/nodes.cljs b/frontend/test/frontend_tests/composable_tests/comp/nodes.cljs index 61c91d62c8..9d2089d601 100644 --- a/frontend/test/frontend_tests/composable_tests/comp/nodes.cljs +++ b/frontend/test/frontend_tests/composable_tests/comp/nodes.cljs @@ -26,15 +26,19 @@ [app.common.data :as d] [app.common.files.changes-builder :as pcb] [app.common.files.helpers :as cfh] + [app.common.geom.point :as gpt] + [app.common.geom.shapes :as gsh] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] [app.common.logic.variants :as clv] + [app.common.math :as mth] [app.common.test-helpers.components :as thc] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] [app.common.types.container :as ctn] + [app.common.types.modifiers :as ctm] [frontend-tests.composable-tests.core :as tm])) ;; --------------------------------------------------------------------------- @@ -127,6 +131,86 @@ (def ^{:doc "Alias of `has-property-of`."} has-attr? has-property-of) (def ^{:doc "Alias of `applied-property`."} applied-attr applied-property) +;; --------------------------------------------------------------------------- +;; Geometry operations +;; +;; Unlike `change-property` (a raw attribute write), geometric changes must go +;; through the TRANSFORM pipeline: on the frontend, the interpreter dispatches +;; the real sidebar events (`dwt/increase-rotation`, `dwt/update-dimensions`), +;; whose apply-modifiers step also runs the placement-vs-override classification +;; for component copies (calculate-ignore-tree / check-delta) — which is part of +;; what these operations exist to exercise. The synchronous `apply-to` fallback +;; below performs the geometrically equivalent transform through the production +;; math, but cannot classify placement (that code is frontend-only), so cases +;; using these operations are meant to run through the interpreter. + +(defrecord Rotate [target angle] + tm/IOperation + (apply-to [this situation] + (let [the-file (tm/file situation) + shape-id (tm/target-shape-id situation target) + page (thf/current-page the-file) + objects (:objects page) + shape (get objects shape-id) + ;; rotating a container rotates its whole subtree, as the real event does + ids (into #{shape-id} (cfh/get-children-ids objects shape-id)) + center (gsh/shape->center shape) + rotate1 (fn [s] (gsh/transform-shape s (ctm/rotation-modifiers s center angle))) + changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + ids + rotate1 + objects + {}) + file' (thf/apply-changes the-file changes)] + (-> situation + (tm/with-file file') + (tm/record-application this {:target target :angle angle}))))) + +(defn rotate + "Constructor for the rotation operation: rotate the shape named by `target` + (a role, a label, or a `(situation -> id)` fn) — including its whole subtree — + by `angle` degrees around its center. On the frontend this dispatches the real + `dwt/increase-rotation` event." + [target angle] + (tm/assign-id (->Rotate target angle))) + +(defrecord ChangeHeight [target value] + tm/IOperation + (apply-to [this situation] + (let [the-file (tm/file situation) + shape-id (tm/target-shape-id situation target) + page (thf/current-page the-file) + objects (:objects page) + shape (get objects shape-id) + resize1 (fn [s] (gsh/transform-shape + s + (ctm/resize-modifiers (gpt/point 1 (/ value (:height s))) + (gpt/point (:x s) (:y s))))) + changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(:id shape)} + resize1 + objects + {}) + file' (thf/apply-changes the-file changes)] + (-> situation + (tm/with-file file') + (tm/record-application this {:target target :value value}))))) + +(defn change-height + "Constructor for the height-change operation: resize the shape named by `target` + to height `value` (width unchanged). On the frontend this dispatches the real + `dwt/update-dimensions` event. Implements `IPropertyCheck`, so a `one-of` over + property and geometry edits can assert uniformly via `has-property-of`." + [target value] + (tm/assign-id (->ChangeHeight target value))) + +(extend-type ChangeHeight + IPropertyCheck + (applied-property [_node] :height) + (applied-value [node] (:value node)) + (has-property-of [node shape] + (mth/close? (:value node) (:height shape)))) + ;; =========================================================================== ;; Synchronisation-scenario building blocks ;; diff --git a/frontend/test/frontend_tests/composable_tests/comp/sync_test.cljs b/frontend/test/frontend_tests/composable_tests/comp/sync_test.cljs index 79a32492a4..11821550b0 100644 --- a/frontend/test/frontend_tests/composable_tests/comp/sync_test.cljs +++ b/frontend/test/frontend_tests/composable_tests/comp/sync_test.cljs @@ -27,6 +27,10 @@ Async: each deftest uses `t/async`; `ftm/check` drives the store and calls `done` when finished." (:require + [app.common.geom.matrix :as gmt] + [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] + [app.common.math :as mth] [app.common.types.component :as ctk] [app.common.types.shape-tree :as ctst] [cljs.test :as t :include-macros true] @@ -343,4 +347,87 @@ (t/is (= (expected-at s i) (level-color s m i)) (str "level " i)))))]))})))) +(t/deftest case-n-geometry-sync-with-rotated-instances + ;; Regression sweep for #10109, with #13267's semantics as its complement: an + ;; instance root's transformation is inherited, overridable content — + ;; asymmetric to position, which is free per-instance placement. + ;; + ;; On the simple component-with-copy, sweep three axes: optionally rotate the + ;; COPY as a whole (an override of its root's placement — must not block + ;; propagation), optionally rotate the MAIN as a whole (inherited content — + ;; an untouched copy must follow it), and apply ONE of a property edit + ;; (fills) or a geometry edit (height) to the main child. In EVERY variant + ;; the chosen edit must arrive at the copy child; the copy's rotation is its + ;; own 45° if the copy was rotated (override wins), else the main's 45° if + ;; the main was rotated (clean copy follows), else 0; and only a rotated + ;; copy's ROOT is touched (:geometry-group) — its child merely follows and + ;; stays untouched. + (t/async + done + (let [rotate-copy (n/rotate :copy-root 45) + rotate-main (n/rotate :main-root 45) + edits (tm/one-of + [(n/change-attr :main-instance :fills red) + (n/change-height :main-instance 80)])] + (ftm/check + done + {:setup setup/simple-component-with-labeled-copy + :operation (tm/in-sequence [(tm/optional rotate-copy) + (tm/optional rotate-main) + edits])} + (fn [situation] + (let [chosen (tm/get-choice situation edits) + copy-root (setup/copy-root situation) + copy-child (setup/copy-instance situation) + expected-rotation (cond + (tm/applied? situation rotate-copy) 45 + (tm/applied? situation rotate-main) 45 + :else 0)] + ;; the chosen main edit arrived at the copy child, in every variant + (t/is (some? chosen)) + (t/is (n/has-property-of chosen copy-child)) + ;; the copy shows the expected rotation (own override > followed main > none) + (t/is (mth/close? (or (:rotation copy-root) 0) expected-rotation)) + (t/is (mth/close? (or (:rotation copy-child) 0) expected-rotation)) + ;; only a rotated copy's ROOT is an override; the child merely follows + (t/is (= (if (tm/applied? situation rotate-copy) #{:geometry-group} nil) + (:touched copy-root))) + (t/is (nil? (:touched copy-child))))))))) + +(t/deftest case-touched-copy-child-survives-main-rotation + ;; A copy whose ROOT is rotated (a geometry override) AND whose CHILD has its + ;; own geometry override (resized) must keep the child exactly in place when the + ;; MAIN is later rotated: the child's placement is its own, shielded by its + ;; touched :geometry-group. We capture the child's position relative to the copy + ;; root before the main rotation and assert it is unchanged after. + (t/async + done + (let [before (atom nil) + rel-pos (fn [situation] + (let [child (setup/copy-instance situation) + root (setup/copy-root situation)] + (-> (gpt/subtract (grc/rect->center (:selrect child)) + (grc/rect->center (:selrect root))) + (gpt/transform (:transform-inverse root (gmt/matrix)))))) + capture (tm/test-that (fn [s] (reset! before (rel-pos s)) (t/is (some? @before))))] + (ftm/check + done + {:setup setup/simple-component-with-labeled-copy + :operation (tm/in-sequence [(n/rotate :copy-root 30) + (n/change-height :copy-instance 60) + capture + (n/rotate :main-root 40)])} + (fn [situation] + (let [copy-child (setup/copy-instance situation) + copy-root (setup/copy-root situation) + after (rel-pos situation)] + ;; both overrides are recorded as geometry touches + (t/is (contains? (:touched copy-root) :geometry-group)) + (t/is (contains? (:touched copy-child) :geometry-group)) + ;; the child keeps its own overridden height + (t/is (mth/close? (:height copy-child) 60)) + ;; and its position relative to the copy root is unchanged by the main rotation + (t/is (mth/close? (:x @before) (:x after) 0.5) (str "rel-x " (:x @before) " -> " (:x after))) + (t/is (mth/close? (:y @before) (:y after) 0.5) (str "rel-y " (:y @before) " -> " (:y after))))))))) + diff --git a/frontend/test/frontend_tests/composable_tests/interpreter.cljs b/frontend/test/frontend_tests/composable_tests/interpreter.cljs index 1890fd0faf..0a6f3b9734 100644 --- a/frontend/test/frontend_tests/composable_tests/interpreter.cljs +++ b/frontend/test/frontend_tests/composable_tests/interpreter.cljs @@ -44,6 +44,7 @@ [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.shapes :as dwsh] [app.main.data.workspace.thumbnails :as dwth] + [app.main.data.workspace.transforms :as dwt] [app.main.data.workspace.undo :as dwu] [app.main.data.workspace.variants :as dwv] [app.main.store :as st] @@ -140,6 +141,19 @@ [(dwsh/update-shapes #{(tm/target-shape-id situation target)} (fn [shape] (n/set-property shape property value)))]) + (instance? n/Rotate op) + ;; The real sidebar rotation event, absolute mode: rotates the shape (and its + ;; whole subtree) around its center. Its apply-modifiers step runs the + ;; placement-vs-override classification for component copies (check-delta), + ;; which is part of what a case using `rotate` exercises. + (let [{:keys [target angle]} op] + [(dwt/increase-rotation [(tm/target-shape-id situation target)] angle)]) + + (instance? n/ChangeHeight op) + ;; The real sidebar dimension event. + (let [{:keys [target value]} op] + [(dwt/update-dimensions [(tm/target-shape-id situation target)] :height value)]) + (instance? n/SwapComponent op) ;; Swap lineage `name`'s nesting level `level` for lineage `target`'s component ;; via the REAL swap event (dwl/component-swap), so it commits through the