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

Documentation misleading concerning the order when combining transforms #1608

Closed
pvigier opened this issue Aug 8, 2019 · 3 comments
Closed
Projects
Milestone

Comments

@pvigier
Copy link

pvigier commented Aug 8, 2019

Hello,

If I understand well the source code of sf::Transform, when we do:

auto A = sf::Transform(...);
auto B = sf::Transform(...);
auto C = A.combine(B);

C is equal to A*B with * the matrix multiplication.

And when we want to transform a point:

auto u = sf::Vector2f(...);
auto v = C.transformPoint(u);

We have v = A*B*u.

So it is B that is applied first on u, then A.

However, in the documentation of combine, it is said that:

The result is a transform that is equivalent to applying *this followed by transform.

According, to this sentence, C would be equal to B*A (applying A followed by B). I think it is misleading.

Correct me if I am wrong.

Best regards,
Pierre

@LaurentGomila
Copy link
Member

I don't remember this kind of stuff that was written a long time ago, so the best thing would probably be to test it. A rotation combined with a translation should be a good case to figure out which one is applied first.

@pvigier
Copy link
Author

pvigier commented Aug 9, 2019

Indeed, I have done exactly that before posting to be sure.

Here is the snippet:

auto transform = sf::Transform();
transform.translate(2.0f, 3.0f);
transform.rotate(90.0f);

Then, I print the entries of the matrix:

-4.37114e-08 -1 0 2 
1 -4.37114e-08 0 3 
0 0 1 0 
0 0 0 1 

Which corresponds to rotation first, translation then.

I think that the tutorial on transforms is also misleading. It is said that :

You can apply several predefined transformations to the same transform as well. They will all be combined sequentially:

I think it should be written: "They will all be combined sequentially in reversed order".

Bromeon added a commit that referenced this issue Dec 7, 2019
Clarifies order of combine() and equivalence of operations.
Closes #1608.
@Bromeon
Copy link
Member

Bromeon commented Dec 7, 2019

Thanks for the report, I created a PR in both SFML and SFML-Website to address this.

Bromeon added a commit that referenced this issue Dec 8, 2019
Clarifies order of combine() and equivalence of operations.
Closes #1608.
Baardi pushed a commit to Baardi/SFML that referenced this issue Jan 8, 2020
Clarifies order of combine() and equivalence of operations.
Closes SFML#1608.
Unarelith pushed a commit to Unarelith/SFML that referenced this issue May 28, 2020
Clarifies order of combine() and equivalence of operations.
Closes SFML#1608.
Unarelith pushed a commit to Unarelith/SFML that referenced this issue May 28, 2020
Clarifies order of combine() and equivalence of operations.
Closes SFML#1608.
@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.6.0 via automation May 11, 2021
@eXpl0it3r eXpl0it3r added this to the 2.6 milestone May 11, 2021
@eXpl0it3r eXpl0it3r moved this from Discussion to Done in SFML 2.6.0 May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.6.0
  
Done
Development

No branches or pull requests

4 participants