a number of undo/redo fixes for ink (snapping to tangent, add points, dragging tangents). also tried to make storage of undo events more efficient when dragging ink controls (avoid saving hundreds of copies of the InkField)

7 files changed, 72 insertions, 43 deletions

diff --git a/src/client/util/UndoManager.ts b/src/client/util/UndoManager.ts
index 05fb9f378..536d90371 100644
--- a/src/client/util/UndoManager.ts
+++ b/src/client/util/UndoManager.ts
@@ -70,6 +70,7 @@ export namespace UndoManager {
     export interface UndoEvent {
         undo: () => void;
         redo: () => void;
+        prop: string;
     }
     type UndoBatch = UndoEvent[];
 
@@ -104,6 +105,23 @@ export namespace UndoManager {
     export function GetOpenBatches(): Without<Batch, 'end'>[] {
         return openBatches;
     }
+    export function FilterBatches(fieldTypes: string[]) {
+        var fieldCounts: { [key: string]: number } = {};
+        const lastStack = UndoManager.undoStack.lastElement();
+        if (lastStack) {
+            lastStack.forEach(ev => fieldTypes.includes(ev.prop) && (fieldCounts[ev.prop] = (fieldCounts[ev.prop] || 0) + 1));
+            var fieldCount2: { [key: string]: number } = {};
+            runInAction(() =>
+                UndoManager.undoStack[UndoManager.undoStack.length - 1] = lastStack.filter(ev => {
+                    if (fieldTypes.includes(ev.prop)) {
+                        fieldCount2[ev.prop] = (fieldCount2[ev.prop] || 0) + 1;
+                        if (fieldCount2[ev.prop] === 1 || fieldCount2[ev.prop] === fieldCounts[ev.prop]) return true;
+                        return false;
+                    }
+                    return true;
+                }));
+        }
+    }
     export function TraceOpenBatches() {
         console.log(`Open batches:\n\t${openBatches.map(batch => batch.batchName).join("\n\t")}\n`);
     }
@@ -140,6 +158,7 @@ export namespace UndoManager {
         batchCounter--;
         // console.log("End " + batchCounter);
         if (batchCounter === 0 && currentBatch?.length) {
+            // console.log("------ended----")
             if (!cancel) {
                 undoStack.push(currentBatch);
             }
diff --git a/src/client/views/DocumentDecorations.tsx b/src/client/views/DocumentDecorations.tsx
index bd61c9f60..bd9c3509b 100644
--- a/src/client/views/DocumentDecorations.tsx
+++ b/src/client/views/DocumentDecorations.tsx
@@ -202,7 +202,10 @@ export class DocumentDecorations extends React.Component<{ PanelWidth: number, P
                 InkStrokeProperties.Instance?.rotateInk(2 * movement.X / angle * (Math.PI / 180));
                 return false;
             },
-            () => this._rotateUndo?.end(),
+            () => {
+                this._rotateUndo?.end();
+                UndoManager.FilterBatches(["data", "x", "y", "width", "height"]);
+            },
             emptyFunction);
         this._prevY = e.clientY;
         this._inkCenterPts = SelectionManager.Views()
diff --git a/src/client/views/InkControlPtHandles.tsx b/src/client/views/InkControlPtHandles.tsx
index eb0eebcdf..898c3bf26 100644
--- a/src/client/views/InkControlPtHandles.tsx
+++ b/src/client/views/InkControlPtHandles.tsx
@@ -24,7 +24,6 @@ export interface InkControlProps {
 export class InkControlPtHandles extends React.Component<InkControlProps> {
 
     @observable private _overControl = -1;
-    @observable private _overAddPoint = -1;
     /**
      * Handles the movement of a selected control point when the user clicks and drags.
      * @param controlIndex The index of the currently selected control point.
@@ -32,8 +31,7 @@ export class InkControlPtHandles extends React.Component<InkControlProps> {
     @action
     onControlDown = (e: React.PointerEvent, controlIndex: number): void => {
         if (InkStrokeProperties.Instance) {
-            InkStrokeProperties.Instance.moveControl(0, 0, 1);
-            const controlUndo = UndoManager.StartBatch("DocDecs set radius");
+            var controlUndo: UndoManager.Batch | undefined;
             const screenScale = this.props.ScreenToLocalTransform().Scale;
             const order = controlIndex % 4;
             const handleIndexA = ((order === 3 ? controlIndex - 1 : controlIndex - 2) + this.props.inkCtrlPoints.length) % this.props.inkCtrlPoints.length;
@@ -41,17 +39,26 @@ export class InkControlPtHandles extends React.Component<InkControlProps> {
             const brokenIndices = Cast(this.props.inkDoc.brokenInkIndices, listSpec("number"));
             setupMoveUpEvents(this, e,
                 (e: PointerEvent, down: number[], delta: number[]) => {
+                    if (!controlUndo) controlUndo = UndoManager.StartBatch("drag ink ctrl pt");
                     InkStrokeProperties.Instance?.moveControl(-delta[0] * screenScale, -delta[1] * screenScale, controlIndex);
                     return false;
                 },
-                () => controlUndo?.end(),
+                () => {
+                    controlUndo?.end();
+                    UndoManager.FilterBatches(["data", "x", "y", "width", "height"]);
+                },
                 action((e: PointerEvent, doubleTap: boolean | undefined) => {
                     const equivIndex = controlIndex === 0 ? this.props.inkCtrlPoints.length - 1 : controlIndex === this.props.inkCtrlPoints.length - 1 ? 0 : controlIndex;
-                    if (doubleTap && brokenIndices?.includes(equivIndex)) {
-                        InkStrokeProperties.Instance?.snapHandleTangent(equivIndex, handleIndexA, handleIndexB);
-                    }
-                    if (doubleTap && brokenIndices?.includes(controlIndex)) {
-                        InkStrokeProperties.Instance?.snapHandleTangent(controlIndex, handleIndexA, handleIndexB);
+                    if (doubleTap) {
+                        if (brokenIndices?.includes(equivIndex)) {
+                            if (!controlUndo) controlUndo = UndoManager.StartBatch("make smooth");
+                            InkStrokeProperties.Instance?.snapHandleTangent(equivIndex, handleIndexA, handleIndexB);
+                        }
+                        if (equivIndex !== controlIndex && brokenIndices?.includes(controlIndex)) {
+                            if (!controlUndo) controlUndo = UndoManager.StartBatch("make smooth");
+                            InkStrokeProperties.Instance?.snapHandleTangent(controlIndex, handleIndexA, handleIndexB);
+                        }
+                        controlUndo?.end();
                     }
                 }));
         }
@@ -62,8 +69,6 @@ export class InkControlPtHandles extends React.Component<InkControlProps> {
      */
     @action onEnterControl = (i: number) => { this._overControl = i; };
     @action onLeaveControl = () => { this._overControl = -1; };
-    @action onEnterAddPoint = (i: number) => { this._overAddPoint = i; };
-    @action onLeaveAddPoint = () => { this._overAddPoint = -1; };
 
     /**
      * Deletes the currently selected point.
diff --git a/src/client/views/InkStrokeProperties.ts b/src/client/views/InkStrokeProperties.ts
index 6a503cc91..3770eb7c1 100644
--- a/src/client/views/InkStrokeProperties.ts
+++ b/src/client/views/InkStrokeProperties.ts
@@ -214,14 +214,16 @@ export class InkStrokeProperties {
      */
     snapHandleTangent = (controlIndex: number, handleIndexA: number, handleIndexB: number) => {
         this.applyFunction((doc: Doc, ink: InkData) => {
-            const brokenIndices = Cast(doc.brokenInkIndices, listSpec("number"));
-            if (brokenIndices) {
-                doc.brokenInkIndices = new List<number>(brokenIndices.filter(brokenIndex => brokenIndex !== controlIndex));
+            const brokenIndices = Cast(doc.brokenInkIndices, listSpec("number"), []);
+            const ind = brokenIndices.findIndex(value => value === controlIndex);
+            if (ind !== -1) {
+                brokenIndices.splice(ind, 1);
                 const [controlPoint, handleA, handleB] = [ink[controlIndex], ink[handleIndexA], ink[handleIndexB]];
                 const oppositeHandleA = this.rotatePoint(handleA, controlPoint, Math.PI);
                 const angleDifference = this.angleChange(handleB, oppositeHandleA, controlPoint);
-                ink[handleIndexB] = this.rotatePoint(handleB, controlPoint, angleDifference);
-                return ink;
+                const inkCopy = ink.slice();
+                inkCopy[handleIndexB] = this.rotatePoint(handleB, controlPoint, angleDifference);
+                return inkCopy;
             }
         });
     }
@@ -277,13 +279,14 @@ export class InkStrokeProperties {
             const oppositeHandlePoint = ink[oppositeHandleIndex];
             const controlPoint = ink[controlIndex];
             const newHandlePoint = { X: ink[handleIndex].X - deltaX / xScale, Y: ink[handleIndex].Y - deltaY / yScale };
-            ink[handleIndex] = newHandlePoint;
+            const inkCopy = ink.slice();
+            inkCopy[handleIndex] = newHandlePoint;
             const brokenIndices = Cast(doc.brokenInkIndices, listSpec("number"));
             // Rotate opposite handle if user hasn't held 'Alt' key or not first/final control (which have only 1 handle).
             if ((!brokenIndices || !brokenIndices?.includes(controlIndex)) && (closed || (handleIndex !== 1 && handleIndex !== ink.length - 2))) {
                 const angle = this.angleChange(oldHandlePoint, newHandlePoint, controlPoint);
-                ink[oppositeHandleIndex] = this.rotatePoint(oppositeHandlePoint, controlPoint, angle);
+                inkCopy[oppositeHandleIndex] = this.rotatePoint(oppositeHandlePoint, controlPoint, angle);
             }
-            return ink;
+            return inkCopy;
         })
 }
\ No newline at end of file
diff --git a/src/client/views/InkTangentHandles.tsx b/src/client/views/InkTangentHandles.tsx
index dbe9ca027..759e48134 100644
--- a/src/client/views/InkTangentHandles.tsx
+++ b/src/client/views/InkTangentHandles.tsx
@@ -27,18 +27,21 @@ export class InkTangentHandles extends React.Component<InkHandlesProps> {
      */
     onHandleDown = (e: React.PointerEvent, handleIndex: number): void => {
         if (InkStrokeProperties.Instance) {
-            InkStrokeProperties.Instance.moveControl(0, 0, 1);
-            const controlUndo = UndoManager.StartBatch("DocDecs set radius");
+            var controlUndo: UndoManager.Batch | undefined;
             const screenScale = this.props.ScreenToLocalTransform().Scale;
             const order = handleIndex % 4;
             const oppositeHandleRawIndex = order === 1 ? handleIndex - 3 : handleIndex + 3;
             const oppositeHandleIndex = (oppositeHandleRawIndex < 0 ? this.props.screenCtrlPoints.length + oppositeHandleRawIndex : oppositeHandleRawIndex) % this.props.screenCtrlPoints.length;
             const controlIndex = (order === 1 ? handleIndex - 1 : handleIndex + 2) % this.props.screenCtrlPoints.length;
             setupMoveUpEvents(this, e, (e: PointerEvent, down: number[], delta: number[]) => {
+                if (!controlUndo) controlUndo = UndoManager.StartBatch("DocDecs move tangent");
                 if (e.altKey) this.onBreakTangent(controlIndex);
                 InkStrokeProperties.Instance?.moveHandle(-delta[0] * screenScale, -delta[1] * screenScale, handleIndex, oppositeHandleIndex, controlIndex);
                 return false;
-            }, () => controlUndo?.end(), emptyFunction
+            }, () => {
+                controlUndo?.end();
+                UndoManager.FilterBatches(["data", "x", "y", "width", "height"]);
+            }, emptyFunction
             );
         }
     }
@@ -50,15 +53,12 @@ export class InkTangentHandles extends React.Component<InkHandlesProps> {
      */
     @action
     onBreakTangent = (controlIndex: number) => {
-        const doc = this.props.inkDoc;
-        if (doc) {
-            const closed = this.props.screenCtrlPoints.lastElement().X === this.props.screenCtrlPoints[0].X && this.props.screenCtrlPoints.lastElement().Y === this.props.screenCtrlPoints[0].Y;
-            const brokenIndices = Cast(doc.brokenInkIndices, listSpec("number")) || new List;
-            if (!brokenIndices?.includes(controlIndex) &&
-                ((controlIndex > 0 && controlIndex < this.props.screenCtrlPoints.length - 1) || closed)) {
-                brokenIndices.push(controlIndex);
-                doc.brokenInkIndices = brokenIndices;
-            }
+        const closed = this.props.screenCtrlPoints.lastElement().X === this.props.screenCtrlPoints[0].X && this.props.screenCtrlPoints.lastElement().Y === this.props.screenCtrlPoints[0].Y;
+        var brokenIndices = Cast(this.props.inkDoc.brokenInkIndices, listSpec("number"));
+        if (!brokenIndices?.includes(controlIndex) &&
+            ((controlIndex > 0 && controlIndex < this.props.screenCtrlPoints.length - 1) || closed)) {
+            if (brokenIndices) brokenIndices.push(controlIndex);
+            else this.props.inkDoc.brokenInkIndices = new List<number>([controlIndex]);
         }
     }
 
diff --git a/src/client/views/InkingStroke.tsx b/src/client/views/InkingStroke.tsx
index b921014a3..867677005 100644
--- a/src/client/views/InkingStroke.tsx
+++ b/src/client/views/InkingStroke.tsx
@@ -77,8 +77,8 @@ export class InkingStroke extends ViewBoxBaseComponent<FieldViewProps, InkDocume
                         this._properties._controlButton = true;
                         InkStrokeProperties.Instance && (InkStrokeProperties.Instance._currentPoint = -1);
                         this._handledClick = true;  // mark the double-click pseudo pointerevent so we can block the real mouse event from propagating to DocumentView
-                    } else {
-                        if (this._properties?._controlButton) this._nearestT && this._nearestSeg !== undefined && InkStrokeProperties.Instance?.addPoints(this._nearestT, this._nearestSeg, this.inkScaledData().inkData);
+                    } else if (this._properties?._controlButton) {
+                        this._nearestT && this._nearestSeg !== undefined && InkStrokeProperties.Instance?.addPoints(this._nearestT, this._nearestSeg, this.inkScaledData().inkData.slice());
                     }
                 }), this._properties?._controlButton, this._properties?._controlButton
             );
@@ -125,7 +125,7 @@ export class InkingStroke extends ViewBoxBaseComponent<FieldViewProps, InkDocume
     }
 
     @action
-    onPointerOver = (e: React.PointerEvent) => {
+    onPointerMove = (e: React.PointerEvent) => {
         const { inkData, inkScaleX, inkScaleY, inkStrokeWidth, inkTop, inkLeft } = this.inkScaledData();
         const screenPts = inkData.map(point => this.props.ScreenToLocalTransform().inverse().transformPoint(
             (point.X - inkLeft - inkStrokeWidth / 2) * inkScaleX + inkStrokeWidth / 2,
@@ -212,7 +212,7 @@ export class InkingStroke extends ViewBoxBaseComponent<FieldViewProps, InkDocume
                     overflow: "visible",
                 }}
                 onPointerLeave={action(e => this._nearestScrPt = undefined)}
-                onPointerMove={this.props.isSelected() ? this.onPointerOver : undefined}
+                onPointerMove={this.props.isSelected() ? this.onPointerMove : undefined}
                 onPointerDown={this.onPointerDown}
                 onClick={e => this._handledClick && e.stopPropagation()}
                 onContextMenu={() => {
diff --git a/src/fields/util.ts b/src/fields/util.ts
index 3590c2dea..99dfc04d9 100644
--- a/src/fields/util.ts
+++ b/src/fields/util.ts
@@ -98,13 +98,12 @@ const _setterImpl = action(function (target: any, prop: string | symbol | number
         } else {
             DocServer.registerDocWithCachedUpdate(receiver, prop as string, curValue);
         }
-        !receiver[Initializing] && (!receiver[UpdatingFromServer] || receiver[ForceServerWrite]) && UndoManager.AddEvent({
-            redo: () => receiver[prop] = value,
-            undo: () => {
-                // console.log("Undo: " + prop + " = " + curValue); // bcz: uncomment to log undo
-                receiver[prop] = curValue;
-            }
-        });
+        !receiver[Initializing] && (!receiver[UpdatingFromServer] || receiver[ForceServerWrite]) &&
+            UndoManager.AddEvent({
+                redo: () => receiver[prop] = value,
+                undo: () => receiver[prop] = curValue,
+                prop: prop?.toString()
+            });
         return true;
     }
     return false;