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

Small clarity improvements to canvas transform methods #2848

Open
domenic opened this issue Jul 19, 2017 · 7 comments · Fixed by #10174
Open

Small clarity improvements to canvas transform methods #2848

domenic opened this issue Jul 19, 2017 · 7 comments · Fixed by #10174
Labels
clarification Standard could be clearer good first issue Ideal for someone new to a WHATWG standard or software project topic: canvas

Comments

@domenic
Copy link
Member

domenic commented Jul 19, 2017

While in the area for #2845, I noticed the following small tweaks that we could make to improve that section of the spec:

  • Replace "abort these steps" with "return" everywhere. (The latter is a more modern convention, per Infra.)
  • Link "multiplying" in the definition of transform(a, b, c, d, e, f) to https://drafts.fxtf.org/geometry/#matrix-multiply
  • Re-do setTransform(a, b, c, d, e, f) steps 2-3 to reset the current transformation matrix to the appropriate matrix (a c e / b d f / 0 0 1), instead of resetting the matrix to the identity matrix then invoking the transform() method. It's bad practice to invoke methods from other methods.

This is a good first bug :)

@domenic domenic added clarification Standard could be clearer good first issue Ideal for someone new to a WHATWG standard or software project labels Jul 19, 2017
@junov
Copy link
Member

junov commented Jul 19, 2017

cc @whatwg/canvas

@annevk
Copy link
Member

annevk commented Jan 29, 2018

Another thing that would be really nice here if we could inline the "The transformations must be performed in reverse order." requirement. Action-at-a-distance is a pattern we actively try to avoid these days.

@tawandamoyo
Copy link
Contributor

tawandamoyo commented Mar 1, 2024

While in the area for #2845, I noticed the following small tweaks that we could make to improve that section of the spec:

  • Replace "abort these steps" with "return" everywhere. (The latter is a more modern convention, per Infra.)
  • Link "multiplying" in the definition of transform(a, b, c, d, e, f) to https://drafts.fxtf.org/geometry/#matrix-multiply
  • Re-do setTransform(a, b, c, d, e, f) steps 2-3 to reset the current transformation matrix to the appropriate matrix (a c e / b d f / 0 0 1), instead of resetting the matrix to the identity matrix then invoking the transform() method. It's bad practice to invoke methods from other methods.

This is a good first bug :)

Giving this a shot. @domenic, would it be correct to replace steps 2-3 of setTransform(a, b, c, d, e, f), with:

reset the current transformation matrix to the matrix described by,

a c e
b d f
0 0 1

?

@domenic
Copy link
Member Author

domenic commented Mar 4, 2024

Yes, that's exactly right!

@tawandamoyo
Copy link
Contributor

Another thing that would be really nice here if we could inline the "The transformations must be performed in reverse order." requirement. Action-at-a-distance is a pattern we actively try to avoid these days.

hi @annevk , how would I do this?

@Kaiido
Copy link
Member

Kaiido commented Mar 4, 2024

That "The transformations must be performed in reverse order." almost sounds like a note rather than an actual normative requirement. It seems to be there since always though it got split up from the note below at some point.

I might miss something, but I don't think it makes sense "normatively". The transformations are added to the "current transformation matrix" in the order the various methods are called. It may conceptually appear as if they are "applied" in reverse order, but the final matrix is actually applied as a whole, not every transformations on their own.

I guess it could be moved back into the note (maybe with a bit of rephrasing), or even removed entirely (along with the note below)?

@domenic
Copy link
Member Author

domenic commented Mar 11, 2024

Oops, I didn't mean to close this, as the reverse-order issue remains open.

I agree that it's pretty confusing. I think the right solution is something like the following:

  • Remove the sentence "The transformations must be performed in reverse order."

  • Inside the following note, add an introductory sentence/paragraph saying something like: "Because of how successive transformations compose to create the [current transformation matrix], transformations will be performed on drawn shapes in the reverse order of how they are applied to the canvas."

@domenic domenic reopened this Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer good first issue Ideal for someone new to a WHATWG standard or software project topic: canvas
Development

Successfully merging a pull request may close this issue.

6 participants
@domenic @annevk @junov @Kaiido @tawandamoyo and others