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

reset function for Canvas2D #5618

Closed
Tracked by #5613
fserb opened this issue Jun 8, 2020 · 11 comments · Fixed by #6660
Closed
Tracked by #5613

reset function for Canvas2D #5618

fserb opened this issue Jun 8, 2020 · 11 comments · Fixed by #6660
Labels
addition/proposal New features or enhancements topic: canvas

Comments

@fserb
Copy link
Contributor

fserb commented Jun 8, 2020

Provide a clear() function that resets the state of the Canvas.

Working proposal: https://github.com/fserb/canvas2D/blob/master/spec/clear.md

(cc @whatwg/canvas )

@fserb fserb mentioned this issue Jun 8, 2020
9 tasks
@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: canvas labels Jun 9, 2020
@othermaciej
Copy link

Why is it that the correct way to clear the canvas should not just clear the backing store but also reset the path, but definitively should not clear style? The linked proposal asserts this without justifying or explaining.

@smfr
Copy link

smfr commented Jun 9, 2020

clear() seems antithetical to shared use of a canvas by multiple JS components. By blowing through the clip and nuking state, clear() would allow some subcomponent to destroy the state set up by an enclosing component (clip, transform etc). I don't think that's a pattern we should encourage.

The draft talks about the "transformation stack" but I think intends to refer to the CanvasState stack.

A good example of why this is bad is:

ctx.push();
...
ctx.clear()
...
ctx.pop(); // now what

@fserb
Copy link
Contributor Author

fserb commented Jun 9, 2020

@othermaciej. That's a fair point, the documents don't explain this well. We want a command to clear the canvas buffer (independent of the current state) and do the minimum number of operations needed to keep that sane and consistent. It seemed that reseting path/clip/transform was a part of this, but not fillStyle/globalAlpha/etc... The path was a borderline case, but we the "draw a path after a clear" would be least surprising like that.

@fserb
Copy link
Contributor Author

fserb commented Jun 9, 2020

@smfr, I don't understand the "shared use of a canvas" argument. Why is your example any different from:

ctx.push();
ctx.canvas.width = ctx.canvas.width;
ctx.pop(); // now what?

Any subcomponent can do this right now. Except it's surrounded in this mysterious "assign to itself" (that, btw, is marked - correctly so - as "bad code" on all JS linters and TypeScript). And is completely magical side-effect of assignment.

We have a situation right now where Context2D can be in a state that it can't clear itself without being destroyed:

ctx.fillRect(0, 0, 1, 1);
ctx.clip(1, 1, 1, 1);
// it's impossible to clear the 0, 0 pixel without destroying the Context 
// through a magical side-effect assignment.

@fserb
Copy link
Contributor Author

fserb commented Jun 9, 2020

I don't particularly care what is the set of operations we keep. From my perspective, we should do the minimum set of operations that makes clearing the whole buffer sane. We could even consider a clear() that only clears the buffer, and/or a reset() that is clear + reset the whole state.

@kdashg
Copy link

kdashg commented Jun 9, 2020 via email

@othermaciej
Copy link

Naively, I'd expect a clear method to do nothing more than a clearRect of the full canvas bounds. That said, having it blast through clip is also potentially unexpected. Maybe the method should be called reset and reset literally all canvas state? But that still leaves to be determined what happens with the state stack.

@fserb
Copy link
Contributor Author

fserb commented Jun 10, 2020

So it seems that we agree on a reset() that resets everything (transformation stack, clip, styles, font, etc). If there are pending transforms in the stack they get discarded as well.

And the arguments against a clear() that clears the full buffer (independent of clip/transform) but that keeps transform stack/clip untouched are:

  • feels weird to ignore clip but keep it
  • feels "too little" (although @jdashg's example is not very clear - as this can't be done right now with regular canvas2D).

Would there be space for a clear() that resets stack, clip, but not anything else? It feels it could be useful, as minimizing setting style and font-related things. It could also be faster. But in practice it may not be worth the trouble.

@smfr
Copy link

smfr commented Jun 10, 2020

If you clear the stack you have to also reset the state tracked by the stack (https://html.spec.whatwg.org/multipage/canvas.html#the-canvas-state).

It would be specified as "unwinding the stack as if restore() was called N times then calling clearRect()" or something.

@othermaciej
Copy link

What about state set before the first call to save()?

@Kaiido
Copy link
Member

Kaiido commented Jul 27, 2020

Regarding the clipping areas, we certainly miss an absolute setClip method à la setTransform.
I remember this argument of "shared components" was already made a long time ago when a proposal to add a method to clear the clipping area was made, but I really don't bite it. In such a case each component should be responsible to set the context in the state it requires it to be, and because of the lack of proper methods to clean after one-self that means we must save and restore the full context state every time, just for the clipping area since it's the only state that we can't remove otherwise.
This makes programming using clip regions so hard that I avoid it as much as possible, and rather rely on compositing when possible, and I know I'm not the only one out there advocating against clip when possible because the API is incomplete.

Now, I can see potential use cases for a reset method, which would directly call reset the rendering context to its default state, avoiding the resize the output bitmap step induced by canvas.width = canvas.width (not sure what it really does to resize it to the same size in current implementations, is a new buffer reassigned?), and more importantly which would greatly improve semantics.
But IMO this method should stay as simple as possible: it resets everything.

@fserb fserb changed the title clear function for Canvas2D reset function for Canvas2D Feb 11, 2021
@domenic domenic removed the needs implementer interest Moving the issue forward requires implementers to express interest label Feb 18, 2021
annevk added a commit that referenced this issue May 6, 2021
6efd0e6 addressed #5771 incorrectly. The current default path should not be part of the drawing state, but should be cleared when resetting the rendering context.

(Also remove some older ideas from the source.)

Closes #5618 and fixes #5771.
domenic pushed a commit that referenced this issue May 11, 2021
6efd0e6 addressed #5771 incorrectly. The current default path should not be part of the drawing state, but should be cleared when resetting the rendering context.

(Also remove some older ideas from the source.)

Closes #5618 and fixes #5771.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: canvas
Development

Successfully merging a pull request may close this issue.

7 participants