The Wayback Machine - https://web.archive.org/web/20201016121259/https://github.com/whatwg/html/issues/5771
Skip to content

/ html

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

Jump to bottom

"reset the rendering context to its default state" should also clear the "current default path" #5771

Open
Kaiido opened this issue Jul 30, 2020 · 7 comments
Open

"reset the rendering context to its default state" should also clear the "current default path" #5771

Kaiido opened this issue Jul 30, 2020 · 7 comments
Labels
compat good first issue topic: canvas

Comments

@Kaiido
Copy link
Contributor

@Kaiido Kaiido commented Jul 30, 2020

Related to #5618 (comment).

When a canvas element with a 2D context has either its width or height being changed, current texts ask that the "set bitmap dimensions" algorithm is executed, itself calling "reset the rendering context to its default state".

When the user agent is to reset the rendering context to its default state, it must clear the drawing state stack and everything that drawing state consists of to initial values.

In the "drawing state" definition there is a note specifying clearly

The current default path and the rendering context's bitmaps are not part of the drawing state. The current default path is persistent, and can only be reset using the beginPath() method. The bitmaps depend on whether and how the rendering context is bound to a canvas element.

The "context's bitmap" is cleared in the second step of "set bitmap dimensions", but the "current default path" is never explicitly reset in this algorithm.
This means that according to these texts the "current default path" should not be cleared when the canvas dimension is set again.

However all implementations do reset it, I think they always did, and I'm sure a lot of websites are expecting it to happen.

So I believe a new step in this "reset the rendering context to its default state" algorithm should be included to specifically reset the "current default path".
@annevk
Copy link
Member

@annevk annevk commented Aug 4, 2020

cc @whatwg/canvas
@annevk
Copy link
Member

@annevk annevk commented Aug 4, 2020

Having some web-platform-tests to back this up would be ideal.
Kaiido pushed a commit to Kaiido/wpt that referenced this issue Aug 5, 2020
Kaiido
Add tests for the html-canvas set-bitmap-dimensions algorithm
9085a32 
Related to whatwg/html#5771

There is currently a compat issue with the specs.
@Kaiido Kaiido mentioned this issue Aug 5, 2020
Open
@annevk
Copy link
Member

@annevk annevk commented Aug 24, 2020

So assuming the tests are correct is there any reason we shouldn't make the path part of the drawing state?
@Kaiido
Copy link
Contributor Author

@Kaiido Kaiido commented Aug 24, 2020

So assuming the tests are correct is there any reason we shouldn't make the path part of the drawing state?

Yes, save and restore would then also take it, which should probably not happen.
@annevk
Copy link
Member

@annevk annevk commented Aug 24, 2020

Thanks, I had missed that as a lot of usage of "drawing state" isn't linked back to its definition apparently.
@Kaiido
Copy link
Contributor Author

@Kaiido Kaiido commented Sep 22, 2020

I was trying to write a PR for this when I realized it's actually a bit strange to have this fit in the canvas state paragraph when this paragraph only applies to objects inheriting from the CanvasState interface, while the current default path is inherited by the CanvasDrawPath interface.

So probably it's better to duplicate something in both places which actually calls this algorithm: 2D context set bitmap dimensions and OffscreenCanvas width & height setters dfn.

I was thinking something like (bold are changes)

  1. Reset the rendering context to its default state.
  2. Empty the list of subpaths in the context's current default path.
  3. Resize the output bitmap to the new width and height and clear it to transparent black.
  4. [...]

for the 2D context's set bitmap dimensions, and

[...] replace the OffscreenCanvas object's bitmap with a new transparent black bitmap, reset the rendering context to its default state and empty the list of subpaths in the context's current default path.

for OffscreenCanvas.

But in the mean time, I'm wondering if it is correct to have two algorithms that much similar, and if the ordering of these steps technically matters, since they do differ there.
So maybe it would be better to have an other algorithm defined somewhere responsible to call the three steps (clear the state, reset he bitmap to transparent black and empty the list of subpaths), and which could be reused by an hypothetical reset() method in the future.

Thoughts? (particularly from team canvas like @fserb maybe?).
@annevk
Copy link
Member

@annevk annevk commented Sep 22, 2020

Ordering shouldn't matter there I think and I agree that these should share more infrastructure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat good first issue topic: canvas
3 participants
@annevk @Kaiido and others
You can’t perform that action at this time.