@rickard said:
Finally got around to do the refactoring (before i read your EDIT).
You were right, it did not work due to the init method. I added a reshape call though, which fixed that. Now I don't see any different behavior from before. Did some more minor reshuffling of the code in OVRAppState to increase readability, as well.
Is there no way to edit the first post in a thread anymore? I think it’s time to update that 15 months old screenshot (and the text as well).
I’m all for refactoring if it results in more readability, but not at the expense of efficiency. You are initializing the filters in OVRAppState anyway, so might as well do it in the right order to avoid the reshape call. You reintroduced a frustrum culling problem as objects were disappearing in the TestStereoCams demo, which I’ve fixed. I cleaned up a few things to maintain readability.
@phr00t said:
I'm all for refactoring if it results in more readability, but not at the expense of efficiency. You are initializing the filters in OVRAppState anyway, so might as well do it in the right order to avoid the reshape call. You reintroduced a frustrum culling problem as objects were disappearing in the TestStereoCams demo, which I've fixed. I cleaned up a few things to maintain readability.
Sorry about the bug, I didn’t see a difference when removing it ( I guess it was the first setProjectionMatrix or setViewPort?). From an OOP point of view, I still think it makes more sense to have those functions in the Filter, but I won’t argue more about it now. My fear is just that OVRAppState will be cluttered with too much functionality. I guess once the SDK is stable, we can take a look at the architecture of the plugin do a refactoring pass.
I tried something new (which for whatever reason I don’t think I’ve done before). I moved the initialization calls from the static main method to the constructor of the application. This worked well. I also moved the OculusRift.destroy call to the finalize method of the application. This removed the “Invalid memory call” exception.
As I mentioned, this has been my biggest gripe. Not as good as having everything contained within the AppState, but that might not be attainable anyway.
I’ve checked in TestStereoCams with this change. @phr00t, can you verify?
@rickard said:
Sorry about the bug, I didn't see a difference when removing it ( I guess it was the first setProjectionMatrix or setViewPort?). From an OOP point of view, I still think it makes more sense to have those functions in the Filter, but I won't argue more about it now. My fear is just that OVRAppState will be cluttered with too much functionality. I guess once the SDK is stable, we can take a look at the architecture of the plugin do a refactoring pass.
I tried something new (which for whatever reason I don’t think I’ve done before). I moved the initialization calls from the static main method to the constructor of the application. This worked well. I also moved the OculusRift.destroy call to the finalize method of the application. This removed the “Invalid memory call” exception.
As I mentioned, this has been my biggest gripe. Not as good as having everything contained within the AppState, but that might not be attainable anyway.
I’ve checked in TestStereoCams with this change. @phr00t, can you verify?
OVRAppState & the filter work closely together to solve one problem: render the two eyes onto the screen. If a function needs to be in OVRAppState to be called in the correct order, I don’t consider it clutter. We can consider refactoring down the road, but we have to weigh the pros and the cons of doing so. Refactoring can (and has) broken things & doesn’t always result in “better code”. Best to err on the side of keeping code that works, in my humble opinion.
You are still calling the constructor for TestStereoCams in a static context, so moving the OculusRift.initialize() call into the constructor doesn’t really change much, but maybe it satisfies some OOP goal. It requires a bit more work on the end-user though, since now they will need to create a constructor for their SimpleApplication & add the OculusRift calls in it, as oppose to just calling it in the static main().
Glad you found a fix for the “invalid memory call” exception, although I wish there was a way to solve that without asking end-users to add the destroy method to their application. Is there a cleanup on the filter or some other place it could be added? I don’t think I see that exception, but maybe because I don’t have an Oculus Rift hooked up?
@phr00t said:
You are still calling the constructor for TestStereoCams in a static context, so moving the OculusRift.initialize() call into the constructor doesn't really change much, but maybe it satisfies some OOP goal. It requires a bit more work on the end-user though, since now they will need to create a constructor for their SimpleApplication & add the OculusRift calls in it, as oppose to just calling it in the static main().
Glad you found a fix for the “invalid memory call” exception, although I wish there was a way to solve that without asking end-users to add the destroy method to their application. Is there a cleanup on the filter or some other place it could be added? I don’t think I see that exception, but maybe because I don’t have an Oculus Rift hooked up?
I actually got it without the rift connected, too.
I added a new class called OVRApplication which extends SimpleApplication and does all the OVR specific stuff. I had hoped that using the plugin would be as simple as attaching an AppState, but since we have the initialization that must happen and the gui node, that will most likely not happen. The OVRGuiNode is not in the OVRApplication, yet, though.
Also added a test case which is basically TestStereoCams but with OVRApplication instead.
Edit: it was actually the finalize method it had to be implemented in, not destroy
I don’t get the error, so I’m not sure if the above would help. If so, we could put it back into the cleanup() of OVRAppState which would simplify things for developers.
@rickard said:
I actually got it without the rift connected, too.
I added a new class called OVRApplication which extends SimpleApplication and does all the OVR specific stuff. I had hoped that using the plugin would be as simple as attaching an AppState, but since we have the initialization that must happen and the gui node, that will most likely not happen. The OVRGuiNode is not in the OVRApplication, yet, though.
Also added a test case which is basically TestStereoCams but with OVRApplication instead.
Creating an OVRApplication that does the construction and finalization is a great idea! We can push the guiNode stuff to it too, or I can do that when you check in those changes.
I don’t get the error, so I’m not sure if the above would help. If so, we could put it back into the cleanup() of OVRAppState which would simplify things for developers.
It’s the finalize method that is the key. Tested placing it in OVRAppState instead, and it worked fine. Will check in an update.
I noticed that TestOVRApplication got an NPE due to OculusRift.isInitialized being false, so the appstate is never created.
I don’t have the OVR plugged in right now, so it fails to initialize. forceInitializeSuccess is not called automatically in this case anymore, which I think is the reason. Should we add it again to the normal flow?
@rickard said:
Removed TestScene from the test cases.
I noticed that TestOVRApplication got an NPE due to OculusRift.isInitialized being false, so the appstate is never created.
I don’t have the OVR plugged in right now, so it fails to initialize. forceInitializeSuccess is not called automatically in this case anymore, which I think is the reason. Should we add it again to the normal flow?
Thanks.
I added a configureOVRApp(…) call that will force the Oculus Rift to always act plugged-in in TestOVRApplication. I don’t think forceInitializeSuccess should be automatically called, because then developers won’t know if an Oculus Rift is really plugged in or not. The original problem with that NPE in TestOVRApplication was assuming the Oculus Rift was connected & initialized when nothing was plugged in. The solution is to either always initialize it anyway (which it is doing now & good for a test case) or have an error saying “Oculus Rift not plugged in!” (which would be good for a production environment).
I added a configureOVRApp(…) call that will force the Oculus Rift to always act plugged-in in TestOVRApplication. I don’t think forceInitializeSuccess should be automatically called, because then developers won’t know if an Oculus Rift is really plugged in or not. The original problem with that NPE in TestOVRApplication was assuming the Oculus Rift was connected & initialized when nothing was plugged in. The solution is to either always initialize it anyway (which it is doing now & good for a test case) or have an error saying “Oculus Rift not plugged in!” (which would be good for a production environment).
Yeah, it just struck me that I might just as well call that method during init
I will commit the change
The DK1 always worked as a second monitor in “Extended” mode. The new Oculus runtime & DK2 supports something called “Direct to Rift”, where the DK2 isn’t configured as another monitor. Instead, the application opens and renders directly to the Rift. This makes it easier to use the Rift without having to worry about refresh rates, extending or cloning monitors in your operating system.
@phr00t said:
The DK1 always worked as a second monitor in "Extended" mode. The new Oculus runtime & DK2 supports something called "Direct to Rift", where the DK2 isn't configured as another monitor. Instead, the application opens and renders directly to the Rift. This makes it easier to use the Rift without having to worry about refresh rates, extending or cloning monitors in your operating system.
Removing Chromatic or Vignette from this int should technically remove the feature from the distortion. I’ve seen it done in realtime in Minecrift & Quake 2 VR, somehow. There is a known bug though, which @jherico reported that just removing Vignette doesn’t actually remove the Vignette… I think you had to remove both Chromatic & Vignette to see the Vignette disappear. Not sure if that has been fixed in the official SDK.
JOVR is updated to the latest SDK. I just haven’t integrated it into our library yet because there is no Linux SDK available yet. My development machine is a Linux machine, so I wouldn’t be able to test anything.
I’m hoping the next SDK release will have a Linux version available.
Oh, I got my DK2 yesterday! Only had enough time to try out the desk scene & the world demo. Still getting my VR legs, but was incredibly impressed with the effect, although the screen door & chromatic aberrations leaves much to be desired. Resolution & tracking was very good though, in my opinion.
I’ve started with the 0.4 implementation, but so far haven’t been able to run it. For some reason I’m having some troubles detecting my DK1.
The new API will also require some architectural changes, at least in the long run, as the endFrame call now requires both eyeTextures.
