Skip to content
This repository has been archived by the owner on Apr 18, 2022. It is now read-only.

[BUG] world_to_screen returns incorrect result #2057

Closed
aforemny opened this issue Dec 4, 2019 · 2 comments
Closed

[BUG] world_to_screen returns incorrect result #2057

aforemny opened this issue Dec 4, 2019 · 2 comments
Labels
diff: easy Achievable by a single junior developer with a bit of guidance. good first issue This is a good issue to start with if you are new and want to contribute to Amethyst. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. team: engine type: bug Something is not working as it should.

Comments

@aforemny
Copy link

aforemny commented Dec 4, 2019 •
edited
Loading

Description

Hi, I have found that with a perspective camera and a non-trivial transform, the function world_to_screen returns an incorrect result.

Cause of the issue

It seems to me that the following line in world_to_screen

let screen_pos =
    (camera_transform.global_matrix() * self.as_matrix()).transform_point(&world_position);

should really say

let screen_pos =
    (self.as_matrix() * camera_transform.global_matrix().try_inverse().unwrap()).transform_point(&world_position);

so that it is the inverse of the matrix *camera_transform.global_matrix() * self.as_inverse_matrix() used in screen_ray.

Reproduction Steps

None provided.

What You Expected to Happen

I expect that world_to_screen performs the inverse operation of screen_ray.

What Actually Happened

The function world_to_screen does not perform the inverse operation of screen_ray.

Screenshots and Logs

None provided.

Your Environment

  1. NixOS
  2. 19.09.1223.cb2cdab7136 (Loris)
  3. 0.13.2

Additional Context

I hope this issue is clear from the described cause of this issue. If you would like me to provide a test case, please let me know!
@aforemny aforemny added status: needs triage type: bug Something is not working as it should. labels Dec 4, 2019
@aforemny aforemny changed the title [BUG] screen_to_world returns incorrect result [BUG] world_to_screen returns incorrect result Dec 4, 2019
@joonazan
Copy link
Contributor

It doesn't work for me either.

A solution similar to your suggestion fixed it.

@distransient distransient added diff: easy Achievable by a single junior developer with a bit of guidance. good first issue This is a good issue to start with if you are new and want to contribute to Amethyst. status: ready team: engine and removed status: needs triage labels Dec 17, 2019
@AnneKitsune AnneKitsune added the pri: important Something other teams are relying on, or a low-level, critical piece of functionality. label Dec 26, 2019
@AnneKitsune AnneKitsune added this to the 0.15 milestone Dec 26, 2019
AidaPaul added a commit to AidaPaul/amethyst that referenced this issue Jan 1, 2020
@AidaPaul
Correcting world_to_screen amethyst#2057
8d8a147
AidaPaul added a commit to AidaPaul/amethyst that referenced this issue Jan 1, 2020
@AidaPaul
Fixing world_to_screen camera transformation amethyst#2057
e12424d
AidaPaul added a commit to AidaPaul/amethyst that referenced this issue Jan 1, 2020
@AidaPaul
Fixing world_to_screen camera transformation amethyst#2057
0e6cd03
@aforemny aforemny mentioned this issue Jan 2, 2020
Merged
5 tasks
bors bot added a commit that referenced this issue Jan 2, 2020
@bors @AidaPaul
Merge #2087
8c022f4 
2087: Fixing world_to_screen camera transformation #2057 r=azriel91 a=Puciek

## Description

Changed `world_to_screen` camera transformation to match inverse of the one in `screen_ray`

## PR Checklist

By placing an x in the boxes I certify that I have:

- [X] Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment.
- [X] Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme.

If this modified or created any rs files:

- [X] Ran `cargo +stable fmt --all`
- [X] Ran `cargo clippy --all --features "empty"`
- [X] Ran `cargo test --all --features "empty"`


Co-authored-by: Tymoteusz Paul <puciek@gmail.com>
@aforemny
Copy link
Author

aforemny commented Jan 2, 2020

The patch in #2087 fixes this issue for me.

@aforemny aforemny closed this as completed Jan 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
diff: easy Achievable by a single junior developer with a bit of guidance. good first issue This is a good issue to start with if you are new and want to contribute to Amethyst. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. team: engine type: bug Something is not working as it should.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@aforemny @distransient @AnneKitsune @joonazan