1260530 - [webvr] Add support for Oculus 1.3 runtime.

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Casey Yee]

Oculus Rift consumer products are now shipping with version 1.3 runtime.

Should work when "allow from external sources" option is checked.

Currently, the navigator.getVRDevices does not resolve the promise.

Comment 1

[:kip (Kearwood Gilbert)]

I have taken this bug -- it is now my first priority.

Comment 2

[:kip (Kearwood Gilbert)]

Attachment: MozReview Request: Bug 1260530 - Add support for Oculus 1.3 runtime

Review commit: https://reviewboard.mozilla.org/r/43343/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43343/

Comment 3

[:kip (Kearwood Gilbert)]

Comment on attachment 8736521 [details]
MozReview Request: Bug 1260530 - Add support for Oculus 1.3 runtime

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43343/diff/1-2/

Comment 4

[:kip (Kearwood Gilbert)]

I have adjusted the order of initialization in a constructor to avoid a warning that broke the non-windows builds.  The try push has been re-submitted

Comment 5

[:kip (Kearwood Gilbert)]

The try run has passed

Comment 6

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8736521 [details]
MozReview Request: Bug 1260530 - Add support for Oculus 1.3 runtime

https://reviewboard.mozilla.org/r/43343/#review40287

Nice work! Just a little bit need to be modified.

::: gfx/vr/gfxVROculus.cpp:34
(Diff revision 2)
>  using namespace mozilla::gfx::impl;
>  
>  namespace {
>  
>  #ifdef OVR_CAPI_LIMITED_MOZILLA
> +

Redundant new line

::: gfx/vr/gfxVROculus.cpp:78
(Diff revision 2)
> +static pfn_ovr_SetFloatArray ovr_SetFloatArray = nullptr;
> +static pfn_ovr_GetString ovr_GetString = nullptr;
> +static pfn_ovr_SetString ovr_SetString = nullptr;
>  
>  #ifdef XP_WIN
> -static pfn_ovr_CreateSwapTextureSetD3D11 ovr_CreateSwapTextureSetD3D11 = nullptr;
> +

Redundant new line

::: gfx/vr/gfxVROculus.cpp:83
(Diff revision 2)
> -static pfn_ovr_CreateSwapTextureSetD3D11 ovr_CreateSwapTextureSetD3D11 = nullptr;
> +
> +static pfn_ovr_CreateTextureSwapChainDX ovr_CreateTextureSwapChainDX = nullptr;
> +static pfn_ovr_GetTextureSwapChainBufferDX ovr_GetTextureSwapChainBufferDX = nullptr;
> +static pfn_ovr_CreateMirrorTextureDX ovr_CreateMirrorTextureDX = nullptr;
> +static pfn_ovr_GetMirrorTextureBufferDX ovr_GetMirrorTextureBufferDX = nullptr;
> +

Redundant new line

::: gfx/vr/gfxVROculus.cpp:364
(Diff revision 2)
>      mDeviceInfo.mEyeFOV[eye] = eye == 0 ? aFOVLeft : aFOVRight;
>      mFOVPort[eye] = ToFovPort(mDeviceInfo.mEyeFOV[eye]);
>  
>      ovrEyeRenderDesc renderDesc = ovr_GetRenderDesc(mSession, (ovrEyeType)eye, mFOVPort[eye]);
>  
>      // As of Oculus 0.6.0, the HmdToEyeViewOffset values are correct and don't need to be negated.

We need to modify the comment to HmdToEyeOffset as well.

::: gfx/vr/gfxVROculus.cpp:498
(Diff revision 2)
>    
>    already_AddRefed<layers::CompositingRenderTarget> GetNextRenderTarget() override {
> -    currentRenderTarget = (currentRenderTarget + 1) % renderTargets.Length();
> -    textureSet->CurrentIndex = currentRenderTarget;
> +    int currentRenderTarget = 0;
> +    ovrResult orv = ovr_GetTextureSwapChainCurrentIndex(session, textureSet, &currentRenderTarget);
> +    if (orv != ovrSuccess) {
> +      // failed?

We should add some logs here for tracking potential bugs.

::: gfx/vr/gfxVROculus.cpp:545
(Diff revision 2)
>      compositor = aCompositor;
>      
> -    renderTargets.SetLength(aTS->TextureCount);
> -    
> -    currentRenderTarget = aTS->CurrentIndex;
> +    int textureCount = 0;
> +    ovrResult orv = ovr_GetTextureSwapChainLength(session, aTS, &textureCount);
> +    if (orv != ovrSuccess) {
> +      // failed?

Same here. We should add some logs

::: gfx/vr/gfxVROculus.cpp:557
(Diff revision 2)
> -      tex11 = (ovrD3D11Texture*)&aTS->Textures[i];
> -      rt = new layers::CompositingRenderTargetD3D11(tex11->D3D11.pTexture, IntPoint(0, 0), DXGI_FORMAT_B8G8R8A8_UNORM);
> +      RefPtr<layers::CompositingRenderTargetD3D11> rt;
> +
> +      ID3D11Texture2D* texture = nullptr;
> +      orv = ovr_GetTextureSwapChainBufferDX(session, aTS, i, IID_PPV_ARGS(&texture));
> +      if (orv != ovrSuccess) {
> +        // failed?

Same here. Should add some logs

::: gfx/vr/gfxVROculus.cpp:628
(Diff revision 2)
>      // disabled.
>      aInputFrameID = 0;
>    }
> +  ovrResult orv = ovr_CommitTextureSwapChain(mSession, rts->textureSet);
> +  if (orv != ovrSuccess) {
> +    // failed?

Same here. Add some logs

::: gfx/vr/gfxVROculus.cpp:681
(Diff revision 2)
>  
>    ovrLayerHeader *layers = &layer.Header;
> -  ovrResult orv = ovr_SubmitFrame(mSession, aInputFrameID, nullptr, &layers, 1);
> +  orv = ovr_SubmitFrame(mSession, aInputFrameID, nullptr, &layers, 1);
>    //printf_stderr("Submitted frame %d, result: %d\n", rts->textureSet->CurrentIndex, orv);
>    if (orv != ovrSuccess) {
>      // not visible? failed?

Same here. Add some logs

::: gfx/vr/ovr_capi_dynamic.h:245
(Diff revision 2)
> +typedef enum {
> +  ovrTextureBind_None,
> +  ovrTextureBind_DX_RenderTarget = 0x0001,
> +  ovrTextureBind_DX_UnorderedAccess = 0x0002,
> +  ovrTextureBind_DX_DepthStencil = 0x0004,
> +

redundant new line

::: gfx/vr/ovr_capi_dynamic.h:265
(Diff revision 2)
> +  OVR_FORMAT_R16G16B16A16_FLOAT,
> +  OVR_FORMAT_D16_UNORM,
> +  OVR_FORMAT_D24_UNORM_S8_UINT,
> +  OVR_FORMAT_D32_FLOAT,
> +  OVR_FORMAT_D32_FLOAT_S8X24_UINT,
> +

redundant new line

::: gfx/vr/ovr_capi_dynamic.h:462
(Diff revision 2)
>      unsigned        Flags;
>  } ovrLayerHeader;
>  
>  typedef struct OVR_ALIGNAS(OVR_PTR_SIZE) {
> -    ovrLayerHeader      Header;
> -    ovrSwapTextureSet*  ColorTexture[ovrEye_Count];
> +    ovrLayerHeader Header;
> +	ovrTextureSwapChain ColorTexture[ovrEye_Count];

Please correct this indentation.

Comment 7

[:kip (Kearwood Gilbert)]

Comment on attachment 8736521 [details]
MozReview Request: Bug 1260530 - Add support for Oculus 1.3 runtime

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43343/diff/2-3/

Comment 8

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8736521 [details]
MozReview Request: Bug 1260530 - Add support for Oculus 1.3 runtime

https://reviewboard.mozilla.org/r/43343/#review41211

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e41f1773b78

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7e41f1773b78

Comment 11

[Cornel Ionce [:noni] [Hubs QA]]

I'm unable to test this due to my current GPU limitation. I get the "Update your graphics card driver" message for the 1.3 runtime.

Naoki, could you please confirm this works as expected so we can close it?