Fix Transform3D basis to OpenVR HmdMatrix conversion in matrix_from_transform - #191
Merged
Merged
Conversation
Collaborator
|
Your analysis makes sense to me, I have a workaround that I didn't realize was a workaround in my code currently. Thanks! It would be nice to have some actual unit tests someday instead of just the demo, but there's so little code that can be tested without mocking openvr… |
Collaborator
|
Ha! I was thinking there was something off all the time. I remember writing at least one of these functions originally with the overlay code, so might have been me 😅 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I ran into some weird behaviour while trying to rotate overlays. To me it seems like there's an issue in
matrix_from_transform.As far as I can tell,
matrix_from_transformandtransform_from_matrixare supposed to be opposites of each other. So if I pipe a matrix intotransform_from_matrix, and then pull the result back throughmatrix_from_transform, I should end up with the same matrix I started with.However, this currently isn't the case.
Showcase
Take this OpenVR matrix as a starting point:
Run it through
transform_from_matrixto get a Godot transform, then run that transform throughmatrix_from_transformto get an OpenVR matrix back out. Call that resultC. We expectC == A.Currently, this is not the case. When we swap the indices
iandjinmatrix_from_transform, the problem is solved.Why this hasn't surfaced before
The mismatch only changes the result when the 3×3 block being converted is non-symmetric. Every existing caller of
matrix_from_transformin the repo happens to pass a value where this doesn't matter:demo/overlay_main.tscnuses the identity transform forabsolute_position.tracked_device_relative_positionwith just an origin offset (no rotation).