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

Replacing VR bindings with LWJGL #932

Closed
wants to merge 4 commits into from
Closed

Conversation

neph1
Copy link
Contributor

@neph1 neph1 commented Oct 4, 2018

This is a suggested change. Replaces the homebrewed JNA bindings with LWJGL's JNI bindings.
It is built on LWJGL 3.2.0. Can't guarantee the bindings are all there in prior versions.

Also one major change in flow; WaitGetPoses is called from render() rather than update(). This is because rendering actually happens there, and "frameTimings" show less cpu latency (top bar, this change darker). There are also seemingly less "late frames" (bottom bar, red stuff)

image

There are opportunities for more refactoring in the lib.

Conflicts:
	jme3-vr/src/main/java/com/jme3/app/VRApplication.java

Conflicts:
	jme3-vr/src/main/java/com/jme3/app/VRApplication.java
Conflicts:
	jme3-vr/src/main/java/com/jme3/input/vr/openvr/OpenVRInput.java
	jme3-vr/src/main/java/com/jme3/input/vr/openvr/OpenVRViewManager.java

Conflicts:
	jme3-vr/src/main/java/com/jme3/input/vr/openvr/OpenVRInput.java
	jme3-vr/src/main/java/com/jme3/input/vr/openvr/OpenVRViewManager.java
@jseinturier
Copy link
Contributor

Hello,

I'm not agree with the replacement of the native JNA bindings with LWJGL bindings as the JNA ones are here in order to keep up to date the bindings to the latest version of OpenVR. If we only use LWJGL we will not be able to follow OpenVR as we want.

However, we can add LWJGL bindings use as an alternative of Occulu and OpenVR existing mappings.

@neph1
Copy link
Contributor Author

neph1 commented Oct 4, 2018

I contemplated that, but why have many different implementations that do the same thing? We just get fragmentation, more support requests and cluttered code. By offseting the support to lwjgl we get less reliant on individual committers. We also increase their usage base which makes them more interested in keeping up support.

@jseinturier
Copy link
Contributor

There is at this time differents implementations (OpenVR and Occulus) so removing OpenVR and replacing it by LWJGL will not solve fragmentation. This fragmentation is due to the various solutions provided for VR Rendering and one of the strength of our implementation is to be able to deal with these various solutions.

For my own example, if i'am limited to the use of LWJGL bindings to OpenVR (that relies on old version and that evolves quite slow) i don't know if i will be interested by using JMonkey for VR rendering as i will not be able to integrate lasts version of the underlying systems.

@jseinturier
Copy link
Contributor

We can integrate LWJGL binding for ones that search for stability and support but we can keep the possibility to use other implementations for the ones that are interested.

@neph1
Copy link
Contributor Author

neph1 commented Oct 4, 2018

As far as I can see, lwjgl is using openvr 1.0.16, which is 2 months old. The latest update of openvr in jme3-vr is 2 years old and is using 1.0.6. How is that evolving more slowly?

All I want is to make updating as simple as possible. What if we have two different openvr implementations. Do we have to supply both with every release too?

@jseinturier
Copy link
Contributor

Hum... The jme-vr module has been updated 7 months ago and relies on OpenVR 1.0.9.

The evolution is slow because i'm working on other projects during the same time. Your argument with LWJGL is not fair. Ok today you have the OpenVR 1.0.16 binding because LWJGL has just be updated few days ago but in 6 months as the LWJGL distribution that JMonkey use is not updated regularly the LWGJL bindings will be outdated.

My problem is that the main goal for the creation of the jme-vr module was to provide a common interface for JMonkey that can relies on various implementations. This argument is a big sense as it exists at this time various implementations. The fact that i've not be able to update the whole implementations during time is true but it cannot be used to avoid this effort to provide multi implementation capabilities.

@jseinturier
Copy link
Contributor

As i said within PR #927, I reaffirm my dissatisfaction with this choice of REPLACING existing and functional code on which people rely for their development. It is quite possible to leave different rendering systems (OpenVR JNA, LWJGL, OCCULUS) side by side without modifying the central classes of the VR module.

If there is a need to use LWJGL OPENVR binding, just update the com.jme3.system.lwjgl package and no not make board effect by modifying classes that are used by other people.

@neph1
Copy link
Contributor Author

neph1 commented Oct 4, 2018

The evolution is slow because i'm working on other projects during the same time. Your argument with LWJGL is not fair.

That is exactly what I'm talking about. If we rely on one person to update the bindings, what happens when that person is working on other things? By allowing the middleware people doing their thing, we can all work on actually using the things. Wouldn't it be nice if other people did this for you?

I'm curious as to why you feel that lwjgl is slow to update. Looking at their history:
https://github.com/LWJGL/lwjgl3/commits/master/modules/lwjgl/openvr
There are a couple of commits every month.

Also, I'd like to know your opinion on the binaries. Should jme then distribute two openvr lib's? Or none?

@jseinturier
Copy link
Contributor

Hello,

I've created PR #937 trying to solve the situation. I've integrated your changes regarding LWJGL binding of OpenVR but instead of replacing all existing JNA bindings i've made another package that implements VR interfaces but that delegates to LWJGL/OpenVR.

I know your concerns about having 2 different bindings for OpenVR but i think this is actually a good way to go as VR systems are evolving really quick and we have to be able to update VR capabilities even if for example LWJGL is not updated (i know that LWJGL itself is updated regularly but the integration of LWJGL within JMonkey is often delayed).

@jseinturier
Copy link
Contributor

Another question Neph1, what is the purpose of the PR #927 ? They've have changed a lot of things and with their modification nothing is no more working for me...

@stephengold stephengold mentioned this pull request Oct 6, 2018
@neph1
Copy link
Contributor Author

neph1 commented Oct 6, 2018

I don't know anything about #927 other than that they were doing the same thing as I was. So I just hinted they don't need to complete it ;)

@jseinturier
Copy link
Contributor

So Neph1, if you're agree, at this time we can validate this PR (i will change the settings of the VR module in order to use LWJGL/OpenVR implementation by default).

I've now time to work on VR module (i need to prepare a museum exposition based on VR within the domain of underwater exploration). I can propose you to work on the externalization of JOpenVR from JMonkey (for example by creating an "Official" distribution of JOpenVR and by referencing it as an optionnal dependency, as we do for lwjgl-openvr).

If it is ok for you we can use this PR as basis.

@empirephoenix
Copy link
Contributor

Is this still alive? Or should it be closed?

@jseinturier
Copy link
Contributor

It has to be close. Solved within another PR

@stephengold stephengold closed this Feb 8, 2019
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.

4 participants