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 head rotation #140

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

chris-hld
Copy link
Contributor

@chris-hld chris-hld commented Apr 3, 2019

This fixes #135
and also corrects the reference angle when using head trackers.
Calibrating the head tracker was turning the head to [1, 0, 0], e.g. to the right.

@mgeier
Copy link
Member

mgeier commented Apr 4, 2019

I don't have access to any head trackers, so I can't check myself, but having to add 90 degrees to each of them looks like there's a bug somewhere.

There's currently a bug regarding loudspeakers (with a fix in 4c66c88, which is part of #131), probably there is another bug (that I introduced) related to the head?

Regarding the GUI stuff, I think you have to add the values of _scene.get_reference() and _scene.get_reference_offset(). The latter only contains an offset. To get the absolute value you have to add the reference orientation (which is 90 degree by default).

@chris-hld
Copy link
Contributor Author

Thanks Matthias!
This PR was intended to be more of a 'hotfix' for the master, and to get this fix started.
However, I found the problem/bug to be the additional 90 deg rotation in
void reference_offset_rotation() override.
Everything works as expected now.

@mgeier
Copy link
Member

mgeier commented Apr 5, 2019

Thanks for the update.

If you use "rotation" + "rotation offset" for displaying the head, you should probably also use "position" + "position offset", right?

The "rotation offset" has now an error of 90 degrees, both in the display of the head and of the symbols for loudspeaker-based renderers. Probably in the audio-rendering, too, but I didn't check this.

I'm sorry for the mess with the orientations, I've been changing the orientations of the "new stuff" to be 90 degrees rotated from the "legacy stuff".
I've implemented this change in the conversion functions between Orientation (legacy) and Rot (new):

/// Convert from 3D rotation.
Orientation::Orientation(const ssr::Rot& three_d_rot) :
azimuth(apf::math::rad2deg(quat2azi(three_d_rot)) + 90.0f)
{}
Orientation::operator ssr::Rot()
{
return azi2quat(apf::math::deg2rad(this->azimuth - 90.0f));
}

This works for "absolute" rotations/orientations, but sadly this is wrong for "relative" rotations, as used in the "reference orientation offset". Therefore I had to compensate this for the API functions that affect the "reference orientation offset", which you are trying to remove again in this PR.

I didn't check this for the trackers (because I don't have access to any trackers), that's why there are still bugs in the tracker code.

I've just added 6731ea6 to #131, which should make it easier to test the "reference orientation" separate from the "reference orientation offset" (without needing a head tracker).

I'm still waiting for a review of #131, but I hope I can merge it some time next week.

Then it would be great if you could rebase this PR onto that.

@mgeier
Copy link
Member

mgeier commented Apr 5, 2019

PS: If you like, you can try to extend the Razor tracker to 3D at some point, see:

// TODO: get 3D rotation

IIRC, it should be possible to get quaternions from the Razor device, which can be passed directly to reference_rotation_offset() without needing a conversion via the legacy Orientation type.

@mgeier
Copy link
Member

mgeier commented Apr 5, 2019

PPS: I've added a visualization for the "reference offset" to my 3D-GUI prototype in 6b52014.

@mgeier
Copy link
Member

mgeier commented Apr 10, 2019

@chris-hld I've merged #131, can you please rebase this PR onto master?

@chris-hld
Copy link
Contributor Author

Thanks Matthias! The web interface is great!
Rotation as well as my razor head tracker seems to work nicely together with the reference and reference offset now.
I'm a bit confused about the 90 degree offset in the head tracker myself, but I guess that comes from the new coordinate system conventions? You are now counting the Azimuth from in front instead of to the right? So I guess compensating this while "reset/calibrate" seems necessary for now.

@mgeier
Copy link
Member

mgeier commented Apr 12, 2019

Thanks for the update!


When using a loudspeaker-based renderer with the Qt GUI, I see this problem:

  • Left-dragging manipulates the "reference rotation offset"
  • Right-dragging manipulates the "reference position"

I'm not sure if there is a clear argument for whether or not to manipulate the "offset", but at least both left and right button should affect the same thing, right?

I think it makes slightly more sense to manipulate the "reference rotation" with the mouse, but I'm not sure.
This way you could wear a head tracker (which manipulates "reference rotation offset") and at the same time use the mouse to manipulate the "reference rotation".

Please note that the "reference rotation offset" is a property of the renderer, while the "reference rotation" is a property of the scene.
This doesn't make too much difference currently, but in the future, multiple users might be able to share the same scene, while each of them has their own renderer.


When using the binaural renderer with the Qt GUI, I see this problem:

  • The "reference offset position" is not visible

I don't know if you want to fix this in this PR or not.
But since "rotation + offset" is shown, it would make sense to also show "position + offset", wouldn't it?

If I'm not mistaken, the binaural renderer actually takes the "reference position offset" into account when rendering.


I'm a bit confused about the 90 degree offset in the head tracker

Yeah, sorry, me too.

It is very confusing to have both ways at the same time. I hope the new system on its own is less confusing than the old system

You are now counting the Azimuth from in front instead of to the right?

Yes, and no.

I replaced the "absolute" Orientation type with a "relative" Rot (which obviously stands for "rotation"), which happens to be stored as a quaternion.

Now there is no "absolute" orientation of the reference, just a "relative" rotation based on its "original" orientation.

The implicit "original" orientation of things is "looking north", which means "along the positive y-axis".

In the old system, we always had to explicitly add 90 degrees at some point when setting up stuff. Now we don't, because the "original" orientation is implicit.

So I guess compensating this while "reset/calibrate" seems necessary for now.

Yes, this is because of the automatic conversion from Orientation to Rot, which subtracts 90 degrees. This has to be compensated at some point for "non-absolute" Orientations (which are used for the "offset").

However, I think it makes more sense to compensate it directly before or after the conversion.

Now you have + 90 in the tracker constructor and + 90 in the calibrate() function.

I think it would be simpler to use + 90 only once, in a situation like this:

_controller.take_control()->reference_rotation_offset(
    Orientation(-_current_azimuth + _az_corr + 90))

Once the trackers are upgraded to 3D (and to using quaternions), the conversion to Orientation goes away, and with it the + 90 compensation.

I'm not sure if that would actually work, though. I didn't test it.

@mgeier
Copy link
Member

mgeier commented Jul 9, 2023

In the meantime, there have been several changes (a.k.a. "fixes") to the GUI.

The binaural GUI can now distinguish between "reference position/rotation" and "reference offset position/rotation", just as the loudspeaker GUI already did before.

Dragging with the mouse now consistently affects only the "reference position/rotation", while the head trackers only affect "reference offset rotation".

Furthermore, the Polhemus tracker code has been extended to support 3D rotations. A 90 degree rotation has been added there to compensate for the fact that the tracker is typically mounted sideways. If it would be mounted "correctly" (which would be a hassle), the 90 degree rotation would not be necessary.

I think the only thing from this PR that has not yet been addressed are the Razor and VRPN trackers.

Do they work correctly now?

Either way, it would be very nice to extend both of them to 3D. Both already provide 3D rotations, so this should be easy to implement. The only problem is that whoever implements it needs access to the actual trackers in order to find out what their conventions are and what compensation is necessary.

I don't have access to any tracker, but I can assist via video call. It can be tricky to get 3D rotations right ...

@chris-hld, @JensAhrens: Do you currently have access to Razor or VRPN trackers?

@mgeier
Copy link
Member

mgeier commented Jul 9, 2023

I have just realized that #146 is already trying to fix VRPN trackers, so we can continue the discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GUI: Head not turning anymore
2 participants