Closed
Bug 1251886
Opened 8 years ago
Closed 8 years ago
WebVR rendering is broken
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
VERIFIED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: caseyyee.ca, Assigned: kip)
Details
(Whiteboard: [webvr])
Attachments
(1 file)
On the latest Firefox Nightly running Windows 10 with 0.8 Oculus runtime: First reported on reddit: https://www.reddit.com/r/WebVR/comments/47ftcp/help_why_is_the_image_in_the_dk2_set_off_to_the/ Examples that can reproduce the issue: - three.js VR examples renders stereo viewport offset incorrectly with a offset. http://threejs.org/examples/#webvr_cubes - When entering VR in any WebVR example (ie. mozvr.com), I get a cropped, rectangle view of the scene.
Flags: needinfo?(kgilbert)
Adding from Slack discussion: @kip with checking regression using MozRegression "I didn't see any bug for those issues specifically. I was hoping that the MozRegression would point to an already identified regression, but it doesn't appear so"
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → kgilbert
Flags: needinfo?(kgilbert)
Assignee | ||
Comment 2•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1b52a4e2e2f
Assignee | ||
Comment 3•8 years ago
|
||
I have found the cause of the issue. The inputFrameID was coming in as -1 to gfxOculus::SubmitFrame, resulting in invalid pose information being used. This only happens with e10s enabled. Disabling e10s effectively works around the problem until the fix is posted.
Assignee | ||
Comment 4•8 years ago
|
||
- ContainerLayerComposite::ContainerRenderVR failed to find a CanvasLayerComposite and get an inputFrameId as it was only looking at immediate children layers. - Initialize Oculus SDK structures with 0 to ensure code is less brittle when Oculus SDK is updated in the future. - Added assert and sanity check to ensure that valid inputFrameID's are used when indexing the Oculus pose buffers. Review commit: https://reviewboard.mozilla.org/r/37537/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37537/
Assignee | ||
Updated•8 years ago
|
Attachment #8725506 -
Flags: review?(dmu)
Comment 5•8 years ago
|
||
Comment on attachment 8725506 [details] MozReview Request: Bug 1251886 - Correct inputFrameID selection when using e10s https://reviewboard.mozilla.org/r/37537/#review34365 Most of all are good. Just have a concern about layer search. ::: gfx/layers/composite/ContainerLayerComposite.cpp:287 (Diff revision 1) > + searchLayers.push(sibling->AsLayerComposite()); Will it add searchLayer back to searchLayers? IIUC, ```Layer* sibling = searchLayer->GetLayer();``` will get itself and push back to searchLayers? That needs to be changed to like below? ``` if (sibling) { sibling = sibling->GetNextSibling(); searchLayers.push(sibling->AsLayerComposite()); } ```
Attachment #8725506 -
Flags: review?(dmu)
Assignee | ||
Comment 6•8 years ago
|
||
https://reviewboard.mozilla.org/r/37537/#review34365 > Will it add searchLayer back to searchLayers? IIUC, ```Layer* sibling = searchLayer->GetLayer();``` will get itself and push back to searchLayers? That needs to be changed to like below? > ``` > if (sibling) { > sibling = sibling->GetNextSibling(); > searchLayers.push(sibling->AsLayerComposite()); > } > ``` Just above that part, it does a "sibling = sibling->GetNextSibling()": Layer* sibling = searchLayer->GetLayer(); if (sibling) { sibling = sibling->GetNextSibling(); } if (sibling) { searchLayers.push(sibling->AsLayerComposite()); } I do this in a separate "if", so that I don't dereference a nullptr if there are no more siblings. On each iteration, it should add both the first child and the next sibling so that it iterates over the entire tree of layers below the layerToRender.
Comment 7•8 years ago
|
||
https://reviewboard.mozilla.org/r/37537/#review34607 It is good to me
Comment 8•8 years ago
|
||
Comment on attachment 8725506 [details] MozReview Request: Bug 1251886 - Correct inputFrameID selection when using e10s https://reviewboard.mozilla.org/r/37537/#review34611
Attachment #8725506 -
Flags: review+
Comment 9•8 years ago
|
||
https://reviewboard.mozilla.org/r/37537/#review34365 > Just above that part, it does a "sibling = sibling->GetNextSibling()": > > Layer* sibling = searchLayer->GetLayer(); > > if (sibling) { > sibling = sibling->GetNextSibling(); > } > > if (sibling) { > searchLayers.push(sibling->AsLayerComposite()); > } > > I do this in a separate "if", so that I don't dereference a nullptr if there are no more siblings. > On each iteration, it should add both the first child and the next sibling so that it iterates over the entire tree of layers below the layerToRender. Ok. Got it!
Updated•8 years ago
|
Component: General → Graphics: Layers
Product: Firefox → Core
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c00b55c469ec
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Casey Yee, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(caseyyee.ca)
Comment 13•8 years ago
|
||
This doesn't seem to be fixed for me in the latest Nightly (Version: 48.0a1, Build ID: 20160325085113)
Comment 14•8 years ago
|
||
Nevermind. I've run into a different bug.
Comment 15•8 years ago
|
||
(In reply to Brian Peiris from comment #14) > Nevermind. I've run into a different bug. What's the bug?
Comment 16•8 years ago
|
||
webvr-boilerplate unnecessarily changes the FoV of the HMD (https://github.com/borismus/webvr-boilerplate/pull/109/files) and it seems Nightly's WebVR implementation retains the FoV between sites. Steps to repro: 1. Visit https://cdn.rawgit.com/mrdoob/three.js/r75/examples/vr_cubes.html, enter VR mode and see that the FoV is correct 2. Visit https://cdn.rawgit.com/borismus/webvr-boilerplate/ecd6afc/index.html, enter VR mode and see that Fov is incorrect 3. Visit https://cdn.rawgit.com/mrdoob/three.js/r75/examples/vr_cubes.html again and see that the FoV is incorrect this time.
Based on comment 17.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•