From 3c821f1b1b32d3e5c99317ceee5e578e79d3cb10 Mon Sep 17 00:00:00 2001 From: Shreyash Agare Date: Tue, 22 Sep 2026 14:51:47 +0530 Subject: [PATCH] :bug: Restore the missing handler when aligning a one-handler node (#11789) * :bug: Restore the missing handler when aligning a one-handler node A curve node with one of its handlers removed could not be switched to aligned or equal: the collapsed handler stayed on the node, or the line on the other side stayed a line, so nothing happened. Switching to equal could even collapse the remaining handler. Such a node now gets a mirrored handler on the other side, turning that line into a curve when needed, also across the start node of a closed path. Removing a handler also resets the node to independent. Closes #11703 AI-assisted-by: claude-opus-5 Signed-off-by: Shreyash Agare <264953665+ShreyashAgare26@users.noreply.github.com> * :bug: Add some quality improvements --------- Signed-off-by: Shreyash Agare <264953665+ShreyashAgare26@users.noreply.github.com> Co-authored-by: Shreyash Agare <264953665+ShreyashAgare26@users.noreply.github.com> Co-authored-by: alonso.torres --- .../app/main/data/workspace/path/helpers.cljs | 144 ++++++++++++++++-- .../app/main/data/workspace/path/tools.cljs | 18 ++- .../frontend_tests/logic/path_tools_test.cljs | 137 +++++++++++++++++ 3 files changed, 278 insertions(+), 21 deletions(-) diff --git a/frontend/src/app/main/data/workspace/path/helpers.cljs b/frontend/src/app/main/data/workspace/path/helpers.cljs index 2ca0a9b760..418560b790 100644 --- a/frontend/src/app/main/data/workspace/path/helpers.cljs +++ b/frontend/src/app/main/data/workspace/path/helpers.cljs @@ -35,13 +35,18 @@ (gpt/to-vec common p1) (gpt/to-vec common p2)))) +(defn mirror-point + "Reflects `point` through `center`." + [center point] + (gpt/subtract (gpt/scale center 2) point)) + (defn opposite-handler-target "Returns the opposite handler target for mirror or aligned modes." [node handler opposite mode] (if (and (some? node) (some? handler) (some? opposite)) (case mode :mirror - (gpt/subtract (gpt/scale node 2) handler) + (mirror-point node handler) :aligned (let [handler-vector (gpt/to-vec node handler)] @@ -170,15 +175,41 @@ [index prefix] (if (= prefix :c1) (dec index) index)) -(defn node-primary-handler - "Returns a curve handler for a node, preferring its incoming handle." - [content node-index] +(defn- subpath-bounds + "Returns the move-to index and last drawing index of `index`'s subpath." + [content index] + (when-let [start (->> (range index -1 -1) + (filter #(= :move-to (:command (nth content % nil)))) + (first))] + [start (->> (range (inc start) (count content)) + (take-while #(not= :move-to (:command (nth content % nil)))) + (remove #(= :close-path (:command (nth content % nil)))) + (last))])) + +(defn- seam-twin-index + "Returns the other command standing on a closed subpath's start node. + + Such a subpath begins and ends at that node, so the selection holds + either its move-to or its last drawing command." + [content index] + (let [[start end] (subpath-bounds content index)] + (when (and (some? end) + (= (path.helpers/segment->point (nth content start nil)) + (path.helpers/segment->point (nth content end nil)))) + (condp = index + start end + end start + nil)))) + +(defn- command-primary-handler + "Returns the curve handler next to `index`, its incoming handle first." + [content index] (let [n (count content) - out-idx (inc node-index)] + out-idx (inc index)] (cond - (and (>= node-index 0) (< node-index n) - (= :curve-to (:command (nth content node-index nil)))) - [node-index :c2] + (and (>= index 0) (< index n) + (= :curve-to (:command (nth content index nil)))) + [index :c2] (and (< out-idx n) (= :curve-to (:command (nth content out-idx nil)))) @@ -186,6 +217,16 @@ :else nil))) +(defn node-primary-handler + "Returns a curve handler for a node, preferring its incoming handle. + + A closed subpath starts and ends at one node, so either of the two + commands standing there finds the handlers of both." + [content node-index] + (or (command-primary-handler content node-index) + (some->> (seam-twin-index content node-index) + (command-primary-handler content)))) + (defn node-handler-ids "Returns a node's curve handlers, its primary handle first." [content node-index] @@ -238,6 +279,67 @@ :else :aligned) :independent)) +(defn- line-beside-node + "Returns the index of the line opposite a node's live handler, looking + across the seam of a closed subpath, or nil." + [plain node-index [_ prefix] node] + (let [line? #(= :line-to (:command (nth plain % nil))) + near-idx (if (= prefix :c2) (inc node-index) node-index)] + (if (line? near-idx) + near-idx + (let [[start end] (subpath-bounds plain node-index) + seam-idx (when (some? end) + (if (= prefix :c2) (inc start) end))] + (when (and (some? seam-idx) + (= node (path.helpers/segment->point (get plain start))) + (= node (path.helpers/segment->point (get plain end))) + (line? seam-idx)) + seam-idx))))) + +(defn- curve-line + "Turns the line at `line-idx` into a curve with `target` as its handler at + `node`. The far handler stays on its node." + [plain line-idx node target] + (let [segment (get plain line-idx) + end (path.helpers/segment->point segment)] + (if (= node end) + (let [start (path.helpers/segment->point (get plain (dec line-idx)))] + (update plain line-idx path.helpers/update-curve-to start target)) + (update plain line-idx path.helpers/update-curve-to target end)))) + +(defn add-missing-handler + "Gives a node with a single handler a mirrored opposite. + + A collapsed handler is moved out; a line on the node's other side becomes + a curve. Other nodes are returned unchanged." + [content node-index] + (let [collapsed? (fn [[idx prefix]] + (= (path/get-handler-point content idx prefix) + (path/handler->node content idx prefix))) + handler-ids (node-handler-ids content node-index) + [live & more] (remove collapsed? handler-ids)] + (if (or (nil? live) (some? more)) + content + (let [[idx prefix] live + node (path/handler->node content idx prefix) + handler (path/get-handler-point content idx prefix) + target (mirror-point node handler) + plain (vec content) + [op-idx op-prefix] (first (filter collapsed? handler-ids)) + line-idx (when (nil? op-idx) + (line-beside-node plain node-index live node))] + (cond + (some? op-idx) + (let [[cx cy] (path.helpers/prefix->coords op-prefix)] + (path/content (update-in plain [op-idx :params] assoc + cx (:x target) cy (:y target)))) + + (some? line-idx) + (path/content (curve-line plain line-idx node target)) + + :else + content))))) + (defn remap-handler-types "Remaps handler types by node position after structural changes." [handler-types old-content new-content] @@ -293,17 +395,27 @@ [content index] (path.helpers/segment->point (nth content index))) +(defn- command-curve-node? + "True when a handler next to `index` stands away from its node." + [content index] + (let [node (node-position content index) + incoming (when (= :curve-to (:command (nth content index nil))) + (path/get-handler-point content index :c2)) + outgoing-index (inc index) + outgoing (when (= :curve-to (:command (nth content outgoing-index nil))) + (path/get-handler-point content outgoing-index :c1))] + (boolean (some #(and (some? %) (not= node %)) [incoming outgoing])))) + (defn curve-node? - "True when the node at `index` has a visible curve handler." + "True when the node at `index` has a visible curve handler. + + A closed subpath starts and ends at one node, so either of the two + commands standing there sees the handlers of both." [content index] (when (node? content index) - (let [node (node-position content index) - incoming (when (= :curve-to (:command (nth content index nil))) - (path/get-handler-point content index :c2)) - outgoing-index (inc index) - outgoing (when (= :curve-to (:command (nth content outgoing-index nil))) - (path/get-handler-point content outgoing-index :c1))] - (boolean (some #(and (some? %) (not= node %)) [incoming outgoing]))))) + (or (command-curve-node? content index) + (boolean (some->> (seam-twin-index content index) + (command-curve-node? content)))))) (defn node-positions "Set of positions for the given node indices in the content." diff --git a/frontend/src/app/main/data/workspace/path/tools.cljs b/frontend/src/app/main/data/workspace/path/tools.cljs index 6b5ef63197..1ab572d7bf 100644 --- a/frontend/src/app/main/data/workspace/path/tools.cljs +++ b/frontend/src/app/main/data/workspace/path/tools.cljs @@ -93,7 +93,9 @@ {})) (defn set-handler-type - "Sets and stores the handler behavior of selected nodes." + "Sets and stores the handler behavior of selected nodes. + + Mirror and aligned give a single-handler node its opposite back." [type] (ptk/reify ::set-handler-type ptk/UpdateEvent @@ -104,7 +106,10 @@ edited (dm/get-in state [:workspace-local :edit-path id :edited-handler]) nodes (helpers/handler-target-nodes content selection)] (if (and (some? content) (seq nodes)) - (let [modifiers (reduce (fn [acc node-index] + (let [content (if (contains? #{:mirror :aligned} type) + (reduce helpers/add-missing-handler content nodes) + content) + modifiers (reduce (fn [acc node-index] (let [reference (helpers/handler-type-reference content selection edited node-index)] (d/deep-merge acc (apply-handler-type-modifiers content reference type)))) @@ -225,14 +230,17 @@ state))))) (defn remove-handler - "Collapses one handler onto its node." + "Collapses one handler onto its node, which becomes independent." [index prefix] (ptk/reify ::remove-handler ptk/UpdateEvent (update [_ state] - (let [content (st/get-path state :content)] + (let [id (st/get-path-id state) + content (st/get-path state :content)] (if (some? content) - (update-path-content state (path/collapse-handler content index prefix)) + (-> (update-path-content state (path/collapse-handler content index prefix)) + (update-in [:workspace-local :edit-path id :handler-types] + dissoc (helpers/handler-node-index index prefix))) state))))) (defn merge-nodes [] diff --git a/frontend/test/frontend_tests/logic/path_tools_test.cljs b/frontend/test/frontend_tests/logic/path_tools_test.cljs index f14888caac..fc5aa33785 100644 --- a/frontend/test/frontend_tests/logic/path_tools_test.cljs +++ b/frontend/test/frontend_tests/logic/path_tools_test.cljs @@ -963,6 +963,143 @@ (t/is (= (gpt/point 16 0) (path/get-handler-point result 2 :c1))) (t/is (= (gpt/point 7 0) (path/get-handler-point result 1 :c2))))) +(t/deftest removing-a-handler-makes-the-node-independent + (let [id (random-uuid) + content (pth/selectable-path-content) + state (-> (pth/selectable-path-state + id content path.helpers/empty-selection) + (assoc-in [:workspace-local :edit-path id :handler-types] {1 :mirror})) + state' (ptk/update (path.tools/remove-handler 1 :c2) state) + types (get-in state' [:workspace-local :edit-path id :handler-types])] + (t/is (= :independent + (:active-type (path.helpers/handler-selection-state + (path.state/get-path state' :content) types #{1})))))) + +(t/deftest aligning-a-node-restores-its-removed-handler + (doseq [type [:aligned :mirror]] + (let [id (random-uuid) + state (pth/selectable-path-state + id (pth/selectable-path-content) + {:nodes #{1} :segments #{} :handlers #{}}) + state (ptk/update (path.tools/remove-handler 1 :c2) state) + result (-> (ptk/update (path.tools/set-handler-type type) state) + (path.state/get-path :content))] + ;; The removed handler comes back mirroring the one that stayed + (t/is (= (gpt/point 8 0) (path/get-handler-point result 1 :c2)) (str type)) + (t/is (= (gpt/point 12 0) (path/get-handler-point result 2 :c1)) (str type))))) + +(t/deftest aligning-a-node-curves-the-line-on-its-other-side + (let [id (random-uuid) + state (pth/selectable-path-state + id (pth/mixed-corner-curve-content) + {:nodes #{1} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/set-handler-type :aligned) state) + (path.state/get-path :content))] + ;; The incoming line becomes a curve mirroring the outgoing handler + (t/is (= :curve-to (:command (nth result 1)))) + (t/is (= (gpt/point 0 0) (path/get-handler-point result 1 :c1))) + (t/is (= (gpt/point 8 -4) (path/get-handler-point result 1 :c2))) + (t/is (= (gpt/point 12 4) (path/get-handler-point result 2 :c1))) + (t/is (= :mirror (path.helpers/derive-handler-type result 1))))) + +(t/deftest making-handlers-equal-curves-the-outgoing-line + (let [id (random-uuid) + content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :curve-to + :params {:c1x 2 :c1y 4 :c2x 8 :c2y 4 :x 10 :y 0}} + {:command :line-to :params {:x 20 :y 0}}]) + state (pth/selectable-path-state + id content {:nodes #{1} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/set-handler-type :mirror) state) + (path.state/get-path :content)) + segment (nth result 2)] + (t/is (= :curve-to (:command segment))) + (t/is (= (gpt/point 12 -4) (path/get-handler-point result 2 :c1))) + (t/is (= (gpt/point 20 0) (path/get-handler-point result 2 :c2))) + (t/is (= [20 0] [(get-in segment [:params :x]) (get-in segment [:params :y])])))) + +(t/deftest making-a-node-independent-adds-no-handler + (let [id (random-uuid) + content (pth/mixed-corner-curve-content) + state (pth/selectable-path-state + id content {:nodes #{1} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/set-handler-type :independent) state) + (path.state/get-path :content))] + (t/is (= (vec content) (vec result))))) + +(t/deftest making-an-end-node-with-one-handler-equal-changes-nothing + (let [content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :curve-to + :params {:c1x 2 :c1y 4 :c2x 8 :c2y 4 :x 10 :y 0}}])] + (doseq [node [0 1]] + (let [id (random-uuid) + state (pth/selectable-path-state + id content {:nodes #{node} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/set-handler-type :mirror) state) + (path.state/get-path :content))] + (t/is (= (vec content) (vec result)) (str "node " node)))))) + +(t/deftest making-two-nodes-equal-curves-the-line-between-them + (let [id (random-uuid) + content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :curve-to + :params {:c1x 2 :c1y 4 :c2x 8 :c2y 4 :x 10 :y 0}} + {:command :line-to :params {:x 20 :y 0}} + {:command :curve-to + :params {:c1x 22 :c1y 4 :c2x 28 :c2y 4 :x 30 :y 0}}]) + state (pth/selectable-path-state + id content {:nodes #{1 2} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/set-handler-type :mirror) state) + (path.state/get-path :content))] + ;; The shared line becomes one curve with a mirrored handler at each end + (t/is (= :curve-to (:command (nth result 2)))) + (t/is (= (gpt/point 12 -4) (path/get-handler-point result 2 :c1))) + (t/is (= (gpt/point 18 -4) (path/get-handler-point result 2 :c2))) + (t/is (= :mirror (path.helpers/derive-handler-type result 1))) + (t/is (= :mirror (path.helpers/derive-handler-type result 2))))) + +(t/deftest aligning-the-seam-node-curves-the-closing-line + (let [id (random-uuid) + content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :curve-to + :params {:c1x 2 :c1y -3 :c2x 8 :c2y -3 :x 10 :y 0}} + {:command :line-to :params {:x 5 :y 10}} + {:command :line-to :params {:x 0 :y 0}} + {:command :close-path :params {}}]) + state (pth/selectable-path-state + id content {:nodes #{0} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/set-handler-type :aligned) state) + (path.state/get-path :content))] + (t/is (= :curve-to (:command (nth result 3)))) + (t/is (= (gpt/point 5 10) (path/get-handler-point result 3 :c1))) + (t/is (= (gpt/point -2 3) (path/get-handler-point result 3 :c2))) + (t/is (= :line-to (:command (nth result 2)))))) + +(t/deftest both-commands-of-a-seam-node-align-the-closing-line + (let [content (path/content + [{:command :move-to :params {:x 0 :y 0}} + {:command :curve-to + :params {:c1x 2 :c1y -3 :c2x 8 :c2y -3 :x 10 :y 0}} + {:command :line-to :params {:x 5 :y 10}} + {:command :line-to :params {:x 0 :y 0}} + {:command :close-path :params {}}])] + ;; The start point is both the move-to and the closing line, and the + ;; selection may hold either one. + (doseq [node [0 3]] + (let [id (random-uuid) + state (pth/selectable-path-state + id content {:nodes #{node} :segments #{} :handlers #{}}) + result (-> (ptk/update (path.tools/set-handler-type :aligned) state) + (path.state/get-path :content))] + (t/is (path.helpers/curve-node? content node) (str "node " node)) + (t/is (= :curve-to (:command (nth result 3))) (str "node " node)) + (t/is (= (gpt/point -2 3) (path/get-handler-point result 3 :c2)) + (str "node " node)))))) + (t/deftest dragging-a-handler-records-it-as-the-last-edited-one (let [id (random-uuid) content (aligned-uneven-handlers-content)