-
Notifications
You must be signed in to change notification settings - Fork 34
Invalid eye transform #32
Comments
Hi, i tried your changes that you mentioned previously from your branch at https://github.com/m4gr3d/godot_oculus_mobile/tree/fix_invalid_eye_transform. I also tried to rewrite the the OvrMobileSession::get_transform_for_eye function to use directly the data provided in head_tracker.Eye[eye].ViewMatrix from the ovr SDK. This is how the VrCubeWorld_NativeActivity sample renders. This is also how godot_open_vr extracts the eye transform. I then compared the resulting eye transform using godot_transform_as_string from your code and from using my code (ViewMatrix directly) rotation is identical; but the eye position is slightly different: here is the Log output (first transform is my code; second transform line is your code; your code seems to only move the eye along x-axis independent of head rotation...) 08-04 17:04:08.408 14144 14160 V GodotOVRMobile: Transform Comparison for godot_eye 1 08-04 17:04:08.408 14144 14160 V GodotOVRMobile: 0.715116, -0.121121, 0.688432, 0.216813, 0.974734, -0.053725, -0.664531, 0.18768, 0.723308 - 0.163868, 1.011757, -0.242753 08-04 17:04:08.408 14144 14160 V GodotOVRMobile: 0.715116, -0.121121, 0.688432, 0.216813, 0.974734, -0.053725, -0.664531, 0.18768, 0.723308 - 0.154765, 1.018685, -0.263985 08-04 17:04:08.408 14144 14160 V GodotOVRMobile: Transform Comparison for godot_eye 2 08-04 17:04:08.409 14144 14160 V GodotOVRMobile: 0.715116, -0.121121, 0.688432, 0.216813, 0.974734, -0.053725, -0.664531, 0.18768, 0.723308 - 0.209564, 1.025612, -0.285218 08-04 17:04:08.409 14144 14160 V GodotOVRMobile: 0.715116, -0.121121, 0.688432, 0.216813, 0.974734, -0.053725, -0.664531, 0.18768, 0.723308 - 0.218667, 1.018685, -0.263985 From testing using the ViewMatrix directly feels quite good; but hard to tell if it is correct. |
@NeoSpark314 Good catch! Using the ViewMatrix is definitely the way to go. @BastiaanOlij any idea why that logic would be an issue for the oculus_mobile plugin, and not for the other VR plugins? |
@BastiaanOlij In addition, that change seems to drastically improve performance for the oculus mobile plugin. To a point where the requirement to enable extra latency mode (added by PR #24) is no longer necessary. |
The code in the visual server attempts to create a view frustum that
encompasses the frustums of both eyes so we only need to cull once and do
stuff like updating shadows once. It may be that it fails if the frustums
are identical or not set up right.
On Mon, 5 Aug 2019 at 2:06 pm, Fredia Huya-Kouadio ***@***.***> wrote:
@BastiaanOlij <https://github.com/BastiaanOlij> In addition, that change
seems to drastically improve performance for the oculus mobile plugin. To a
point where the requirement to enable extra latency mode (added by PR #24
<#24>) is no longer
necessary.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#32?email_source=notifications&email_token=AAO262NJ5MKHRDCG7S3ZLDTQC6RK3A5CNFSM4IHO6IN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3QT4NI#issuecomment-518078005>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAO262PTLUWS6PCFTQNDRULQC6RK3ANCNFSM4IHO6INQ>
.
|
Btw, the change you did in the visual server turns stereo rendering off,
now you’re just rendering a normal camera. It makes sense it ‘fixes’ the
culling if it could merge the frustums together (which it only uses for
culling)
On Mon, 5 Aug 2019 at 2:25 pm, Bastiaan Olij ***@***.***> wrote:
The code in the visual server attempts to create a view frustum that
encompasses the frustums of both eyes so we only need to cull once and do
stuff like updating shadows once. It may be that it fails if the frustums
are identical or not set up right.
On Mon, 5 Aug 2019 at 2:06 pm, Fredia Huya-Kouadio <
***@***.***> wrote:
> @BastiaanOlij <https://github.com/BastiaanOlij> In addition, that change
> seems to drastically improve performance for the oculus mobile plugin. To a
> point where the requirement to enable extra latency mode (added by PR #24
> <#24>) is no longer
> necessary.
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <#32?email_source=notifications&email_token=AAO262NJ5MKHRDCG7S3ZLDTQC6RK3A5CNFSM4IHO6IN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3QT4NI#issuecomment-518078005>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAO262PTLUWS6PCFTQNDRULQC6RK3ANCNFSM4IHO6INQ>
> .
>
--
Kindest regards,
Bastiaan Olij
https://www.facebook.com/bastiaan.olij
https://twitter.com/mux213
https://www.youtube.com/BastiaanOlij
<https://www.youtube.com/channel/UCrbLJYzJjDf2p-vJC011lYw>
https://github.com/BastiaanOlij
|
I have the same culling problem when using two asymmetrical projection
matrices for stereo rendering on the Powerwall. Could the code for
combining the frustums have a problem with of center projections?
…On Mon, Aug 5, 2019, 06:28 Bastiaan Olij ***@***.***> wrote:
Btw, the change you did in the visual server turns stereo rendering off,
now you’re just rendering a normal camera. It makes sense it ‘fixes’ the
culling if it could merge the frustums together (which it only uses for
culling)
On Mon, 5 Aug 2019 at 2:25 pm, Bastiaan Olij ***@***.***> wrote:
> The code in the visual server attempts to create a view frustum that
> encompasses the frustums of both eyes so we only need to cull once and do
> stuff like updating shadows once. It may be that it fails if the frustums
> are identical or not set up right.
>
> On Mon, 5 Aug 2019 at 2:06 pm, Fredia Huya-Kouadio <
> ***@***.***> wrote:
>
>> @BastiaanOlij <https://github.com/BastiaanOlij> In addition, that
change
>> seems to drastically improve performance for the oculus mobile plugin.
To a
>> point where the requirement to enable extra latency mode (added by PR
#24
>> <#24>) is no longer
>> necessary.
>>
>> —
>> You are receiving this because you were mentioned.
>>
>>
>> Reply to this email directly, view it on GitHub
>> <
#32?email_source=notifications&email_token=AAO262NJ5MKHRDCG7S3ZLDTQC6RK3A5CNFSM4IHO6IN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3QT4NI#issuecomment-518078005
>,
>> or mute the thread
>> <
https://github.com/notifications/unsubscribe-auth/AAO262PTLUWS6PCFTQNDRULQC6RK3ANCNFSM4IHO6INQ
>
>> .
>>
> --
> Kindest regards,
>
> Bastiaan Olij
>
> https://www.facebook.com/bastiaan.olij
> https://twitter.com/mux213
> https://www.youtube.com/BastiaanOlij
> <https://www.youtube.com/channel/UCrbLJYzJjDf2p-vJC011lYw>
> https://github.com/BastiaanOlij
>
--
Kindest regards,
Bastiaan Olij
https://www.facebook.com/bastiaan.olij
https://twitter.com/mux213
https://www.youtube.com/BastiaanOlij
<https://www.youtube.com/channel/UCrbLJYzJjDf2p-vJC011lYw>
https://github.com/BastiaanOlij
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#32?email_source=notifications&email_token=AAAXWZFJ26NTIZZNVWOJGSDQC6T7VA5CNFSM4IHO6IN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3QUVXA#issuecomment-518081244>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAXWZGXGBU6NAHBHYBB5XTQC6T7VANCNFSM4IHO6INQ>
.
|
Can you clarify how stereo rendering is turned off? Before the change, the logic would call When testing on the device, all 3d objects (controllers, table) are rendered at the proper depth with the correct stereo separation. |
Thats what I get for reading it on a mobile phone :) Yes you are right, it will do prepare and render twice, however it doesn't make sense that it would render differently when you prepare once with the combined frustum and then have problems calling render_scene. It is render_scene that will define the projection matrix and render out the scene. all prepare scene does is the culling and shadow updates and stuff like that. Keep in mind, this is working fine for every other headset so if it fails, it fails because we're not returning the correct matrices |
One thing that may throw things off is that it is expecting the camera left/right offset for each eye to be returned in get_transform_for_eye not mixed in with the projection matrix returned by get_projection_for_eye |
Hi, I think I found the issue with the clipping: In OvrMobileSession::fill_projection_for_eye the oculus provided projection matrix is directly used and the given function parameters z_near and z_far are ignored. The problem now is that oculus uses an +infinite far plane for their projection (the reason is documented here: https://developer.oculus.com/reference/mobile/1.24/vr_api_helpers_8h/ in the documentation of the function static ovrMatrix4f ovrMatrix4f_CreateProjection: "...except for things right up against the near plane, it always provides better precision...") But in this case the godot z far computation code in CameraMatrix::get_z_far() returns 0.0 instead of +inf. One solution to this problem would be to add to visual_server_scene.cpp in the function VisualServerScene::render_camera An alternative is to respect the given near and far values in vrMobileSession::fill_projection_for_eye
at line 198 after getting the ovrMatrix4f matrix from the tracking state. |
Indeed, having an invalid far plane value would screw up the entire calculation. Interesting choice from Oculus because the reason you don't have an infinite far plane is because the near<->far range directly impacts the precision of your Z-buffer. With an infinite far plane the depth buffer would fail to work properly. So they're obviously doing more here. |
(btw, if we check for z_far == 0 in the code that merges the frustums and change it to something arbitrary like 4096.0 (max value for the far plane property in Godot) in theory the logic will work. |
The logic seems work even when setting z_far to +infinity if it was 0 but I did not check the result in detail. Oculus references this paper to explain the choice https://www.cs.cornell.edu/~paulu/tightening.pdf and writes in there documentation |
Fixed by PR #36 |
The plugin doesn't seem to properly set the eye transforms as the rendered 3D content is not stereoscopic.
The text was updated successfully, but these errors were encountered: