Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can only undo for one step #98

Closed
oyuntuayC opened this issue Mar 28, 2024 · 4 comments
Closed

Can only undo for one step #98

oyuntuayC opened this issue Mar 28, 2024 · 4 comments
Assignees
Labels
Milestone

Comments

@oyuntuayC
Copy link

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
Copy link
Author

oyuntuayC commented Mar 28, 2024

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

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

    this.#dirty = false;
    this.dispatchEvent('clean');

@oyuntuayC
Copy link
Author

and by getting context and clear it manually it works,

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

@jakubfiala jakubfiala added the bug label Mar 28, 2024
@jakubfiala jakubfiala self-assigned this Mar 28, 2024
@jakubfiala
Copy link
Owner

@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 jakubfiala added this to the 4.1.0 milestone Mar 28, 2024
@jakubfiala
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants