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.

Sign up for GitHub

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

Jump to bottom

Force the distortion correction if RenderManager is not available #216

Closed
wants to merge 2 commits into from
Closed

Force the distortion correction if RenderManager is not available #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d9bf8b
Added distortion correction when RenderMananger is not available
demonixis Apr 1, 2017
4c26519
Applying the distortion on VREye
demonixis Apr 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion OSVR-Unity/Assets/OSVRUnity/src/DisplayController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ namespace Unity
//*/
public class DisplayController : MonoBehaviour
{

public const uint NUM_VIEWERS = 1;
private const int TARGET_FRAME_RATE = 60; //@todo get from OSVR

Expand Down
47 changes: 30 additions & 17 deletions OSVR-Unity/Assets/OSVRUnity/src/VREye.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ public class VREye : MonoBehaviour
private VRSurface[] _surfaces; //the surfaces associated with this eye
private uint _surfaceCount;
private uint _eyeIndex;


#endregion
#region Public Variables
public uint EyeIndex
{
get { return _eyeIndex; }
set { _eyeIndex = value; }
}
public VRSurface[] Surfaces { get { return _surfaces; } }
public VRSurface[] Surfaces { get { return _surfaces; } }
public uint SurfaceCount { get { return _surfaceCount; } }
public VRViewer Viewer
{
Expand All @@ -75,7 +75,7 @@ void Init()
{
//cache:
cachedTransform = transform;
}
}
#endregion

// Updates the position and rotation of the eye
Expand Down Expand Up @@ -122,7 +122,7 @@ public void UpdateSurfaces()

//get projection matrix from RenderManager and set surface projection matrix
surface.SetProjectionMatrix(Viewer.DisplayController.RenderManager.GetEyeProjectionMatrix((int)EyeIndex));

surface.Render();
}
else
Expand All @@ -145,7 +145,7 @@ public void UpdateSurfaces()

//render the surface
surface.Render();
}
}

}
}
Expand Down Expand Up @@ -209,7 +209,7 @@ public void CreateSurfaces(uint surfaceCount)
//get distortion parameters
OSVR.ClientKit.RadialDistortionParameters distortionParameters =
Viewer.DisplayController.DisplayConfig.GetViewerEyeSurfaceRadialDistortion(
Viewer.ViewerIndex, (byte)_eyeIndex, surfaceIndex);
Viewer.ViewerIndex, (byte)_eyeIndex, surfaceIndex);
surface.SetDistortion(distortionParameters);
}

Expand All @@ -229,6 +229,8 @@ public void CreateSurfaces(uint surfaceCount)
}
}

var displayController = FindObjectOfType<DisplayController>();
Copy link
Member

@DuFF14 DuFF14 Apr 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Viewer.DisplayController here instead of creating a new local var

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll update my PR



//loop through surfaces because at some point we could support eyes with multiple surfaces
//but this implementation currently supports exactly one
Expand All @@ -247,20 +249,31 @@ public void CreateSurfaces(uint surfaceCount)

//distortion
bool useDistortion = Viewer.DisplayController.DisplayConfig.DoesViewerEyeSurfaceWantDistortion(Viewer.ViewerIndex, (byte)_eyeIndex, surfaceIndex);
if(useDistortion)
useDistortion |= !displayController.UseRenderManager;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also want to apply this change here, right? https://github.com/OSVR/OSVR-Unity/blob/master/OSVR-Unity/Assets/OSVRUnity/src/VREye.cs#L204

When I debug the OSVRDemo2 scene, it doesn't reach this code.


if (useDistortion)
{
//@todo figure out which type of distortion to use
//right now, there is only one option, SurfaceRadialDistortion
//get distortion parameters
OSVR.ClientKit.RadialDistortionParameters distortionParameters =
Viewer.DisplayController.DisplayConfig.GetViewerEyeSurfaceRadialDistortion(
Viewer.ViewerIndex, (byte)_eyeIndex, surfaceIndex);
OSVR.ClientKit.RadialDistortionParameters distortionParameters;

try
{
distortionParameters = Viewer.DisplayController.DisplayConfig.GetViewerEyeSurfaceRadialDistortion(Viewer.ViewerIndex, (byte)_eyeIndex, surfaceIndex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this call always fails? So it will always use the hardcoded params?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It always fail on my three PC with various HDKs yes. So the hardcoded values are here just in case. I know it's not the best way but it allow Linux and Mac users to have a better quality.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which HDKs have you tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both HDK 1.3/1.4 using various firmwares. My last test was using the latest firmware.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a 1.3 or 1.4 to test on, but it does not look good on HDK2. I can't really tell if I'd call it better or worse than the default without testing some other scenes. Using default values might work for one set of lenses, but it's not a good generalized solution. HDK2 and other HMDs do not have radially distorted lenses, so that distortion effect isn't accurate.

However, if it does look better than the default for 1.3 and 1.4 users, I'd be ok with merging it in given that most users are using the RenderManager path anyway. Porting RenderManager-Unity to OSX/Linux is still the best solution, as you've pointed out in another thread.

@rpavlik any idea why GetViewerEyeSurfaceRadialDistortion fails? Does it depend on RM?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the moment, I would not recommend using the DisplayConfig API - I don't know if it's up to date with how the RenderManager works - if I recall correctly, the code that parses the display config and render manager configs is currently forked in RenderManager and OSVR-Core. Eventually that needs to be moved to OSVR-Core and removed from RenderManager (so that RM calls into OSVR-Core), but in the meantime look out for issues with the OSVR-Core DisplayConfig.

Copy link
Member

@DuFF14 DuFF14 Apr 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetViewerEyeSurfaceRadialDistortion is looking for radial distortion coefficients, which only some of the display descriptors use (depending on whether or not the lenses are radially distorted). HDK1.3/1.4 use polynomial distortion or a mesh. HDK2 uses a mesh.

bool useDistortion = Viewer.DisplayController.DisplayConfig.DoesViewerEyeSurfaceWantDistortion(Viewer.ViewerIndex, (byte)_eyeIndex, surfaceIndex);
useDistortion |= !displayController.UseRenderManager;

Does DoesViewerEyeSurfaceWantDistortion return false? According to the comments it could still return true if any type of distortion is used in the display descriptor, but it doesn't mean that radial distortion coefficients (k1,k2,k3) are available. The call to GetViewerEyeSurfaceRadialDistortion won't work if those coefficients are not in the display descriptor, so I don't think anything is broken there.

Can you try using a display descriptor that uses those same hard-coded values for k1,k2,k3 and see if you get the same results? Here is an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it returns false.. There is maybe another way to do that, like OSVR-MonoGame by example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, it shouldn't crash Unity even if the distortion is not available.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, check if GetViewerEyeSurfaceRadialDistortion fails even when those distortion coefficients are there. We should be able to get those coefficients from Core when RM is not being used.

}
catch (Exception ex)
{
// The previous call fails, so we use the minimal distortion parameters to ensure a better experience.
distortionParameters.centerOfProjection = new OSVR.ClientKit.Vec2() { x = 0.5, y = 0.5 };
distortionParameters.k1 = new OSVR.ClientKit.Vec3() { x = 0.25, y = 0.25, z = 0.25 };
}

surface.SetDistortion(distortionParameters);
}
}

//render manager
if(Viewer.DisplayController.UseRenderManager)
if (Viewer.DisplayController.UseRenderManager)
{
//Set the surfaces viewport from RenderManager
surface.SetViewport(Viewer.DisplayController.RenderManager.GetEyeViewport((int)EyeIndex));
Expand All @@ -271,8 +284,8 @@ public void CreateSurfaces(uint surfaceCount)
{
renderTexture.antiAliasing = QualitySettings.antiAliasing;
}
surface.SetRenderTexture(renderTexture);
}
surface.SetRenderTexture(renderTexture);
}
}
}

Expand All @@ -284,7 +297,7 @@ private void CopyCamera(Camera srcCamera, Camera destCamera)
destCamera.CopyFrom(srcCamera);
destCamera.depth = 0;
//@todo Copy other components attached to the DisplayController?
}
}
}
}
}