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

[css-transforms-1] Fix mistakes in Example 5 (closes #4767) #5690

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

afdw
Copy link
Contributor

@afdw afdw commented Oct 31, 2020

No description provided.

@afdw
Copy link
Contributor Author

afdw commented Oct 31, 2020

Marked as non substantive for IPR from ash-nazg.

@astearns astearns requested a review from smfr November 4, 2020 02:58
@astearns
Copy link
Member

astearns commented Nov 4, 2020

@smfr this seems fine to me, but I’d like to get your eyes on it.

@mclegrand
Copy link
Member

mclegrand commented Nov 6, 2020

Ah, I missed this MR when I finally decided to act on my report #5520 this morning (in #5702), sorry. I think the illustration also needs an update

@afdw
Copy link
Contributor Author

afdw commented Nov 6, 2020

@mclegrand my PR was designed so that it corresponds to the illustration. See http://web.cse.ohio-state.edu/~wang.3602/courses/cse5542-2013-spring/6-Transformation_II.pdf, there are 2 different ways to think about transformations. So I think my version is actually fully coherent if we think about it as post-multiplication / transform with respect to the local origin and basis. Also, this corresponds to the order of matrices in the resulting product. But the canvas specification implies the other variant (pre-multiplication / transform with respect to the global origin and basis). So what do you think? Which approach will be better?

@mclegrand
Copy link
Member

Well, the important thing is that it's the same example as in Example 1 and Example 2 :)

IMO, the whole spec (mostly section 3) revolves around the CTM (current transformation matrix) concept which (still IMO) works best when thought in terms of pre-multiplication ( f(x) = F×x multiplied by the left ). But even with post-mul, I don't think the illustration makes sense as it shows an object being modified and not a coordinate system ?

@afdw
Copy link
Contributor Author

afdw commented Nov 29, 2020

But even with post-mul, I don't think the illustration makes sense as it shows an object being modified and not a coordinate system ?

Yes, the axis are kind of implicit currently, and should probable be added.

IMO, the whole spec (mostly section 3) revolves around the CTM (current transformation matrix) concept which (still IMO) works best when thought in terms of pre-multiplication ( f(x) = F×x multiplied by the left ).

My opinion is that ideally both approaches to thinking about it should be described. I think it would eliminate the confusion one might have here. As this is a specification about transformations, this should should not be too unrelated I think?

Base automatically changed from master to main February 2, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants