Closed Bug 1343730 Opened 7 years ago Closed 7 years ago

[webvr] VRDisplayPuppet submitFrame support

Categories

(Core :: WebVR, enhancement)

Product:
Core
Component:
WebVR
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: daoshengmu, Assigned: daoshengmu)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(3 files)

the frame from gfxVRPuppet at the top-left corner
6.02 KB, image/png
Details
Bug 1343730 - Part 1: Support submitFrame and encode the frame as a base64 image in VRPuppet;
59 bytes, text/x-review-board-request
kip
: review+
Details
Bug 1343730 - Part 2: Get the submitframe result from VRDisplay;
59 bytes, text/x-review-board-request
kip
: review+
baku
: review+
Details 
As Bug 1229480 Comment 10, we need to make submitFrame in VRDisplayPuppet not only block until finishing vblank but also capture frames for use in reftests.
Blocks: 1229480
Whiteboard: [gfx-noted]
Component: Graphics → WebVR
Assignee: nobody → dmu
After some experiment, I can draw the shared texture that is from WebGL canvas at gfxVRPuppet. Please refer attachment 8865386 [details] at Bug 1229481, we can see the green rect is rendered at gfxVRPuppet::SubmitFrame(), and the yellow rect is rendered by WebGL context before calling VRDisplay::RequestPresent().

Apparently, there are still a few things need to do.

1. Transform the screenQuad that is used for rendering the TextureSourceD3D11 to the WebGL canvas layer position.

2. The yellow rect always covers the gfxVRPuppet frame result. It is probably the render order from the compositor is always after gfxVRPuppet::SubmitFrame().
(In reply to Daosheng Mu[:daoshengmu] from comment #1)
> Created attachment 8865391 [details]
> the frame from gfxVRPuppet at the top-left corner
> 
> After some experiment, I can draw the shared texture that is from WebGL
> canvas at gfxVRPuppet. Please refer attachment 8865386 [details] at Bug
> 1229481, we can see the green rect is rendered at
> gfxVRPuppet::SubmitFrame(), and the yellow rect is rendered by WebGL context
> before calling VRDisplay::RequestPresent().
> 
> Apparently, there are still a few things need to do.
> 
> 1. Transform the screenQuad that is used for rendering the
> TextureSourceD3D11 to the WebGL canvas layer position.
> 
After applying the effectiveTransform from ClientCanvasLayer to the viewport at gfxVRPuppet::SubmitFrame(), it can be showed at the correct position as the WebGL canvas. But, the transformation info is from the container layer, it seems to be not available to get the layer info from this WebGL canvas element.


> 2. The yellow rect always covers the gfxVRPuppet frame result. It is
> probably the render order from the compositor is always after
> gfxVRPuppet::SubmitFrame().

It seems to be a good idea to make "webglCanvas.style.visibility = 'hidden';" to avoid the WebGL canvas element be showed at the top of TextureSourceD3D11.
(In reply to Daosheng Mu[:daoshengmu] from comment #1)
> Created attachment 8865391 [details]
> the frame from gfxVRPuppet at the top-left corner
> 
> After some experiment, I can draw the shared texture that is from WebGL
> canvas at gfxVRPuppet. Please refer attachment 8865386 [details] at Bug
> 1229481, we can see the green rect is rendered at
> gfxVRPuppet::SubmitFrame(), and the yellow rect is rendered by WebGL context
> before calling VRDisplay::RequestPresent().
> 
> Apparently, there are still a few things need to do.
> 
> 1. Transform the screenQuad that is used for rendering the
> TextureSourceD3D11 to the WebGL canvas layer position.
> 
> 2. The yellow rect always covers the gfxVRPuppet frame result. It is
> probably the render order from the compositor is always after
> gfxVRPuppet::SubmitFrame().

One option if this makes your work easier:

It does not matter if we display the submitted VR frames on the monitor, as long as they are compared during the reftest.  If we output base64 encoded images for the VR frames and their delta, then it should be enough to help with debugging failed tests.
(In reply to :kip (Kearwood Gilbert) from comment #4)
> 
> One option if this makes your work easier:
> 
> It does not matter if we display the submitted VR frames on the monitor, as
> long as they are compared during the reftest.  If we output base64 encoded
> images for the VR frames and their delta, then it should be enough to help
> with debugging failed tests.

It sounds like I can encode this ID3D11Texture2D to a base64 format image and dump them as a file to let reftest.html to load this image src.
(In reply to Daosheng Mu[:daoshengmu] from comment #6)
> Comment on attachment 8865392 [details]
> Bug 1343730 - (WIP) VRDisplayPuppet submitFrame support
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/137070/diff/1-2/

In this update, I encode the BGRA8 raw image data from ID3D11Texture2D to a base64 format string and send it to the content process. Then, this base64 format string will be decoded and converted to a PNG base64 format.

The reason I don't send it as a PNG base64 image from the GPU process is we can't have the privilege for creating the image library to help us convert a BGRA image to a PNG image at gfxUtils::GetAsDataURI().

In this approach, developers can get a base64 PNG image at VRDisplay::GetSubmitFrameResult(), and we can let this image to be the source of ImageElement to help us do reftests.

Besides, I add a "dom.vr.puppet.submitframe" preference to for users to decide "0: don't show", "1: save as a base64 png format string", and "2: display it on the screen."
(In reply to Daosheng Mu[:daoshengmu] from comment #8)
> Comment on attachment 8865392 [details]
> Bug 1343730 - (WIP) VRDisplayPuppet submitFrame support
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/137070/diff/2-3/

I define a VRSubmitFrameResultInfo class to help us record the VRSubmitFrame result and send it to the content process instead of some hard-coded work.
Comment on attachment 8865392 [details]
Bug 1343730 - Part 1: Support submitFrame and encode the frame as a base64 image in VRPuppet;

https://reviewboard.mozilla.org/r/137070/#review145652

r=me with the nits marked in the patch fixed (replace "oculus" with "puppet" in messages and remove extra blank line)

Looks good, thanks!

::: gfx/vr/gfxVRPuppet.cpp:184
(Diff revision 4)
> +
> +#if defined(XP_WIN)
> +  if (!mDevice) {
> +    mDevice = gfx::DeviceManagerDx::Get()->GetCompositorDevice();
> +    if (!mDevice) {
> +      NS_WARNING("Failed to get a D3D11Device for Oculus");

The NS_WARNING text should say "Puppet" rather than "Oculus"

::: gfx/vr/gfxVRPuppet.cpp:191
(Diff revision 4)
> +    }
> +  }
> +
> +  mDevice->GetImmediateContext(getter_AddRefs(mContext));
> +  if (!mContext) {
> +    NS_WARNING("Failed to get immediate context for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:197
(Diff revision 4)
> +    return;
> +  }
> +
> +  if (FAILED(mDevice->CreateVertexShader(sLayerQuadVS.mData,
> +                      sLayerQuadVS.mLength, nullptr, &mQuadVS))) {
> +    NS_WARNING("Failed to create vertex shader for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:203
(Diff revision 4)
> +    return;
> +  }
> +
> +  if (FAILED(mDevice->CreatePixelShader(sRGBShader.mData,
> +                      sRGBShader.mLength, nullptr, &mQuadPS))) {
> +    NS_WARNING("Failed to create pixel shader for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:213
(Diff revision 4)
> +                                 D3D11_BIND_CONSTANT_BUFFER,
> +                                 D3D11_USAGE_DYNAMIC,
> +                                 D3D11_CPU_ACCESS_WRITE);
> +
> +  if (FAILED(mDevice->CreateBuffer(&cBufferDesc, nullptr, getter_AddRefs(mVSConstantBuffer)))) {
> +    NS_WARNING("Failed to vertex shader constant buffer for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:219
(Diff revision 4)
> +    return;
> +  }
> +
> +  cBufferDesc.ByteWidth = sizeof(layers::PixelShaderConstants);
> +  if (FAILED(mDevice->CreateBuffer(&cBufferDesc, nullptr, getter_AddRefs(mPSConstantBuffer)))) {
> +    NS_WARNING("Failed to pixel shader constant buffer for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:225
(Diff revision 4)
> +    return;
> +  }
> +
> +  CD3D11_SAMPLER_DESC samplerDesc(D3D11_DEFAULT);
> +  if (FAILED(mDevice->CreateSamplerState(&samplerDesc, getter_AddRefs(mLinearSamplerState)))) {
> +    NS_WARNING("Failed to create sampler state for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:239
(Diff revision 4)
> +  if (FAILED(mDevice->CreateInputLayout(layout,
> +                                        sizeof(layout) / sizeof(D3D11_INPUT_ELEMENT_DESC),
> +                                        sLayerQuadVS.mData,
> +                                        sLayerQuadVS.mLength,
> +                                        getter_AddRefs(mInputLayout)))) {
> +    NS_WARNING("Failed to create input layout for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:249
(Diff revision 4)
> +  CD3D11_BUFFER_DESC bufferDesc(sizeof(vertices), D3D11_BIND_VERTEX_BUFFER);
> +  D3D11_SUBRESOURCE_DATA data;
> +  data.pSysMem = (void*)vertices;
> +
> +  if (FAILED(mDevice->CreateBuffer(&bufferDesc, &data, getter_AddRefs(mVertexBuffer)))) {
> +    NS_WARNING("Failed to create vertex buffer for Oculus");

/s/puppet/oculus/g...

::: gfx/vr/gfxVRPuppet.cpp:268
(Diff revision 4)
>    }
>  
>    mIsPresenting = false;
>  }
>  
> +

nit: remove extra line here
Attachment #8865392 - Flags: review?(kgilbert) → review+
Comment on attachment 8870371 [details]
Bug 1343730 - Part 2: Get the submitframe result from VRDisplay;

https://reviewboard.mozilla.org/r/141824/#review145658

This LGTM, thanks!

Please have a DOM peer review as well, for the webidl change
Attachment #8870371 - Flags: review?(kgilbert) → review+
Comment on attachment 8865392 [details]
Bug 1343730 - Part 1: Support submitFrame and encode the frame as a base64 image in VRPuppet;

https://reviewboard.mozilla.org/r/137070/#review145652

> The NS_WARNING text should say "Puppet" rather than "Oculus"

Good catch! thanks.
Comment on attachment 8870371 [details]
Bug 1343730 - Part 2: Get the submitframe result from VRDisplay;

https://reviewboard.mozilla.org/r/141824/#review145832

::: dom/vr/VRDisplay.cpp:456
(Diff revision 2)
> +    return false;
> +  }
> +
> +  VRSubmitFrameResultInfo resultInfo;
> +  mClient->GetSubmitFrameResult(resultInfo);
> +  nsCString decodedImg;

nsAutoCString, plus, move it to line 462.

::: dom/vr/VRDisplay.cpp:468
(Diff revision 2)
> +    MOZ_ASSERT(false, "Failed to do decode base64 images.");
> +    return false;
> +  }
> +
> +  const char* srcData = (decodedImg.get());
> +  nsCString encodedImg;

nsAutoCString ? This can be moved to line 478:

nsCString encodeImg = gfxUtils::GetAsDataURI(dataSurface);

::: dom/vr/VRDisplay.cpp:947
(Diff revision 2)
> +{
> +  return VRSubmitFrameResultBinding::Wrap(aCx, this, aGivenProto);
> +}
> +
> +void
> +VRSubmitFrameResult::Update(uint32_t aFrameNum, const nsCString& aBase64Image)

const nsACString
Attachment #8870371 - Flags: review?(amarchesini) → review+
(In reply to Daosheng Mu[:daoshengmu] from comment #18)
> Comment on attachment 8870371 [details]
> Bug 1343730 - Part 2: Get the submitframe result from VRDisplay;
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/141824/diff/2-3/

Follow the comment 17 to do some modifications.
Pushed by dmu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c66058d34b17
Part 1: Support submitFrame and encode the frame as a base64 image in VRPuppet; r=kip
https://hg.mozilla.org/integration/autoland/rev/7ee7cf42cc02
Part 2: Get the submitframe result from VRDisplay; r=baku,kip
https://hg.mozilla.org/mozilla-central/rev/c66058d34b17
https://hg.mozilla.org/mozilla-central/rev/7ee7cf42cc02
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: