Undo implementation

[nidoro]

Hello, I'm getting unexpected results from my undo implementation. This is how I'm doing it:

1. Initialize (global) empty array of strokes: let strokes = []

2. When strokerecorded is fired, push stroke to array:

atrament.addEventListener('strokerecorded', function(obj) {
    if (!atrament.recordPaused) {
        strokes.push(obj.stroke);
    }
});

3. When the undo button is pressed, do the following:

function undoStroke() {
    strokes.pop();
    atrament.clear();
    atrament.recordPaused = true;
    for (let stroke of strokes) {
        // set drawing options
        atrament.mode = stroke.mode;
        atrament.weight = stroke.weight;
        atrament.smoothing = stroke.smoothing;
        atrament.color = stroke.color;
        atrament.adaptiveStroke = stroke.adaptiveStroke;

        // don't want to modify original data
        const points = stroke.points.slice();

        const firstPoint = points.shift();
        // beginStroke moves the "pen" to the given position and starts the path
        atrament.beginStroke(firstPoint.x, firstPoint.y);

        let prevPoint = firstPoint;
        while (points.length > 0) {
            const point = points.shift();

            // the `draw` method accepts the current real coordinates
            // (i. e. actual cursor position), and the previous processed (filtered)
            // position. It returns an object with the current processed position.
            const { x, y } = atrament.draw(point.x, point.y, prevPoint.x, prevPoint.y);

            // the processed position is the one where the line is actually drawn to
            // so we have to store it and pass it to `draw` in the next step
            prevPoint = { x, y };
        }

        // endStroke closes the path
        atrament.endStroke(prevPoint.x, prevPoint.y);
    }
    atrament.recordPaused = false;
}

But this is what I get:

Notice how after I press undo, the last line disappears, as expected, but the other two lines are extended a little bit.

Can you help me with this? I'm on Linux. This happens on both Firefox 72.0.1 and Google Chrome 89.0.4389.90.

Thank you for the library!

[Gnarly-Charley]

Hi @nidoro

I ran into the same problem. In my case it seemed to be solved after modifying the while loop from "while (points.length > 0)" to "while (points.length > 1)". I haven't quite figured out why this works yet, but for now this is good enough for me.

[jakubfiala]

@nidoro thank you for submitting the issue! Apologies for only getting to this now, I haven't been able to dedicate time to this lib for a while. I'll test your use case, try to reproduce the bug and fix it in the next few weeks.

[feored]

Thanks for this example @nidoro . I modified it slightly to add support for fills, and fixed a few things that I assume have changed since your post (Point.point.x instead of point.x, ...). Not battle tested or anything but might save someone else a bit of trouble.

        this.sketchpad.addEventListener('strokerecorded', (obj) => {
            if (!this.sketchpad.recordPaused) {
                obj.stroke.type = "stroke";
                this.strokes.push(obj.stroke);
            }
        });

        this.sketchpad.addEventListener('fillstart', ({ x, y }) => {
            var obj = {};
            obj.type = "fill";
            obj.fillColor = this.sketchpad.color;
            obj.x = x;
            obj.y = y;
            this.strokes.push(obj);
        });

undo(){
            this.strokes.pop();
            this.sketchpad.clear();
            this.sketchpad.recordPaused = true;
            for (let stroke of this.strokes) {
                if (stroke.type == "stroke"){
                    // set drawing options
                    this.sketchpad.mode = stroke.mode;
                    this.sketchpad.weight = stroke.weight;
                    this.sketchpad.smoothing = stroke.smoothing;
                    this.sketchpad.color = stroke.color;
                    this.sketchpad.adaptiveStroke = stroke.adaptiveStroke;

                    // don't want to modify original data
                    const points = stroke.points.slice();

                    const firstPoint = points.shift().point;
                    // beginStroke moves the "pen" to the given position and starts the path
                    this.sketchpad.beginStroke(firstPoint.x, firstPoint.y);

                    let prevPoint = firstPoint;
                    while (points.length > 0) {
                        const point = points.shift();

                        // the `draw` method accepts the current real coordinates
                        // (i. e. actual cursor position), and the previous processed (filtered)
                        // position. It returns an object with the current processed position.
                        const { x, y } = this.sketchpad.draw(point.point.x, point.point.y, prevPoint.x, prevPoint.y);

                        // the processed position is the one where the line is actually drawn to
                        // so we have to store it and pass it to `draw` in the next step
                        prevPoint = { x, y };
                    }

                    // endStroke closes the path
                    this.sketchpad.endStroke(prevPoint.x, prevPoint.y);
                } else {
                    this.sketchpad.color = stroke.fillColor;
                    const startColor = Array.from(this.sketchpad.context.getImageData(stroke.x, stroke.y, 1, 1).data);
                    this.sketchpad._floodFill(stroke.x, stroke.y, startColor);
                }

            }
            this.sketchpad.recordPaused = false;
        }

[jakubfiala]

@nidoro I may be nearly 3 years late, but I believe this bug has now been fixed via db7242e2b580ecee5f545e457a9c20a63c274ae9 :) it looks like the stroke segments recorded when beginStroke and endStroke are called were redundant. Specifically the segment recorded during endStroke caused the re-drawn stroke to be slightly longer than the original.

I am now working on a big update which will be released as v4 in the next few weeks. This fix will be part of it.

Thank you again for the bug report and apologies for being very, very late!

[nidoro]

Hi @jakubfiala, no need for apologies, that's the nature of open source. I'm currently not using atrament in any of my projects, but it's good to know that, if I ever need it, this will be fixed. Thank a lot!