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

Add Windows Mixed Reality support #85

Merged
merged 5 commits into from Aug 21, 2018
Merged

Add Windows Mixed Reality support #85

merged 5 commits into from Aug 21, 2018

Conversation

Aminator
Copy link
Contributor

@Aminator Aminator commented Aug 9, 2018

In addition to Oculus and OpenVR I added Windows Mixed Reality support for UWP. This brings not only support for WMR headsets (which were already supported thanks to SteamVR) but also HoloLens, although testing for the latter would be needed. As the WMR motion controllers have a thumbstick and a touchpad, I made it available to differentiate between the two while they still map to the same on the other platforms.

To use Windows Mixed Reality you just add the API to the list of required APIs and preferably set it at the top of the list so that it won't use OpenVR which would result in an exception. Next you need to start the game from a CoreWindow context and set the optional parameter "isWindowsMixedReality" to true or else it uses the swap chain of the CoreWindow instead of the holographic space. Be sure to not activate the CoreWindow as this must happen after creating the holographic space which is done internally.

Current issues:

  • No post-processing
  • No MSAA
  • Controller movement is still a bit finicky especially when throwing things.
  • Just-In-Time debugging breaks when the headset looses tracking (just turn it off).

I hope this addition will lead to more titles produced with WMR support and as always please test it if you have a WMR device (or emulate it with the Windows Mixed Reality Portal app).

Copy link
Member

@Kryptos-FR Kryptos-FR left a comment

Choose a reason for hiding this comment

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

I added a few (personal) thoughts. I'll let @xen2 review it properly as I'm no expert in that area.

/// </summary>
/// <param name="value">The value.</param>
/// <returns>The result of the conversion.</returns>
public static implicit operator Matrix(System.Numerics.Matrix4x4 value)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of having the conversion implicit in both way. Usually one is implicit and the other explicit.

@xen2 What is the impact of having dependency to System.Numerics? Shouldn't that be handled more generally (including testing) in a separate issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Kryptos-FR As Windows.Perception.Spatial uses System.Numerics types I needed to add these implicit conversions to reduce bloat. There should be nothing wrong with making the conversion implicit both ways as both use floats. I mean, even the existing conversions are implicit both ways if both types use the same underlying primitive types.

Copy link
Member

@xen2 xen2 Aug 9, 2018

Choose a reason for hiding this comment

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

@Kryptos-FR It seems like System.Numerics is already an implicit dependency (quite surprised, not sure where it comes from, seems automatic for Windows and UWP platforms!)
@Aminator9000 Not a big fan of having things halfway (conversion only on UWP).
So either we:

  • Add System.Numerics explicitly (NuGet version) and this conversion for all platforms. It implies you are able to tests those platforms (at least compilation with the Xenko.*.sln)
  • You move this System.Numerics implicit conversion temporarily where you need it as an explicit method (and we can always add it back to Mathematics later for everybody)

/// </summary>
/// <param name="value">The value.</param>
/// <returns>The result of the conversion.</returns>
public static implicit operator Quaternion(System.Numerics.Quaternion value)
Copy link
Member

Choose a reason for hiding this comment

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

Same remark about both-way implicit conversion.

/// </summary>
/// <param name="value">The value.</param>
/// <returns>The result of the conversion.</returns>
public static implicit operator Vector3(System.Numerics.Vector3 value)
Copy link
Member

Choose a reason for hiding this comment

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

Same remark about both-way implicit conversion.

@@ -116,7 +116,7 @@ public partial class ForwardRenderer : SceneRendererBase, ISharedRenderer
[DefaultValue(true)]
public bool BindDepthAsResourceDuringTransparentRendering { get; set; } = true;

private Logger logger { get; } = GlobalLogger.GetLogger(nameof(ForwardRenderer));
private Logger Logger { get; } = GlobalLogger.GetLogger(nameof(ForwardRenderer));
Copy link
Member

Choose a reason for hiding this comment

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

I was about to say that private fields are always lowercase. But then I realized it is actually a private property.

I don't see the point of having a private property here. I think it should be converted to a private readonly field.


namespace Xenko.Graphics
{
public class WindowsMixedRealityGraphicsPresenter : GraphicsPresenter
Copy link
Member

Choose a reason for hiding this comment

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

Having that class in Xenko.Graphics forces to have dependency on Windows.Graphics.Holographic for all UWP projects (even those that are not targeting such device. I think we should come up with a better design.

Copy link
Member

@xen2 xen2 Aug 9, 2018

Choose a reason for hiding this comment

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

Yes and ideally it should all be doable as a plugin, but there is unfortunately quite a bunch of non-trivial extensions point to add, could be a lot of work (it affects renderer code and game creation).
Maybe we'll have to keep it together for now (just like we do for all platforms currently) and consider splitting it later down the road.

}

public bool IsWindowMixedReality { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Abstraction leak?

@HeadClot
Copy link

HeadClot commented Aug 9, 2018

Will start testing this. Good work :)

backBuffer = new Texture(GraphicsDevice).InitializeFromImpl(d3DBackBuffer, false);

LeftEyeBuffer = backBuffer.ToTextureView(new TextureViewDescription() { ArraySlice = 0, Type = ViewType.Single });
RightEyeBuffer = backBuffer.ToTextureView(new TextureViewDescription() { ArraySlice = 1, Type = ViewType.Single });
Copy link
Member

Choose a reason for hiding this comment

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

Those two buffers should be disposed if they exist before being recreated.

@@ -0,0 +1,202 @@
// Copyright (c) Xenko contributors (https://xenko.com) and Silicon Studio Corp. (https://www.siliconstudio.co.jp)
Copy link
Member

Choose a reason for hiding this comment

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

New files should only have Copyright for Xenko contributors, not Silicon Studio anymore

Copy link
Member

Choose a reason for hiding this comment

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

@xen2 Let's add something in the doc/readme or somewhere else (wiki?) to precise a template for new files.
I think we could also port the "coding guidelines" we had in confluence and ad it to the wiki, to ensure a uniform coding style and naming conventions.

Copy link
Member

@xen2 xen2 Aug 9, 2018

Choose a reason for hiding this comment

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

@Kryptos-FR Yes we should add that. The new version (with Xenko contributors only) was already in DotSettings, but

  • not everybody has resharper
  • people might edit files through another solution (we have a DotSettings only for Xenko.sln but maybe it was done through Xenko.UWP.sln in this case)

@@ -0,0 +1,172 @@
// Copyright (c) Xenko contributors (https://xenko.com) and Silicon Studio Corp. (https://www.siliconstudio.co.jp)
Copy link
Member

Choose a reason for hiding this comment

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

New files should only have Copyright for Xenko contributors, not Silicon Studio anymore

/// </summary>
/// <param name="value">The value.</param>
/// <returns>The result of the conversion.</returns>
public static implicit operator Matrix(System.Numerics.Matrix4x4 value)
Copy link
Member

@xen2 xen2 Aug 9, 2018

Choose a reason for hiding this comment

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

@Kryptos-FR It seems like System.Numerics is already an implicit dependency (quite surprised, not sure where it comes from, seems automatic for Windows and UWP platforms!)
@Aminator9000 Not a big fan of having things halfway (conversion only on UWP).
So either we:

  • Add System.Numerics explicitly (NuGet version) and this conversion for all platforms. It implies you are able to tests those platforms (at least compilation with the Xenko.*.sln)
  • You move this System.Numerics implicit conversion temporarily where you need it as an explicit method (and we can always add it back to Mathematics later for everybody)

@xen2
Copy link
Member

xen2 commented Aug 13, 2018

@Aminator9000 Please let me know when this is ready for final review/merge (if not already).

@Aminator
Copy link
Contributor Author

@xen2 It is ready now although the above mentioned issues are still there. Those could be fixed later as I am currently not sure how to fix them.

@xen2
Copy link
Member

xen2 commented Aug 21, 2018

Sorry for the late review, LGTM!
Thanks!

@xen2 xen2 merged commit 9f4f5b4 into stride3d:master Aug 21, 2018
@yar3333
Copy link

yar3333 commented Nov 15, 2018

Please, help me to create a minimal WMR application. I do all what neccessary after reading all about VR in docs. I create Game from template with UWP support, remove XAML Page component and edit App.OnLaunched code to this:

protected override void OnLaunched(LaunchActivatedEventArgs e)
{
	Game = new Game();
	Game.UnhandledException += Game_UnhandledException;
	Game.Run(new GameContextUWPCoreWindow(null, 0, 0, true));
}

WMR portal started, VR controllers visible in simulator, but all around is black. What I do wrong?

@yar3333
Copy link

yar3333 commented Nov 15, 2018

I found solution - *.t4 in Xenko sources contain code to create FrameworkView. No action needed :)

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.

None yet

5 participants