godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.33k stars 21.06k forks source link

Problem with camera direction VR #12602

Closed AndersonFirmino closed 6 years ago

AndersonFirmino commented 6 years ago

Operating system or device, Godot version, GPU Model and driver (if graphics related):

Android 6.0 (Asus Zefone2) Godot 3.0 alpha 2

Issue description:

I'm testing the VR mode on Android.

And when I move my head to the left the camera goes to the right. The camera is assuming that Left = Right Right = Left.

I do not know if it's bug or my fault. Could someone guide me with this problem?

Thanks in advance for any help. 🍰

Steps to reproduce:

  1. Install the APK or mount one with the project.
  2. Install on android and open the application.
  3. Move your head left or right.

I'll leave the APK file. And the Project I'm using for testing.

Link to minimal example project: Project APK

akien-mga commented 6 years ago

CC @BastiaanOlij

BastiaanOlij commented 6 years ago

@AndersonFirmino very possible that is wrong. I'm assuming you are using the ARVR Native Mobile interface?

I had issues with the axis being mixed up on Android but as I haven't got a suitable Android phone to test with I had to go based on the experimenting we did with 2.1. It may very well be that they are now the reverse of what they should be as things have changed in Godot 3.

Note the code here: https://github.com/godotengine/godot/blob/master/modules/mobile_vr/mobile_interface.cpp#L155 This reverses the gyro on Android. If someone since I wrote the original logic has fixed up the axis for android it is very possible that code is no longer needed. It is also possible there are settings in Android that change the output and this code is only needed sometimes.

Again I lack the hardware to test :( It would be good if someone with an Android phone could do more testing here.

AndersonFirmino commented 6 years ago

@BastiaanOlij I'm using Native Mobile ARVR. Looking at the case and analyzing the code I understood the problem. Unfortunately I am not very skilled with C++. But I can help by testing how many times you want here on my android. I really want to be able to create VR on Godot Engine applications.

And I would love to be able to contribute to the development of Godot Engine in some way. :cake:

AndersonFirmino commented 6 years ago

Talking with @BastiaanOlij he suggested that I remove these lines. https://github.com/godotengine/godot/blob/master/modules/mobile_vr/mobile_interface.cpp#L155 I did. And I compiled a version of Godot and compiled the export of this version.

Now the camera is assuming that: Left = Right Right = Left Up = Down Down = Up

@BastiaanOlij would you mind compiling a version so I could run tests? 😅 My computers took "half a day" to compile everything. 😞

BastiaanOlij commented 6 years ago

Hey @AndersonFirmino ,

Now that its compiled making changes and compiling again shouldn't take that long. Half a day is alot though. Unfortunately I don't have an android environment to test.

Interesting that the right/left is still reversed but up/down is now also reversed. That leads me to wonder what sensors your device has and whether we're using the combo Gyro + Accelerometer or Magnetometer + Accelerometer (gyro + accelerometer is the better one).

Best is to try things one at a time and see what works.

This code applies the gyro:

if (has_gyro) {
    Basis rotate;
    rotate.rotate(orientation.get_axis(0), gyro.x * delta_time);
    rotate.rotate(orientation.get_axis(1), gyro.y * delta_time);
    rotate.rotate(orientation.get_axis(2), gyro.z * delta_time);
    orientation = rotate * orientation;

This code works if there is no gyro and combines the magnetometer and accelerometer:

if (has_magneto && has_grav && !has_gyro) {
    // convert to quaternions, easier to smooth those out
    Quat transform_quat(orientation);
    Quat acc_mag_quat(combine_acc_mag(grav, magneto));
    transform_quat = transform_quat.slerp(acc_mag_quat, 0.1);
    orientation = Basis(transform_quat);

And finally this code applies the accelerometer correction if you do have a gyro:

} else if (has_grav) {
    // use gravity vector to make sure down is down...
    // transform gravity into our world space
    grav.normalize();
    Vector3 grav_adj = orientation.xform(grav);
    float dot = grav_adj.dot(down);
    if ((dot > -1.0) && (dot < 1.0)) {
        // axis around which we have this rotation
        Vector3 axis = grav_adj.cross(down);
        axis.normalize();

        Basis drift_compensation(axis, acos(dot) * delta_time * 10);
        orientation = drift_compensation * orientation;
    };

It would be good to comment out the last 2 code blocks, so the code that uses the magnetometer or applies drift correction on the accelerometer. That leaves you with just the gyro working. You can try that both without inverting the x and z axis, and with inverting the x and z axis. If just using the gyro gives no change in orientation that means your device does not have a gyro. It is very possible that only the code using the gyro is working (with the axis inverted) as that is what was tested before in the 2.1 implementation by other people and there may be a mistake in the combine_acc_mag code. But lets try one step at a time and not get ahead of ourselves.

So yeah, if you can try commenting out everything from: https://github.com/godotengine/godot/blob/master/modules/mobile_vr/mobile_interface.cpp#L177 until: https://github.com/godotengine/godot/blob/master/modules/mobile_vr/mobile_interface.cpp#L201 And try that out both with, and without inverting the x and z axis, that be a good start :)

AndersonFirmino commented 6 years ago

Just to complement I will leave here a screenshoot of the sensors that my device has

sensors2

sensors3

sensors

When I bought this smartphone it was to use with VR

AndersonFirmino commented 6 years ago

I'll change the lines and make new compilations and I'll come back with the results. 👨‍💻

AndersonFirmino commented 6 years ago

@BastiaanOlij First test, done. Commenting on the two blocks of code you suggested and making a new apk the directions remain inverted. 😞

AndersonFirmino commented 6 years ago

I did not quite understand the part you mentioned that we could reverse the directions of the gyroscope. How can I do this in code?

I think this will probably work.

BastiaanOlij commented 6 years ago

@AndersonFirmino we're getting somewhere:) Ok so the gyro works, thats good. We're just talking about direction. The thing to understand is how the different components work together and trying them out one by one, or fixing one will be undone by having the other wrong and we'll get confused :)

Gyro is applied first. The gyro, even though it returns a vector, actually measure rotational velocity. So grav.x is the speed at which we're rotating around the X axis, grav.y is the speed at which we're rotating around the Y axis and grav.z is the speed at which we're rotating around the Z axis. The code I asked you disable waaaaay at the beginning inverted grav.x and grav.z but not grav.y. Removing that code made everything worse, so my guess is that all 3 axis are reversed.

The Magnetometer is a vector in line with the sum of magnetical forces acting on the phone. In normal circumstances it points to magnetic north but it can easily be swayed by any magnets that are near enough. We'll ignore it for now but once we have everything else working it would be good to ignore the gyro for a minute and try out if we it is setup correctly.

Finally the accelerometer returns a vector that points in the direction of the sum of acceleration forces acting on the phone. When the phone is in rest it should be pointing straight down as only gravity forces are acting on the phone. The logic in the OS has extra logic to try and filter out other forces acting on the phone when its moving and return a gravity vector which is the one we use. We use this vector to stabilise the orientation of the phone.

Ok, so next step, keep the magneto and accelerometer logic disabled for now but put back this logic:

#ifdef ANDROID_ENABLED
    // On Android axis seem inverted
    gyro.x = -gyro.x;
    gyro.y = -gyro.y;
    gyro.z = -gyro.z;
    grav.x = -grav.x;
    grav.z = -grav.z;
    magneto.x = -magneto.x;
    magneto.z = -magneto.z;
#endif

Note that I have added gyro.y = -gyro.y It is possible that the same needs to be done with the grav and magneto values.

AndersonFirmino commented 6 years ago

@BastiaanOlij Okay, if I understood correctly, I would disable this logic.

if (has_magneto && has_grav && !has_gyro) {
        // convert to quaternions, easier to smooth those out
        Quat transform_quat(orientation);
        Quat acc_mag_quat(combine_acc_mag(grav, magneto));
        transform_quat = transform_quat.slerp(acc_mag_quat, 0.1);
        orientation = Basis(transform_quat);

        tracking_state = ARVRInterface::ARVR_NORMAL_TRACKING;
    } else if (has_grav) {
        // use gravity vector to make sure down is down...
        // transform gravity into our world space
        grav.normalize();
        Vector3 grav_adj = orientation.xform(grav);
        float dot = grav_adj.dot(down);
        if ((dot > -1.0) && (dot < 1.0)) {
            // axis around which we have this rotation
            Vector3 axis = grav_adj.cross(down);
            axis.normalize();

            Basis drift_compensation(axis, acos(dot) * delta_time * 10);
            orientation = drift_compensation * orientation;
        };
    };

And I add this logic

#ifdef ANDROID_ENABLED
    // On Android axis seem inverted
    gyro.x = -gyro.x;
    gyro.y = -gyro.y;
    gyro.z = -gyro.z;
    grav.x = -grav.x;
    grav.z = -grav.z;
    magneto.x = -magneto.x;
    magneto.z = -magneto.z;
#endif

I compile the code and test it. If it does not work I'll try this

#ifdef ANDROID_ENABLED
    // On Android axis seem inverted
    gyro.x = -gyro.x;
    gyro.y = -gyro.y;
    gyro.z = -gyro.z;
    grav.x = -grav.x;
    grav.y = -grav.y;
    grav.z = -grav.z;
    magneto.x = -magneto.x;
    magneto.y = -magneto.y;
    magneto.z = -magneto.z;
#endif
AndersonFirmino commented 6 years ago

I will do new tests and return with the results.

AndersonFirmino commented 6 years ago

I compiled the APK here for android and the camera stays:

Left = Right Right = Left Up = Down Down = Up

BastiaanOlij commented 6 years ago

Hmm, very confusing. I'm going to see if I can borrow a friends Android phone. That might be a bit more effective then pinging back and forth like this :)

One last thing to try, maybe just invert the y axis like this:

#ifdef ANDROID_ENABLED
    // On Android axis seem inverted
    gyro.y = -gyro.y;
    grav.y = -grav.y;
    magneto.y = -magneto.y;
#endif
AndersonFirmino commented 6 years ago

@BastiaanOlij I did the tests with this code. And the camera did not move anymore. 😞

BastiaanOlij commented 6 years ago

OK, that doesn't make sense, you must have removed something more...

I really need to get my hands on an android device.... :(

AndersonFirmino commented 6 years ago

In any case I will leave attached the CPP file that I am compiling so you can take a look. If you can change it and upload it here. Then I can do new tests. Maybe I did not understand something that should have been changed in the code. :(

mobile_interface.zip

BastiaanOlij commented 6 years ago

Whow.. I just notices I've mixed up the has_gyro and has_grav checks. SUPER AUCH. Don't know if that explains anything..

I've changed that in the attached file (I'll do a PR for it in a minute), but nothing else stands out to me, this should just work...

mobile_interface.cpp.zip

BastiaanOlij commented 6 years ago

@AndersonFirmino I found a few more changes for that grav/gyro mixup so see #12734 for details.

Peter is dropping his note 4 off at my place on Friday so this weekend with a bit of luck I'll be able to test this myself and figure out what is going on.

AndersonFirmino commented 6 years ago

I'm going to do new tests here to see how it behaves. I am anxious to compile haha. I even dreamed that I was compiling the APK and the VR started to work. lol 😆

BastiaanOlij commented 6 years ago

Thanks for trying @AndersonFirmino, I know this back and forth is less then ideal :)

AndersonFirmino commented 6 years ago

That nothing. About helping with VR. It is more than an honor to collaborate with Godot. I really like this engine. Besides being passionate about virtual reality I can not wait to be able to create my first experiences for Android.

AndersonFirmino commented 6 years ago

Now you've had that problem before. :(

The camera is responding: Left = Right Right = Left Up = Down Down = Up

BastiaanOlij commented 6 years ago

@AndersonFirmino the PR only fixes the has_grav and has_gyro variables, didn't change anything around orientation.

I think we'll keep running around in circles until I get my hands on a device this weekend.

That reminds me, I need to find some time to create a little test project that lets you play with the accelerometer/magnetometer and gyro as a normal app. That would also help debug this a lot.

BastiaanOlij commented 6 years ago

I just noticed in your screenshots, your PSH Gyroscope sensor shows all zeroes. When you move the device, do those values change?

AndersonFirmino commented 6 years ago

Yes when I move it changes the values. I have a small library of games here that use virtual reality.

AndersonFirmino commented 6 years ago

I think now it would be best for me to wait until you get Android and run the tests there. So we will not be running in circles. 😅

BastiaanOlij commented 6 years ago

@AndersonFirmino I just flung this little test project together, also submitted it as a demo: https://github.com/godotengine/godot-demo-projects/pull/90

It would be worth trying out on Android, seeing what the output is and whether things are drawn correctly and behave correctly

AndersonFirmino commented 6 years ago

I'm going to do new compiling tests with your project. I'll recompile from Godot's master branch and run the tests.

BastiaanOlij commented 6 years ago

This seems to give the correct alignment of axis, solved on the android platform code itself so there no longer is a need to override the android axis.

https://github.com/godotengine/godot/pull/12826

Interestingly enough the magnetometer on android gives much more usable output then on iPhone.

Also did some testing on a Samsung Note 4 with Gear VR (the early development model), had to change the oversample to 1.0 or it was getting pretty slow. Also the LCD width is smaller then the default 14, I need to measure it when I get home but it seems the lenses are pretty centered. I did get a feeling the aspect ratio was off.

AndersonFirmino commented 6 years ago

Woow. So now does VR work on android the camera will get the correct directions?

BastiaanOlij commented 6 years ago

Seemed to work on the note 4, the aspect ratio seemed off which was weird.. I'm going to put a little demo together to better test things.

Might have a closer look at the gear VR SDK tonight aswell.

AndersonFirmino commented 6 years ago

I got your godot fork in the branch you made the changes I'm compiling here to test. The camera worked perfectly 👨‍💻

AndersonFirmino commented 6 years ago

I compiled the latest version and it worked! Thank you very much @BastiaanOlij 🤓

BastiaanOlij commented 6 years ago

Sweet!

akien-mga commented 6 years ago

Fixed by ##12826 then?

AndersonFirmino commented 6 years ago

@akien-mga Yes, it did.

AndersonFirmino commented 6 years ago

I think you can close this issue.

BastiaanOlij commented 6 years ago

Closed it is :)