From 947954933c2de822b885b383fb055b1e44943ddd Mon Sep 17 00:00:00 2001 From: Alonso Torres Date: Fri, 11 Sep 2026 14:20:09 +0200 Subject: [PATCH] :bug: Fix problem with node splitting in paths (#11553) * :bug: Fix problem with node splitting in paths * :bug: Fix merge splitting node in paths --- common/src/app/common/types/path.cljc | 13 ++ common/src/app/common/types/path/segment.cljc | 169 ++++++++++++++- common/src/app/common/types/path/subpath.cljc | 71 ++++--- .../common_tests/types/path_data_test.cljc | 194 +++++++++++++++++- .../app/main/data/workspace/path/common.cljs | 2 +- .../app/main/data/workspace/path/drawing.cljs | 37 ++-- .../app/main/data/workspace/path/edition.cljs | 3 + .../app/main/data/workspace/path/helpers.cljs | 32 ++- .../app/main/data/workspace/path/tools.cljs | 71 ++++--- .../main/ui/workspace/shapes/path/editor.cljs | 14 +- .../main/ui/workspace/sidebar/options.cljs | 11 +- .../logic/path_actions_test.cljs | 42 +++- .../logic/path_lifecycle_test.cljs | 28 ++- .../frontend_tests/logic/path_tools_test.cljs | 122 +++++++++-- 14 files changed, 695 insertions(+), 114 deletions(-) diff --git a/common/src/app/common/types/path.cljc b/common/src/app/common/types/path.cljc index a198e57cfa..66996ddd3b 100644 --- a/common/src/app/common/types/path.cljc +++ b/common/src/app/common/types/path.cljc @@ -460,6 +460,19 @@ (let [content (impl/path-data content)] (segment/merge-nodes content points))) +(defn merge-coincident-nodes + "Collapses the nodes sharing a position into one node. + + Without `points` every position held by more than one command is merged." + ([content] + (let [content (impl/path-data content)] + (-> (segment/merge-coincident-nodes content) + (impl/from-plain)))) + ([content points] + (let [content (impl/path-data content)] + (-> (segment/merge-coincident-nodes content points) + (impl/from-plain))))) + (defn join-nodes "Creates new segments between points that weren't previously connected." [content points] diff --git a/common/src/app/common/types/path/segment.cljc b/common/src/app/common/types/path/segment.cljc index 82c83483ca..561dd4e44b 100644 --- a/common/src/app/common/types/path/segment.cljc +++ b/common/src/app/common/types/path/segment.cljc @@ -940,18 +940,21 @@ (not= :close-path (:command c))))] (loop [i 0 k 0 + start nil result (transient [])] (if (>= i n) (persistent! result) (let [cmd (nth content i) nxt (nth content (inc i) nil) + move? (= :move-to (:command cmd)) + start (if move? (helpers/segment->point cmd) start) at-p? (and (not= :close-path (:command cmd)) (gpt/close? point (helpers/segment->point cmd)))] (cond ;; Offset a subpath start. - (and at-p? (= :move-to (:command cmd))) + (and at-p? move?) (let [off (gpt/point (* k ox) (* k oy))] - (recur (inc i) (inc k) + (recur (inc i) (inc k) start (conj! result (-> cmd (update-in [:params :x] + (:x off)) (update-in [:params :y] + (:y off)))))) @@ -972,7 +975,7 @@ (= :curve-to (:command nxt)) (-> (update-in [:params :c1x] + (:x off2)) (update-in [:params :c1y] + (:y off2))))] - (recur (+ i 2) (inc k2) + (recur (+ i 2) (inc k2) start (-> result (conj! cmd') (conj! mv) (conj! nxt')))) ;; Open and offset a closed seam. @@ -985,12 +988,20 @@ (-> (update-in [:params :c2x] + (:x off)) (update-in [:params :c2y] + (:y off))))] ;; Drop the close command so the seam stays open. - (recur (+ i 2) (inc k) (conj! result cmd'))) + (recur (+ i 2) (inc k) start (conj! result cmd'))) + + ;; Open the seam of a subpath that closes back onto the node. + (and (= :close-path (:command cmd)) + (some? start) + (gpt/close? point start)) + (let [off (gpt/point (* k ox) (* k oy))] + (recur (inc i) (inc k) start + (conj! result (helpers/make-line-to (gpt/add point off))))) ;; Offset the end of an open subpath. (and at-p? (seg? cmd) (not= :close-path (:command nxt))) (let [off (gpt/point (* k ox) (* k oy))] - (recur (inc i) (inc k) + (recur (inc i) (inc k) start (conj! result (cond-> (-> cmd (update-in [:params :x] + (:x off)) (update-in [:params :y] + (:y off))) @@ -999,7 +1010,7 @@ (update-in [:params :c2y] + (:y off))))))) :else - (recur (inc i) k (conj! result cmd)))))))) + (recur (inc i) k start (conj! result cmd)))))))) (defn separate-nodes "Removes segments between points or splits one node into offset open ends." @@ -1072,7 +1083,7 @@ result (cond-> result (and (nil? set-a) (nil? set-b)) - (conj #{point-a point-b}) + (conj (hash-set point-a point-b)) (and (some? set-a) (nil? set-b)) (add-to-set set-a point-b) @@ -1108,6 +1119,144 @@ (->> content (mapv replace-command)))) +(defn- remove-empty-segments + "Drops segments with no length whose ends are accepted by `at-point?`." + [content at-point?] + (loop [result (transient []) + prev nil + segments? false + pending (seq content)] + (if-let [{:keys [command] :as segment} (first pending)] + (let [close? (= :close-path command) + move? (= :move-to command) + point (when-not close? (helpers/segment->point segment)) + ;; A close command on a subpath without segments draws nothing. + empty? (if close? + (not segments?) + (and (not move?) + (some? prev) + (gpt/close? prev point) + (at-point? point)))] + (if empty? + (recur result prev segments? (next pending)) + (recur (conj! result segment) + (if close? nil point) + (not (or move? close?)) + (next pending)))) + (persistent! result)))) + +(defn- point-key + "Rounded coordinates of a point, usable as a map key." + [point] + [(mth/round (:x point) 0.1) (mth/round (:y point) 0.1)]) + +(defn- curve-key + "Key for the curve a segment draws, equal in either direction." + [from segment to] + (let [c1 (or (get-handler segment :c1) from) + c2 (or (get-handler segment :c2) to) + fwd [(point-key from) (point-key c1) (point-key c2) (point-key to)] + bwd [(point-key to) (point-key c2) (point-key c1) (point-key from)]] + (if (neg? (compare fwd bwd)) fwd bwd))) + +(defn- node-point-groups + "Node positions of the content grouped by their rounded coordinates." + [content] + (group-by point-key + (into [] + (comp (remove #(= :close-path (:command %))) + (map helpers/segment->point)) + content))) + +(defn- coincident-points + "Positions of the content that more than one command holds." + [content] + (into #{} + (comp (filter (fn [[_ points]] (> (count points) 1))) + (map (fn [[_ points]] (first points)))) + (node-point-groups content))) + +(defn- repeated-nodes + "Rounded positions accepted by `at-point?` that more than one command holds." + [content at-point?] + (into #{} + (comp (filter (fn [[_ points]] + (and (> (count points) 1) + (at-point? (first points))))) + (map key)) + (node-point-groups content))) + +(defn- resume-segment + "Commands that reopen the subpath at `from` and draw `segment` from there." + [from segment start] + (if (= :close-path (:command segment)) + (when-not (subpath/pt= from start) + [(helpers/make-move-to from) (helpers/make-line-to start)]) + [(helpers/make-move-to from) segment])) + +(defn- remove-retraced-segments + "Drops the segments that draw a curve already drawn through a node. + + A node held by several commands is a junction, but two segments meeting + there and drawing the same curve are one line traced twice." + [content at-point?] + (let [repeated (repeated-nodes content at-point?) + retraced? (fn [from to] + (or (contains? repeated (point-key from)) + (contains? repeated (point-key to))))] + (if (empty? repeated) + content + (loop [result (transient []) + pending (seq content) + drawn #{} + from nil + start nil + lifted? false] + (if-let [{:keys [command] :as segment} (first pending)] + (if (= :move-to command) + (let [point (helpers/segment->point segment)] + (recur (conj! result segment) (next pending) drawn point point false)) + (let [to (if (= :close-path command) + start + (helpers/segment->point segment)) + key (curve-key from segment to)] + (if (and (contains? drawn key) + (retraced? from to)) + (recur result (next pending) drawn to start true) + (recur (reduce conj! result (if lifted? + (resume-segment from segment start) + [segment])) + (next pending) (conj drawn key) to start false)))) + (persistent! result)))))) + +(defn merge-coincident-nodes + "Collapses the commands sharing a position at `points` into a single node. + + Drops the empty segments and the ones retracing another through such a + point, and stitches the subpath ends meeting there, closing the resulting + loops. A point where more than two distinct segments meet is left alone: the + format needs one command per segment there. Without `points` every position + held by more than one command is merged." + ([content] + (merge-coincident-nodes content (coincident-points content))) + ([content points] + (let [at-point? (fn [point] (some #(gpt/close? point %) points)) + + stitch (fn [content] + (-> content + (subpath/close-subpaths at-point?) + ;; A subpath whose ends meet carries an explicit close command. + (subpath/close-loops))) + + content (-> (vec content) + (remove-empty-segments at-point?) + (stitch)) + + retraced (remove-retraced-segments content at-point?)] + (if (= retraced content) + content + (stitch retraced))))) + (defn merge-nodes "Joins and merges `points` into one point." [content points] @@ -1116,10 +1265,12 @@ (if (seq segments) (let [point->merge-point (-> segments (group-segments) - (calculate-merge-points points))] + (calculate-merge-points points)) + merge-points (set (vals point->merge-point))] (-> content (separate-nodes points) - (replace-points point->merge-point))) + (replace-points point->merge-point) + (merge-coincident-nodes merge-points))) content))) (defn transform-content diff --git a/common/src/app/common/types/path/subpath.cljc b/common/src/app/common/types/path/subpath.cljc index 95e16664f2..309ee8a450 100644 --- a/common/src/app/common/types/path/subpath.cljc +++ b/common/src/app/common/types/path/subpath.cljc @@ -99,25 +99,30 @@ (defn- merge-paths "Tries to merge into candidate the subpaths. Will return the candidate with the subpaths merged - and removed from subpaths the subpaths merged" - [candidate subpaths] - (let [merge-with-candidate + and removed from subpaths the subpaths merged. Only meeting points accepted + by `meet?` are joined" + [candidate subpaths meet?] + (let [joins? + (fn [point other] + (and (pt= point other) (meet? point))) + + merge-with-candidate (fn [[candidate result] current] (cond (pt= (:to current) (:from current)) ;; Subpath is already a closed path [candidate (conj result current)] - (pt= (:to candidate) (:from current)) + (joins? (:to candidate) (:from current)) [(subpaths-join candidate current) result] - (pt= (:from candidate) (:to current)) + (joins? (:from candidate) (:to current)) [(subpaths-join current candidate) result] - (pt= (:to candidate) (:to current)) + (joins? (:to candidate) (:to current)) [(subpaths-join candidate (reverse-subpath current)) result] - (pt= (:from candidate) (:from current)) + (joins? (:from candidate) (:from current)) [(subpaths-join (reverse-subpath current) candidate) result] :else @@ -163,35 +168,37 @@ (into [] xf-mapcat-data merged))) (defn close-subpaths - "Searches a path for possible subpaths that can create closed loops and merge them" - [content] - (let [subpaths (get-subpaths content) - closed-subpaths - (loop [result [] - current (first subpaths) - subpaths (rest subpaths)] + "Searches a path for possible subpaths that can create closed loops and merge them. + When `meet?` is given only subpaths that touch at an accepted point are merged" + ([content] + (close-subpaths content (constantly true))) + ([content meet?] + (let [subpaths (get-subpaths content) + closed-subpaths + (loop [result [] + current (first subpaths) + subpaths (rest subpaths)] - (if (some? current) - (let [[new-current new-subpaths] - (if (is-closed? current) - [current subpaths] - (merge-paths current subpaths))] + (if (some? current) + (let [[new-current new-subpaths] + (if (is-closed? current) + [current subpaths] + (merge-paths current subpaths meet?))] - (if (= current new-current) - ;; If equal we haven't found any matching subpaths we advance - (recur (conj result new-current) - (first new-subpaths) - (rest new-subpaths)) + (if (= current new-current) + ;; If equal we haven't found any matching subpaths we advance + (recur (conj result new-current) + (first new-subpaths) + (rest new-subpaths)) - ;; If different we need to pass again the merge to check for additional - ;; subpaths to join - (recur result - new-current - new-subpaths))) - result))] + ;; If different we need to pass again the merge to check for additional + ;; subpaths to join + (recur result + new-current + new-subpaths))) + result))] - - (into [] xf-mapcat-data closed-subpaths))) + (into [] xf-mapcat-data closed-subpaths)))) (defn- close-loop "Adds an explicit close command when a subpath's endpoints meet." diff --git a/common/test/common_tests/types/path_data_test.cljc b/common/test/common_tests/types/path_data_test.cljc index 70285f21ad..94f96d8870 100644 --- a/common/test/common_tests/types/path_data_test.cljc +++ b/common/test/common_tests/types/path_data_test.cljc @@ -1373,6 +1373,20 @@ (t/is (= {:c2x 4.0 :c2y 4.0} (select-keys (:params (peek result)) [:c2x :c2y]))))) +(t/deftest segment-separate-single-node-closed-subpath-start + ;; The seam of a closed subpath opens even when it is the subpath start. + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 10.0}} + {:command :close-path :params {}}]) + result (vec (path/separate-nodes content #{(gpt/point 0.0 0.0)}))] + ;; the close command becomes the second, offset, open end + (t/is (= [:move-to :line-to :line-to :line-to] (mapv :command result))) + (t/is (= [{:x 0.0 :y 0.0} {:x 10.0 :y 0.0} + {:x 10.0 :y 10.0} {:x 8.0 :y 8.0}] + (mapv #(select-keys (:params %) [:x :y]) result))))) + (t/deftest segment-separate-single-node-endpoint-noop ;; an endpoint node has no following segment, so nothing is split (let [content (path/content @@ -2092,7 +2106,7 @@ (t/is (some? result))))) (t/deftest path-merge-disconnected-nodes - ;; Merging separate subpaths joins them at the shared midpoint. + ;; Merging separate subpaths stitches them into one at the shared midpoint. (let [content (path/content [{:command :move-to :params {:x 0.0 :y 0.0}} {:command :line-to :params {:x 10.0 :y 0.0}} @@ -2100,10 +2114,184 @@ {:command :line-to :params {:x 10.0 :y 10.0}}]) pts #{(gpt/point 10.0 0.0) (gpt/point 0.0 10.0)} result (vec (path/merge-nodes content pts))] - (t/is (= [{:x 0.0 :y 0.0} {:x 5.0 :y 5.0} - {:x 5.0 :y 5.0} {:x 10.0 :y 10.0}] + (t/is (= [:move-to :line-to :line-to] (mapv :command result))) + (t/is (= [{:x 0.0 :y 0.0} {:x 5.0 :y 5.0} {:x 10.0 :y 10.0}] (mapv :params result))))) +(t/deftest path-merge-nodes-leaves-a-single-node + ;; The merged node exists once, so separating it yields a fresh split. + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 10.0}} + {:command :move-to :params {:x 20.0 :y 0.0}} + {:command :line-to :params {:x 12.0 :y 12.0}}]) + merged (path/merge-nodes content #{(gpt/point 10.0 10.0) + (gpt/point 12.0 12.0)}) + node (gpt/point 11.0 11.0)] + (t/is (= 1 (count (path/point-indices merged node)))) + ;; separating splits the node in two ends, none of them the merged nodes + (let [result (vec (path/separate-nodes merged #{node} (gpt/point 8.0 8.0)))] + (t/is (= [{:x 0.0 :y 0.0} {:x 11.0 :y 11.0} + {:x 19.0 :y 19.0} {:x 20.0 :y 0.0}] + (mapv #(select-keys (:params %) [:x :y]) result)))))) + +(t/deftest path-merge-nodes-on-empty-segment + ;; Merging across an empty segment returns a content instead of throwing + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 20.0 :y 0.0}}])] + (t/is (some? (path/merge-nodes content #{(gpt/point 0.0 0.0) + (gpt/point 20.0 0.0)}))))) + +(t/deftest path-merge-coincident-nodes-stitches-dragged-ends + ;; Two open ends left at the same position become one node + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 10.0}} + {:command :move-to :params {:x 20.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 10.0}}]) + result (vec (path/merge-coincident-nodes content #{(gpt/point 10.0 10.0)}))] + (t/is (= [:move-to :line-to :line-to] (mapv :command result))) + (t/is (= [{:x 0.0 :y 0.0} {:x 10.0 :y 10.0} {:x 20.0 :y 0.0}] + (mapv :params result))))) + +(t/deftest path-merge-coincident-nodes-drops-empty-segment + ;; A node dragged onto its neighbour leaves no segment behind + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 20.0 :y 0.0}}]) + result (vec (path/merge-coincident-nodes content #{(gpt/point 0.0 0.0)}))] + (t/is (= [:move-to :line-to] (mapv :command result))) + (t/is (= [{:x 0.0 :y 0.0} {:x 20.0 :y 0.0}] (mapv :params result))))) + +(t/deftest path-merge-coincident-nodes-closes-the-loop + ;; Dragging both ends of a subpath together closes it + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 0.0}} + {:command :line-to :params {:x 0.0 :y 0.0}}]) + result (vec (path/merge-coincident-nodes content #{(gpt/point 0.0 0.0)}))] + (t/is (= [:move-to :line-to :close-path] (mapv :command result))))) + +(t/deftest path-merge-coincident-nodes-only-at-given-points + ;; Subpaths touching somewhere else are left alone + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 10.0}} + {:command :move-to :params {:x 20.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 10.0}}]) + result (path/merge-coincident-nodes content #{(gpt/point 20.0 0.0)})] + (t/is (= (vec content) (vec result))))) + +(t/deftest path-merge-coincident-nodes-keeps-closed-subpaths + ;; Closed subpaths keep their close command, wherever the merge happens + (let [rect (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 10.0}} + {:command :line-to :params {:x 0.0 :y 10.0}} + {:command :close-path :params {}}]) + curve (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :curve-to :params {:c1x 2.0 :c1y 2.0 :c2x 8.0 :c2y 8.0 + :x 10.0 :y 10.0}} + {:command :curve-to :params {:c1x 8.0 :c1y -8.0 :c2x 2.0 :c2y -2.0 + :x 0.0 :y 0.0}} + {:command :close-path :params {}}])] + (t/is (= (vec rect) (vec (path/merge-coincident-nodes rect #{(gpt/point 10.0 0.0)})))) + (t/is (= (vec rect) (vec (path/merge-coincident-nodes rect #{(gpt/point 0.0 0.0)})))) + (t/is (= (vec curve) (vec (path/merge-coincident-nodes curve #{(gpt/point 0.0 0.0)})))))) + +(t/deftest path-merge-coincident-nodes-keeps-junctions + ;; Four distinct segments meeting at a point need one command each + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 5.0 :y 5.0}} + {:command :line-to :params {:x 10.0 :y 0.0}} + {:command :move-to :params {:x 0.0 :y 10.0}} + {:command :line-to :params {:x 5.0 :y 5.0}} + {:command :line-to :params {:x 10.0 :y 10.0}}]) + result (path/merge-coincident-nodes content #{(gpt/point 5.0 5.0)})] + (t/is (= (vec content) (vec result))))) + +(t/deftest path-merge-coincident-nodes-drops-a-retraced-segment + ;; The rest of the loop draws the same two lines backwards; dropping them + ;; leaves a single node where they meet. + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 5.0}} + {:command :line-to :params {:x 20.0 :y 10.0}} + {:command :curve-to :params {:c1x 20.0 :c1y 10.0 + :c2x 10.0 :c2y 5.0 + :x 10.0 :y 5.0}} + {:command :close-path :params {}}]) + result (vec (path/merge-coincident-nodes content #{(gpt/point 10.0 5.0)}))] + (t/is (= [:move-to :line-to :line-to] (mapv :command result))) + (t/is (= [{:x 0.0 :y 0.0} {:x 10.0 :y 5.0} {:x 20.0 :y 10.0}] + (mapv :params result))) + (t/is (= 1 (count (path/point-indices result (gpt/point 10.0 5.0))))))) + +(t/deftest path-merge-coincident-nodes-stitches-a-retraced-junction + ;; The same two lines, drawn out and back from the subpath start + (let [content (path/content + [{:command :move-to :params {:x 5.0 :y 5.0}} + {:command :line-to :params {:x 10.0 :y 0.0}} + {:command :line-to :params {:x 5.0 :y 5.0}} + {:command :line-to :params {:x 0.0 :y 10.0}} + {:command :close-path :params {}}]) + result (vec (path/merge-coincident-nodes content #{(gpt/point 5.0 5.0)}))] + (t/is (= [:move-to :line-to :line-to] (mapv :command result))) + (t/is (= [{:x 0.0 :y 10.0} {:x 5.0 :y 5.0} {:x 10.0 :y 0.0}] + (mapv :params result))))) + +(t/deftest path-merge-coincident-nodes-collapses-every-repeated-node + ;; Without points every position held by more than one command is merged + (let [content (path/content + [{:command :move-to :params {:x 119.0 :y 231.0}} + {:command :line-to :params {:x 447.0 :y 253.0}} + {:command :curve-to :params {:c1x 447.0 :c1y 253.0 + :c2x 774.0 :c2y 384.0 + :x 774.0 :y 384.0}} + {:command :curve-to :params {:c1x 774.0 :c1y 384.0 + :c2x 447.0 :c2y 253.0 + :x 447.0 :y 253.0}} + {:command :close-path :params {}}]) + result (vec (path/merge-coincident-nodes content))] + (t/is (= [:move-to :line-to :curve-to] (mapv :command result))) + (t/is (= 1 (count (path/point-indices result (gpt/point 447.0 253.0))))))) + +(t/deftest path-merge-coincident-nodes-keeps-distinct-curves + ;; Two different curves between the same two points are not a retrace + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :curve-to :params {:c1x 2.0 :c1y 2.0 :c2x 8.0 :c2y 8.0 + :x 10.0 :y 10.0}} + {:command :curve-to :params {:c1x 8.0 :c1y -8.0 :c2x 2.0 :c2y -2.0 + :x 0.0 :y 0.0}} + {:command :line-to :params {:x 0.0 :y 20.0}}]) + result (path/merge-coincident-nodes content #{(gpt/point 0.0 0.0)})] + (t/is (= (vec content) (vec result))))) + +(t/deftest path-separate-nodes-after-merge-yields-one-end-per-line + ;; A node with two visible lines separates into two ends. + (let [content (path/content + [{:command :move-to :params {:x 0.0 :y 0.0}} + {:command :line-to :params {:x 10.0 :y 5.0}} + {:command :line-to :params {:x 20.0 :y 10.0}} + {:command :curve-to :params {:c1x 20.0 :c1y 10.0 + :c2x 10.0 :c2y 5.0 + :x 10.0 :y 5.0}} + {:command :close-path :params {}}]) + node (gpt/point 10.0 5.0) + merged (path/merge-coincident-nodes content #{node}) + result (vec (path/separate-nodes merged #{node} (gpt/point 4.0 4.0)))] + (t/is (= [:move-to :line-to :move-to :line-to] (mapv :command result))) + (t/is (= [{:x 0.0 :y 0.0} {:x 10.0 :y 5.0} + {:x 14.0 :y 9.0} {:x 20.0 :y 10.0}] + (mapv #(select-keys (:params %) [:x :y]) result))))) + (t/deftest path-duplicate-node-content ;; Duplicating a node copies its incident segments as subpaths. (let [content (path/content diff --git a/frontend/src/app/main/data/workspace/path/common.cljs b/frontend/src/app/main/data/workspace/path/common.cljs index e2f3a06c61..783d073081 100644 --- a/frontend/src/app/main/data/workspace/path/common.cljs +++ b/frontend/src/app/main/data/workspace/path/common.cljs @@ -15,7 +15,7 @@ (defn clean-edit-state [state] - (dissoc state :last-point :prev-handler :drag-handler :preview)) + (dissoc state :last-point :prev-handler :drag-handler :preview :pending-start)) (defn- drop-trailing-move-to "Drops a trailing subpath start without segments." diff --git a/frontend/src/app/main/data/workspace/path/drawing.cljs b/frontend/src/app/main/data/workspace/path/drawing.cljs index fa4db3cf7f..f2a0ab1377 100644 --- a/frontend/src/app/main/data/workspace/path/drawing.cljs +++ b/frontend/src/app/main/data/workspace/path/drawing.cljs @@ -113,7 +113,9 @@ (update [_ state] (let [id (st/get-path-id state) fix-angle? shift? - {:keys [last-point prev-handler]} (get-in state [:workspace-local :edit-path id]) + {:keys [last-point prev-handler pending-start]} + (get-in state [:workspace-local :edit-path id]) + position (cond-> (gpt/point x y) fix-angle? (path.helpers/position-fixed-angle last-point))] (if-not (= last-point position) @@ -121,6 +123,9 @@ (assoc-in [:workspace-local :edit-path id :last-point] position) (update-in [:workspace-local :edit-path id] dissoc :prev-handler) (update-in [:workspace-local :edit-path id] dissoc :preview) + (update-in [:workspace-local :edit-path id] dissoc :pending-start) + (cond-> (some? pending-start) + (update-in (st/get-path-location state) helpers/start-subpath pending-start)) (update-in (st/get-path-location state) helpers/append-node position last-point prev-handler)) state))))) @@ -398,16 +403,19 @@ (cond-> (some? drop-index) (with-meta {:index drop-index}))))))))) -(defn- close-drawn-loops - "Adds explicit close commands to completed loops." +(defn- clean-drawn-content + "Collapses the nodes drawn on top of each other and closes completed loops. + + Clicking a node already in the path draws its segments again backwards, and + only one copy of each line is kept." [] - (ptk/reify ::close-drawn-loops + (ptk/reify ::clean-drawn-content ptk/UpdateEvent (update [_ state] (d/update-in-when state [:workspace-drawing :object] (fn [object] (-> object - (update :content path/close-loops) + (update :content path/merge-coincident-nodes) (path/update-geometry))))))) (defn- handle-drawing-end @@ -427,13 +435,13 @@ (cond (and (> (count content) 1) restart?) (rx/of (common/finish-path) - (close-drawn-loops) + (clean-drawn-content) (setup-frame) (dwdc/handle-finish-drawing) (start-created-path-edition shape-id)) (> (count content) 1) - (rx/of (close-drawn-loops) + (rx/of (clean-drawn-content) (setup-frame) (dwdc/handle-finish-drawing) (dwe/clear-edition-mode)) @@ -529,16 +537,11 @@ pos (helpers/node-position content index) last-idx (dec (count content)) tip? (and (= index last-idx) - (not= :close-path (:command (nth content index nil)))) - state (assoc-in state [:workspace-local :edit-path id :last-point] pos)] - (if tip? - state - (update-in state (st/get-path-location state) - (fn [shape] - (-> shape - (update :content path/append-segment - {:command :move-to :params (select-keys pos [:x :y])}) - (path/update-geometry)))))) + (not= :close-path (:command (nth content index nil))))] + (cond-> (assoc-in state [:workspace-local :edit-path id :last-point] pos) + ;; A tip already ends the content; an inner node needs its own start. + (not tip?) + (assoc-in [:workspace-local :edit-path id :pending-start] pos))) state))) (defn change-edit-mode diff --git a/frontend/src/app/main/data/workspace/path/edition.cljs b/frontend/src/app/main/data/workspace/path/edition.cljs index 05b40fd92c..2186500b8a 100644 --- a/frontend/src/app/main/data/workspace/path/edition.cljs +++ b/frontend/src/app/main/data/workspace/path/edition.cljs @@ -470,6 +470,7 @@ (rx/map #(move-selected-path-point start-position %)) (rx/take-until stopper)) (rx/of (apply-content-modifiers) + (tools/merge-coincident-nodes) (merge-dragged-on-drop))))))) (declare drag-selected-segments) @@ -559,6 +560,7 @@ (rx/map #(move-selected-path-segment start-position %)) (rx/take-until stopper)) (rx/of (apply-content-modifiers) + (tools/merge-coincident-nodes) (merge-dragged-on-drop)))))))) (defn bend-segment-modifier @@ -757,6 +759,7 @@ (rx/of (move-selected direction shift?))) (rx/of (apply-content-modifiers) + (tools/merge-coincident-nodes) (finish-move-selected)))) (rx/empty))))))) diff --git a/frontend/src/app/main/data/workspace/path/helpers.cljs b/frontend/src/app/main/data/workspace/path/helpers.cljs index 5f28983e78..2ca0a9b760 100644 --- a/frontend/src/app/main/data/workspace/path/helpers.cljs +++ b/frontend/src/app/main/data/workspace/path/helpers.cljs @@ -15,6 +15,12 @@ [app.common.types.path :as path] [app.common.types.path.helpers :as path.helpers])) +(defn start-subpath + "Adds the subpath start a pending node draws its first segment from." + [shape position] + (update shape :content path/append-segment + {:command :move-to :params (select-keys position [:x :y])})) + (defn append-node "Creates a new node in the path. Usually used when drawing." [shape position prev-point prev-handler] @@ -331,17 +337,33 @@ (remove nil?)) (segment-entries content)))) +(defn coincident-node-indices + "Adds to `indices` every other command sharing one of their positions. + + Commands at the same position are one node: they move together, so an + action cannot depend on which of them the selection holds." + [content indices] + (let [indices (into #{} (filter #(node? content %)) indices)] + (into indices + (mapcat #(path/point-indices content %)) + (node-positions content indices)))) + +(defn selected-node-count + "Number of nodes in the selection, counting coincident commands as one." + [content selection] + (count (node-positions content (get selection :nodes #{})))) + (defn check-enabled "Returns path actions enabled for selected node indices." [content selected-nodes] (when content - (let [selected-nodes (into #{} (filter #(node? content %)) selected-nodes) + (let [selected-nodes (coincident-node-indices content selected-nodes) selected-segments (filter (fn [{:keys [from-index to-index]}] (and (contains? selected-nodes from-index) (contains? selected-nodes to-index))) (segment-entries content)) num-segments (count selected-segments) - num-nodes (count selected-nodes) + num-nodes (count (node-positions content selected-nodes)) nodes-selected? (seq selected-nodes) segments-selected? (seq selected-segments) max-segments (/ (* num-nodes (dec num-nodes)) 2) @@ -553,6 +575,12 @@ (let [selection (or selection empty-selection)] (if (= (count old-content) (count new-content)) (-> selection + ;; Drop indices that stopped being nodes. + (update :nodes + (fn [nodes] + (into #{} + (filter #(node? new-content %)) + nodes))) (update :handlers (fn [handlers] (into #{} diff --git a/frontend/src/app/main/data/workspace/path/tools.cljs b/frontend/src/app/main/data/workspace/path/tools.cljs index 364115b79e..6b5ef63197 100644 --- a/frontend/src/app/main/data/workspace/path/tools.cljs +++ b/frontend/src/app/main/data/workspace/path/tools.cljs @@ -142,18 +142,22 @@ (make-curve point))))))))) (defn- update-path-content - "Updates path content, geometry, selection, and handler types." - [state new-content] - (let [id (st/get-path-id state) - old-content (st/get-path state :content)] - (-> (cond-> (st/set-content state new-content) - (seq new-content) - (update-in (st/get-path-location state) path/update-geometry)) - (update-in [:workspace-local :edit-path id :selection] - #(helpers/remap-selection % old-content new-content)) - (update-in [:workspace-local :edit-path id :handler-types] - #(helpers/remap-handler-types % old-content new-content)) - (update-in [:workspace-local :edit-path id] dissoc :edited-handler)))) + "Updates path content, geometry, selection, and handler types. + + The selection is remapped by position, so a tool that moves nodes before + changing the content structure passes the moved content as `old-content`." + ([state new-content] + (update-path-content state (st/get-path state :content) new-content)) + ([state old-content new-content] + (let [id (st/get-path-id state)] + (-> (cond-> (st/set-content state new-content) + (seq new-content) + (update-in (st/get-path-location state) path/update-geometry)) + (update-in [:workspace-local :edit-path id :selection] + #(helpers/remap-selection % old-content new-content)) + (update-in [:workspace-local :edit-path id :handler-types] + #(helpers/remap-handler-types % old-content new-content)) + (update-in [:workspace-local :edit-path id] dissoc :edited-handler))))) (defn remove-segments "Removes segments and opens the path at their endpoints." @@ -234,6 +238,30 @@ (defn merge-nodes [] (process-path-tool path/merge-nodes)) +(defn- merge-coincident + "Collapses the nodes of `indices` sharing a position with another node." + [content indices] + (path/merge-coincident-nodes content (helpers/node-positions content indices))) + +(defn merge-coincident-nodes + "Merges the selected nodes sharing a position with another node. + + Runs after a move, which leaves the nodes it brings together as one + command each." + [] + (ptk/reify ::merge-coincident-nodes + ptk/UpdateEvent + (update [_ state] + (let [id (st/get-path-id state) + content (st/get-path state :content) + indices (helpers/selected-node-indices content (st/get-selection state id)) + new-content (when (and (some? content) (seq indices)) + (merge-coincident content indices))] + (if (and (some? new-content) + (not= (vec new-content) (vec content))) + (update-path-content state new-content) + state))))) + (defn join-nodes [] (process-path-tool path/join-nodes)) @@ -288,9 +316,8 @@ indices (if (seq selected) selected (helpers/node-indices content)) - content (path/flip-content content indices axis)] - (-> (st/set-content state content) - (update-in (st/get-path-location state) path/update-geometry)))))) + flipped (path/flip-content content indices axis)] + (update-path-content state flipped (merge-coincident flipped indices)))))) (defn align-nodes "Aligns selected nodes and their handles within their bounds." @@ -301,9 +328,8 @@ (let [id (st/get-path-id state) content (st/get-path state :content) selected (get (st/get-selection state id) :nodes #{}) - content (path/align-content content selected axis)] - (-> (st/set-content state content) - (update-in (st/get-path-location state) path/update-geometry)))))) + aligned (path/align-content content selected axis)] + (update-path-content state aligned (merge-coincident aligned selected)))))) (defn distribute-nodes "Distributes selected nodes evenly along `axis`." @@ -314,9 +340,8 @@ (let [id (st/get-path-id state) content (st/get-path state :content) selected (get (st/get-selection state id) :nodes #{}) - content (path/distribute-content content selected axis)] - (-> (st/set-content state content) - (update-in (st/get-path-location state) path/update-geometry)))))) + spread (path/distribute-content content selected axis)] + (update-path-content state spread (merge-coincident spread selected)))))) (defn- axis-point "Copy of `p` with `axis` (`:x`/`:y`) replaced by `value`." @@ -386,8 +411,8 @@ (cond-> content (seq node-idx) (path/set-nodes-coordinate node-idx axis value) (seq pts) (path/set-handler-points pts))))] - (-> (st/set-content state new-content) - (update-in (st/get-path-location state) path/update-geometry)))))) + (update-path-content state new-content + (merge-coincident new-content node-idx)))))) (defn toggle-snap [] (ptk/reify ::toggle-snap diff --git a/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs b/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs index ef886cf5dc..e716223909 100644 --- a/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs @@ -467,6 +467,7 @@ drag-handler prev-handler preview + last-point content-modifiers selection moving-nodes @@ -534,9 +535,6 @@ (mf/with-memo [content selected-segments] (dwp.helpers/segment-node-indices content selected-segments)) - last-p - (->> content last path.helpers/segment->point) - handlers (mf/with-memo [content] (path/get-handlers content)) @@ -589,7 +587,7 @@ (and is-hover (some? hover-point))))}])) (when (and preview (not drag-handler)) [:> path-preview* {:segment preview - :from last-p + :from last-point :zoom zoom}]) ;; Let insertion preview clicks reach the segment. @@ -600,9 +598,9 @@ :is-new true :zoom zoom}]]) - (when (and drag-handler last-p) + (when (and drag-handler last-point) [:g.drag-handler {:pointer-events "none"} - [:> path-handler* {:point last-p + [:> path-handler* {:point last-point :handler drag-handler :edit-mode edit-mode :zoom zoom}]]) @@ -624,10 +622,10 @@ :drag-cursor drag-cursor :any-node-selected any-node-selected?}]) - (when (and prev-handler last-p) + (when (and prev-handler last-point) [:g.prev-handler [:> path-handler* - {:point last-p + {:point last-point :edit-mode edit-mode :handler prev-handler :zoom zoom diff --git a/frontend/src/app/main/ui/workspace/sidebar/options.cljs b/frontend/src/app/main/ui/workspace/sidebar/options.cljs index d607168954..9229b36f70 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options.cljs @@ -15,6 +15,7 @@ [app.main.data.helpers :as dsh] [app.main.data.workspace :as udw] [app.main.data.workspace.common :as dwc] + [app.main.data.workspace.path.helpers :as path.helpers] [app.main.data.workspace.path.state :as path.state] [app.main.features :as features] [app.main.refs :as refs] @@ -115,8 +116,16 @@ path-editing? (path.state/editing? edit-path edition) + path-content + (dm/get-in drawing [:object :content]) + + selected-nodes + (:nodes (:selection edit-path-state)) + + ;; Coincident commands are one node, so count positions. path-node-count - (count (dm/get-in edit-path-state [:selection :nodes])) + (mf/with-memo [path-content selected-nodes] + (path.helpers/selected-node-count path-content {:nodes selected-nodes})) files (mf/deref refs/files) diff --git a/frontend/test/frontend_tests/logic/path_actions_test.cljs b/frontend/test/frontend_tests/logic/path_actions_test.cljs index b191f7e8a4..0d0e6f0a6c 100644 --- a/frontend/test/frontend_tests/logic/path_actions_test.cljs +++ b/frontend/test/frontend_tests/logic/path_actions_test.cljs @@ -7,6 +7,7 @@ (ns frontend-tests.logic.path-actions-test (:require [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] [app.common.types.path :as path] [app.main.data.workspace.path.helpers :as path.helpers] [app.main.ui.workspace.viewport.path-actions :as path.actions] @@ -22,7 +23,8 @@ (t/is (true? (:make-corner enabled))) (t/is (true? (:make-curve enabled))))) -(t/deftest action-eligibility-keeps-coincident-node-identities +(t/deftest action-eligibility-treats-coincident-commands-as-one-node + ;; Both commands sit at (0,0): one node, with a corner and a curve on it. (let [content (path/content [{:command :move-to :params {:x 0 :y 0}} {:command :line-to :params {:x 10 :y 0}} @@ -32,8 +34,42 @@ enabled (path.helpers/check-enabled content #{0 2})] (t/is (true? (:make-corner enabled))) (t/is (true? (:make-curve enabled))) - (t/is (true? (:merge-nodes enabled))) - (t/is (true? (:join-nodes enabled))))) + ;; there is a single node, so there is nothing to merge it with + (t/is (false? (:merge-nodes enabled))) + (t/is (false? (:join-nodes enabled))) + (t/is (true? (:separate-nodes enabled))))) + +(t/deftest a-junction-node-offers-the-same-actions-however-it-is-selected + ;; Three lines meeting at (50,50); a rubber band catches every command + ;; there while a click catches one of them. + (let [content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :line-to :params {:x 50 :y 50}} + {:command :move-to :params {:x 100 :y 0}} + {:command :line-to :params {:x 50 :y 50}} + {:command :move-to :params {:x 50 :y 100}} + {:command :line-to :params {:x 50 :y 50}}]) + in-rect (path.helpers/nodes-in-rect content (grc/make-rect 45 45 10 10)) + clicked (path.helpers/check-enabled content #{1}) + dragged (path.helpers/check-enabled content in-rect)] + (t/is (= #{1 3 5} in-rect)) + (t/is (= clicked dragged)) + ;; and they are the actions of a single node + (t/is (false? (:merge-nodes dragged))) + (t/is (false? (:join-nodes dragged))) + (t/is (true? (:separate-nodes dragged))) + ;; two distinct nodes offer the multiple-node actions + (t/is (true? (:merge-nodes (path.helpers/check-enabled content #{1 4})))))) + +(t/deftest selected-node-count-counts-a-junction-once + (let [content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :line-to :params {:x 50 :y 50}} + {:command :move-to :params {:x 100 :y 0}} + {:command :line-to :params {:x 50 :y 50}}])] + (t/is (= 1 (path.helpers/selected-node-count content {:nodes #{1 3}}))) + (t/is (= 2 (path.helpers/selected-node-count content {:nodes #{0 1}}))) + (t/is (= 0 (path.helpers/selected-node-count content {}))))) (t/deftest toolbar-separators-only-render-between-visible-tool-groups (t/are [structural? shape? handler? expected] diff --git a/frontend/test/frontend_tests/logic/path_lifecycle_test.cljs b/frontend/test/frontend_tests/logic/path_lifecycle_test.cljs index c73eddd205..1e88902f90 100644 --- a/frontend/test/frontend_tests/logic/path_lifecycle_test.cljs +++ b/frontend/test/frontend_tests/logic/path_lifecycle_test.cljs @@ -161,7 +161,7 @@ (run-handle-drawing-end false (fn [emissions] - (t/is (= [::path.drawing/close-drawn-loops + (t/is (= [::path.drawing/clean-drawn-content ::path.drawing/setup-frame ::dwdc/handle-finish-drawing ::dwe/clear-edition-mode] @@ -175,13 +175,37 @@ true (fn [emissions] (t/is (= [::path.common/finish-path - ::path.drawing/close-drawn-loops + ::path.drawing/clean-drawn-content ::path.drawing/setup-frame ::dwdc/handle-finish-drawing ::path.drawing/start-created-path-edition] (mapv ptk/type emissions))) (done))))) +(t/deftest ending-a-draw-collapses-the-nodes-drawn-on-top-of-each-other + (t/async + done + (run-handle-drawing-end + false + (fn [emissions] + ;; A path drawn back onto one of its own nodes and then closed. + (let [clean (first (filter #(= ::path.drawing/clean-drawn-content (ptk/type %)) + emissions)) + state {:workspace-drawing + {:object {:id (random-uuid) + :type :path + :content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :line-to :params {:x 10 :y 5}} + {:command :line-to :params {:x 20 :y 10}} + {:command :line-to :params {:x 10 :y 5}} + {:command :close-path :params {}}])}}} + content (-> (ptk/update clean state) + (get-in [:workspace-drawing :object :content]))] + (t/is (= [:move-to :line-to :line-to] (mapv :command (vec content)))) + (t/is (= 1 (count (path/point-indices content (gpt/point 10.0 5.0))))) + (done)))))) + (t/deftest escape-with-pending-segment-cancels-it-and-keeps-drawing (let [id (random-uuid) state {:workspace-local diff --git a/frontend/test/frontend_tests/logic/path_tools_test.cljs b/frontend/test/frontend_tests/logic/path_tools_test.cljs index 0c4bd36ead..f14888caac 100644 --- a/frontend/test/frontend_tests/logic/path_tools_test.cljs +++ b/frontend/test/frontend_tests/logic/path_tools_test.cljs @@ -306,8 +306,11 @@ state' (ptk/update (path.tools/set-selection-coordinate :x 5) state) content' (get-in state' [:workspace-drawing :object :content])] (t/is (= (gpt/point 5 0) (path.helpers/node-position content' 0))) - (t/is (= (gpt/point 5 0) (path.helpers/node-position content' 2))) - (t/is (= (gpt/point 10 0) (path.helpers/node-position content' 1))))) + (t/is (= (gpt/point 10 0) (path.helpers/node-position content' 1))) + ;; both ends land on (5,0), where they merge and close the subpath + (t/is (= [:move-to :line-to :close-path] (mapv :command (vec content')))) + ;; the merged node stays selected + (t/is (= #{0} (get-in state' [:workspace-local :edit-path id :selection :nodes]))))) ;; a coincident closed-seam node moves as one logical node (let [id (random-uuid) content (path/content @@ -319,7 +322,8 @@ state' (ptk/update (path.tools/set-selection-coordinate :y 7) state) content' (get-in state' [:workspace-drawing :object :content])] (t/is (= (gpt/point 0 7) (path.helpers/node-position content' 0))) - (t/is (= (gpt/point 0 7) (path.helpers/node-position content' 2)))) + ;; the seam is one node, so the subpath closes on it + (t/is (= [:move-to :line-to :close-path] (mapv :command (vec content'))))) ;; a selected handler on an independent node moves only its own control point (let [id (random-uuid) content (path/content @@ -359,17 +363,17 @@ [{:command :move-to :params {:x 0 :y 0}} {:command :line-to :params {:x 10 :y 0}} {:command :line-to :params {:x 20 :y 0}}])] - ;; a middle node: opens a new subpath (move-to) at the node and makes it the - ;; pending origin, so the next click draws a line from it + ;; a middle node: becomes the pending origin of a new subpath, which stays + ;; out of the content until the next click draws a line from it (let [state (pth/selectable-path-state id content {:nodes #{1} :segments #{} :handlers #{}}) state' (ptk/update (path.drawing/change-edit-mode :draw) state) content' (get-in state' [:workspace-drawing :object :content])] (t/is (= (gpt/point 10 0) (get-in state' [:workspace-local :edit-path id :last-point]))) - (t/is (= 4 (count content'))) - (t/is (= :move-to (:command (nth content' 3)))) - (t/is (= (gpt/point 10 0) (path.helpers/node-position content' 3)))) + (t/is (= (gpt/point 10 0) + (get-in state' [:workspace-local :edit-path id :pending-start]))) + (t/is (= (vec content) (vec content')))) ;; the drawing tip: just becomes the pending origin (extends), no new subpath (let [state (pth/selectable-path-state id content {:nodes #{2} :segments #{} :handlers #{}}) @@ -377,6 +381,7 @@ content' (get-in state' [:workspace-drawing :object :content])] (t/is (= (gpt/point 20 0) (get-in state' [:workspace-local :edit-path id :last-point]))) + (t/is (nil? (get-in state' [:workspace-local :edit-path id :pending-start]))) (t/is (= 3 (count content')))) ;; nothing selected: no pending line (let [state (pth/selectable-path-state id content @@ -415,7 +420,8 @@ content' (get-in state' [:workspace-drawing :object :content])] (t/is (= (gpt/point 20 0) (path.helpers/node-position content' 0))) (t/is (= (gpt/point 20 10) (path.helpers/node-position content' 3))) - (t/is (= (gpt/point 20 0) (path.helpers/node-position content' 4))))) + ;; the seam commands merge into the subpath close + (t/is (= :close-path (:command (nth (vec content') 4)))))) (t/deftest set-selection-coordinate-translates-mixed-segment-and-node-selection ;; Selected segments and nodes translate as one group. @@ -428,12 +434,12 @@ ;; The combined bounds start at x=0. state (pth/selectable-path-state id content {:nodes #{0} :segments #{3} :handlers #{}}) - state' (ptk/update (path.tools/set-selection-coordinate :x 10) state) + state' (ptk/update (path.tools/set-selection-coordinate :x 5) state) content' (get-in state' [:workspace-drawing :object :content])] - (t/is (= (gpt/point 10 0) (path.helpers/node-position content' 0))) + (t/is (= (gpt/point 5 0) (path.helpers/node-position content' 0))) (t/is (= (gpt/point 10 0) (path.helpers/node-position content' 1))) - (t/is (= (gpt/point 30 0) (path.helpers/node-position content' 2))) - (t/is (= (gpt/point 40 0) (path.helpers/node-position content' 3))))) + (t/is (= (gpt/point 25 0) (path.helpers/node-position content' 2))) + (t/is (= (gpt/point 35 0) (path.helpers/node-position content' 3))))) (t/deftest set-selection-coordinate-translates-mixed-segment-and-handler-selection ;; Standalone selected handlers translate with the group. @@ -801,6 +807,96 @@ (t/testing "a node dropped with no neighbour in range does not merge" (t/is (empty? (emit-of (mk {:nodes #{3} :segments #{} :handlers #{}}))))))) +(t/deftest nodes-dropped-on-the-same-position-are-merged + ;; An exact drop leaves both commands at one position, with no node near it. + (let [id (random-uuid) + content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :line-to :params {:x 10 :y 10}} + {:command :move-to :params {:x 20 :y 0}} + {:command :line-to :params {:x 10 :y 10}}]) + state (pth/selectable-path-state + id content {:nodes #{3} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/merge-coincident-nodes) state) + (path.state/get-path :content))] + (t/is (= [:move-to :line-to :line-to] (mapv :command (vec result)))) + ;; the two ends are one node, so separating them cannot restore them + (t/is (= 1 (count (path/point-indices result (gpt/point 10.0 10.0))))))) + +(t/deftest a-node-dropped-inside-a-closed-subpath-leaves-one-node + ;; The rest of the loop retraces the two visible lines backwards, so only + ;; those lines survive and the node they meet at exists once. + (let [id (random-uuid) + content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :line-to :params {:x 10 :y 5}} + {:command :line-to :params {:x 20 :y 10}} + {:command :curve-to :params {:c1x 20 :c1y 10 + :c2x 10 :c2y 5 + :x 10 :y 5}} + {:command :close-path :params {}}]) + state (pth/selectable-path-state + id content {:nodes #{3} :segments #{} :handlers #{}}) + state' (ptk/update (path.tools/merge-coincident-nodes) state) + result (path.state/get-path state' :content)] + (t/is (= [:move-to :line-to :line-to] (mapv :command (vec result)))) + (t/is (= 1 (count (path/point-indices result (gpt/point 10.0 5.0))))) + ;; the surviving node stays selected + (t/is (= #{1} (get-in state' [:workspace-local :edit-path id :selection :nodes]))))) + +(defn- three-node-line [] + (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :line-to :params {:x 10 :y 5}} + {:command :line-to :params {:x 20 :y 10}}])) + +(t/deftest splitting-a-node-in-draw-mode-yields-one-end-per-line + ;; Two lines meet at the node, so it separates into two ends. + (let [id (random-uuid) + state (->> (pth/selectable-path-state + id (three-node-line) + {:nodes #{1} :segments #{} :handlers #{}}) + (ptk/update (path.drawing/change-edit-mode :draw))) + state' (ptk/update (path.tools/separate-nodes) state) + result (vec (path.state/get-path state' :content))] + (t/is (= [:move-to :line-to :move-to :line-to] (mapv :command result))) + ;; one end stays on the node and the other is offset away from it + (t/is (= (gpt/point 10 5) (path.helpers/node-position result 1))) + (t/is (not= (gpt/point 10 5) (path.helpers/node-position result 2))))) + +(t/deftest adding-a-node-after-a-pending-start-opens-the-subpath + ;; The start reaches the content together with the segment it draws. + (let [id (random-uuid) + state (->> (pth/selectable-path-state + id (three-node-line) + {:nodes #{1} :segments #{} :handlers #{}}) + (ptk/update (path.drawing/change-edit-mode :draw))) + state' (ptk/update (path.drawing/add-node {:x 30 :y 30}) state) + result (vec (path.state/get-path state' :content))] + (t/is (= [:move-to :line-to :line-to :move-to :line-to] (mapv :command result))) + (t/is (= [{:x 0 :y 0} {:x 10 :y 5} {:x 20 :y 10} {:x 10 :y 5} {:x 30 :y 30}] + (mapv #(select-keys (:params %) [:x :y]) result))) + (t/is (nil? (get-in state' [:workspace-local :edit-path id :pending-start]))))) + +(t/deftest aligning-nodes-onto-each-other-merges-them + (let [id (random-uuid) + content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :line-to :params {:x 20 :y 0}} + {:command :move-to :params {:x 0 :y 10}} + {:command :line-to :params {:x 20 :y 10}}]) + state (pth/selectable-path-state + id content {:nodes #{1 3} :segments #{} :handlers #{}}) + state' (ptk/update (path.tools/align-nodes :vcenter) state) + result (path.state/get-path state' :content)] + ;; both ends meet at (20,5) and become a single node + (t/is (= [:move-to :line-to :line-to] (mapv :command (vec result)))) + (t/is (= 1 (count (path/point-indices result (gpt/point 20.0 5.0))))) + ;; and that node stays selected + (t/is (= #{1} (get-in state' [:workspace-local :edit-path id :selection :nodes]))) + (t/is (= (gpt/point 20.0 5.0) + (path.helpers/node-position result 1))))) + ;; Path-local undo and redo events use a seeded local stack. ;; --- Handler type changes pick which handler keeps its geometry