-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Comments
cc @whatwg/canvas |
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. |
Giving this a shot. @domenic, would it be correct to replace steps 2-3 of
? |
Yes, that's exactly right! |
hi @annevk , how would I do this? |
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)? |
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:
|
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.)transform(a, b, c, d, e, f)
to https://drafts.fxtf.org/geometry/#matrix-multiplysetTransform(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 :)
The text was updated successfully, but these errors were encountered: