🐛 Restore the missing handler when aligning a one-handler node (#11789)

* 🐛 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>

* 🐛 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 <alonso.torres@kaleidos.net>

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."

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 []

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)