jakubfiala / atrament

A small JS library for beautiful drawing and handwriting on the HTML Canvas.
http://fiala.space/atrament/demo
MIT License
1.56k stars 115 forks source link

Can only undo for one step #98

Closed oyuntuayC closed 7 months ago

oyuntuayC commented 7 months ago

I‘m using the same implementation as #71, But I can only undo for one step. I noticed that each time I undo, the previous strokes seems overlapped by new stokes.

It's not very obvious in the video, but the first time I click undo, everything works fine. By the second time I click undo, the first two stoke would became bolder, the third time I do it, only the first stroke became bolder. Nothing seems to change until I draw something manually and click again.

So I assume that clear() only works for one time unless the user draw something again. bandicam2024-03-2801-51-31-441-ezgif com-video-to-gif-converter

oyuntuayC commented 7 months ago

I found this part in the code, maybe it's related

  clear() {
    if (!this.#dirty) {
      return;
    }

    this.#dirty = false;
    this.dispatchEvent('clean');
oyuntuayC commented 7 months ago

and by getting context and clear it manually it works,

const ctx = canvas.getContext("2d")
ctx.clearRect(-10, -10, 1000 + 20, 500 + 20);
jakubfiala commented 7 months ago

@oyuntuayC thank you for reporting the issue! I believe I know what's going on here - the fact that manually doing a clearRect works was a great hint. The clear() method currently returns early if #dirty is false. This was to avoid running all the subsequent code if the canvas is already "clean" (i.e. empty).

However, I now see how this could lead to issues like yours. Atrament currently toggles #dirty only when the pointer has moved. This code doesn't run when drawing programmatically, because no pointermove event fires. I would like to solve this by doing two things:

  1. clear() should always clear the canvas, regardless of the value of #dirty. I think this is more predictable behaviour, especially since applications might draw other non-Atrament stuff onto the canvas, and as a user of the library I'd expect that stuff to be cleared, too.
  2. #dirty should be set to true in the draw() method, rather than in #pointerMove. That way, it will also be true if only programmatic drawing has been performed.

I'll make these changes and release them in v4.1.0, hopefully today or tomorrow.

jakubfiala commented 7 months ago

@oyuntuayC a fix to this has now been published in v4.1.0 - please do let me know if you encounter any other issues!