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

[bug] offset + custom screen coordinates #439

Closed
ozkriff opened this issue Jul 22, 2018 · 6 comments
Closed

[bug] offset + custom screen coordinates #439

ozkriff opened this issue Jul 22, 2018 · 6 comments

Comments

@ozkriff
Copy link
Contributor

@ozkriff ozkriff commented Jul 22, 2018

Offset moves image to a strange position when used with custom [-1..+1] coordinate system

Demo repository with two binaries: https://github.com/ozkriff/ggez_scale_offest_bug_demo

^ These test apps are supposed to give similar results, but in absolute_coords image's offset moves it to a strange position - towards the edge of the screen.

(maybe this issue is related to #435)


UPD: Results in this for Zemeroth:

image

@icefoxen
Copy link
Contributor

@icefoxen icefoxen commented Jul 22, 2018

@ozkriff
Copy link
Contributor Author

@ozkriff ozkriff commented Jul 22, 2018

0.4.3

UPD: misclicked close button

@ozkriff ozkriff closed this Jul 22, 2018
@ozkriff ozkriff reopened this Jul 22, 2018
@icefoxen
Copy link
Contributor

@icefoxen icefoxen commented Jul 22, 2018

Well, crap.

@robojumper
Copy link
Contributor

@robojumper robojumper commented Jul 26, 2018

How is offset supposed to work? Both examples produce results that are inconsistent with the documentation, which states:

    /// specifies an offset from the center for transform operations like scale/rotation,
    /// with `0,0` meaning the origin and `1,1` meaning the opposite corner from the origin.
    /// By default these operations are done from the top-left corner, so to rotate something
    /// from the center specify `Point2::new(0.5, 0.5)` here.

yet the window_coords example shows that this affects the translation too.

Code for reference:

translate * offset * rotation * shear * scale * offset_inverse

Making the second example match the first (a fixed offset that affects the translation) is as simple as removing the * offset multiplication from this, while making both examples consistent with the documentation (also producing the same result) is a matter of changing

let offset = Matrix4::new_translation(&Vec3::new(self.offset.x, self.offset.y, 0.0));

to

let offset = Matrix4::new_translation(&Vec3::new(self.offset.x * self.scale.x, self.offset.y * self.scale.y, 0.0));

because in the code, offset_inverse is affected by scale, while offset is not.

@ozkriff
Copy link
Contributor Author

@ozkriff ozkriff commented Jan 12, 2019

The bug is still there in 0.5.0-rc.0, though I confirm that the fix from @robojumper seems to work.

@PSteinhaus
Copy link
Contributor

@PSteinhaus PSteinhaus commented Jun 12, 2021

translate * offset * rotation * shear * scale * offset_inverse

This was the source of this problem, indeed. Multiplying offset into this matrix just didn't make any sense, so we removed it in #880. I'm pretty sure this fixed the problem, therefore I'm closing this now.

@PSteinhaus PSteinhaus closed this Jun 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants