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

Correct reset the rendering context #6660

Merged
merged 2 commits into from
May 11, 2021
Merged

Correct reset the rendering context #6660

merged 2 commits into from
May 11, 2021

Conversation

annevk
Copy link
Member

@annevk annevk commented 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.


/canvas.html ( diff )

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.
@annevk
Copy link
Member Author

annevk commented May 6, 2021

cc @Kaiido @whatwg/canvas

@annevk annevk requested a review from domenic May 6, 2021 11:40
@Kaiido
Copy link
Member

Kaiido commented May 6, 2021

Thanks, looks good to me, but I'm not well versed in editorial reviews.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Of course it'd be better if this took explicit arguments and didn't say things like "canvas" and "context", but that can be future work.

source Show resolved Hide resolved
@annevk
Copy link
Member Author

annevk commented May 7, 2021

@mysteryDate FYI (I thought you were already on the canvas team, just added you).

@domenic domenic merged commit 20a0ddc into main May 11, 2021
@domenic domenic deleted the annevk/canvas-reset- branch May 11, 2021 19:48
@mysteryDate
Copy link
Contributor

Thanks for clearing this up @annevk!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants