OSVR / OSVR-Unity

Package for authoring OSVR experiences with Unity.
Apache License 2.0
99 stars 38 forks source link

OnVRSurfaceCreated event for image effects #99

Closed DuFF14 closed 8 years ago

DuFF14 commented 8 years ago

One possible solution for: https://github.com/OSVR/OSVR-Unity/issues/75

This PR adds an event to VREye that fires when a VRSurface is created. Another object in the scene can subscribe to this event, and provide game-specific code in a callback function. This would be for adding image effects and other components that need to be copied to each camera in the scene. Example script and scene added, although there is not an included image effect demonstrated, just comments where that code would go.

ImperialPenguin commented 8 years ago

Regarding image effects, this would require the user to add them manually wouldn't it? Unity doesn't provide an easy way to copy components. I was just sitting down to work on it when I saw this. Components can be copied via reflection, so I figured the easiest way to go about it was to loop through all of the components and copy them that way, excluding the display controller, of course. This would also replace the current camera copy code, since the camera is a component. The current code doesn't seem to copy over my HDR or color settings anyhow. I reckon that is a bug in Unity, since the current camera copy is simply a built in Unity function.

I'm using a version of Unity one patch release newer than the one the project was built with. I don't know what effect that will have on Git, so it may be best for me to make the changes to my main project and then copy them over to my repository.

DuFF14 commented 8 years ago

Yes, this still requires a manual process. Reflection will work but I think @rpavlik had a good reason not to use it in this project. Should be fine in Git if its a Unity 4 project.

ImperialPenguin commented 8 years ago

If avoiding reflection is desired then the other idea I had was to instantiate a copy of VRDisplayTracked, since that it where the camera is and instantiating it would create an exact copy. Of course, that is also where the Display Controller is, and we wouldn't want that running again so it would need to be disable before the instantiation, deleted form the new instance, and then reenabled on VRDisplayTracked proper.

I'm using Unity 5.2.2p1, when I tried to open the project it said it was created with 5.2.2f1. I suspect that those are close enough not to cause issues, and I know it won't if the only things included in the commit are .cs files. I'm not sure how many changes Git will register that I'll have to sort through. I'm gonna give it a try and see though I just haven't had a chance to yet.

ImperialPenguin commented 8 years ago

I've got image effects working using the instantiate method I mentioned above. It is slightly messy because by the time CreateSurface is called there have already been two gameobjects added as children to the tracker object that get duplicated as well (VRViewer and VREye). So those, along with the display controller need to be removed. I don't see a way around this since camera and its image effects need to be duplicated multiple times, so creating it before everything is setup would require passing that object through VRViewer to VREye, duplicating it, and then deleting that original after all the surfaces are created. That would be even messier. Another alternative would be to separate the display controller into its own gameobject and create the camera gameobject as a child of it. VRViewer and VREye would no longer be a child of the camera gameobject that needs to be duplicated. That would take a bit of reworking, would make the hierarchy view messier, and I'm not sure what other ramifications it may have.

There is a bit of a performance hit compared to TrinusVR when the image effects are added. That performance hit exists without them though, so I see no reason I shouldn't submit the changes once we decide the best way. It occurs because the image effects on the main camera are still processed. They don't take as much time as they normally would, but my project loses about 1ms to them. There are other areas where the time is lost rendering that camera as well. I'll have to do a deep profile to figure out what is going on.

DuFF14 commented 8 years ago

Closed. This feature was implemented in #111