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

Fix coordinate transforms #61

Merged
merged 5 commits into from
May 30, 2019
Merged

Fix coordinate transforms #61

merged 5 commits into from
May 30, 2019

Conversation

erikwijmans
Copy link
Contributor

@erikwijmans erikwijmans commented May 30, 2019

Motivation and Context

#57 raised that the transform on the meshes is weird! This is actually a bug that, due to serendipity, actually works.

This PR fixes this by first fixing CoordinateFrame::rotationWorldToFrame, using that to implement a CoordinateFrame::rotationFrameToWorld, then using that to rotate the mesh to ESP coordinates.

CC: @mosra (no need for review, just tagging for visibility)

How Has This Been Tested

Via pytest (rendering tests would complain) and via viewer

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

@erikwijmans erikwijmans requested a review from msavva May 30, 2019 03:59
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 30, 2019
@mosra
Copy link
Collaborator

mosra commented May 30, 2019

OK, so once this is in I'll do the corresponding change in #57.

Copy link
Collaborator

@msavva msavva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @erikwijmans -- this PR fixes the buried issue and makes things far more readable! Left a couple of small doc comments, but otherwise LGTM! Let's land it. 👍

src/esp/geo/CoordinateFrame.h Outdated Show resolved Hide resolved
src/esp/assets/Asset.cpp Outdated Show resolved Hide resolved
@erikwijmans
Copy link
Contributor Author

Issues addressed

@msavva
Copy link
Collaborator

msavva commented May 30, 2019

Issues addressed

Looks great -- let's land it!

@erikwijmans erikwijmans merged commit 1c38836 into master May 30, 2019
@erikwijmans erikwijmans deleted the fix-rotate branch May 30, 2019 23:57
Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
Fix weird coordinate transform
Fix geo::CoordinateFrame
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants