New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 867057 link

Starred by 1 user
Status: Started
Owner:
█ lincolnfrog@chromium.org
Last visit > 30 days ago
Cc:
█ lincolnfrog@chromium.org
█ offenwanger@chromium.org
Components:
EstimatedDays: 1
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-VR
Proj-XR-AR
Proj-XR



Sign in to add a comment

EnvironmentIntegrationProvider and FrameDataProvider must be share a message queue

Project Member Reported by billorr@chromium.org, Jul 24 
Hit test has a guarantee that results use the pose from the next frame.  This means that GetFrameData's callback on the blink side must be ordered correctly relative to GetHitTest's callback.

This may be solved by using associated interfaces.  That said, we don't necessarily need/want all interfaces to be associated with the source interface.

Comment 1 by billorr@chromium.org, Jul 24 

XRSession could become the "master interface" if we want these to be associated, or XRFrameDataProvider could be the "master interface" that we obtain XREnviromentIntegrationProvider from.

https://www.chromium.org/developers/design-documents/mojo/associated-interfaces

Comment 2 by lincolnfrog@chromium.org, Aug 7

Owner: lincolnfrog@chromium.org

Comment 3 by lincolnfrog@chromium.org, Aug 7

Labels: -Pri-3 Pri-1

Comment 4 by lincolnfrog@chromium.org, Aug 29

EstimatedDays: 1
Status: Started (was: Assigned)

Comment 5 by btebbs@chromium.org, Sep 4

Labels: AR-Cleanup

Comment 6 by ericde@google.com, Sep 14

Labels: M-71

Comment 7 by ddorwin@chromium.org, Oct 2

Labels: -M-71 Target-71
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 6 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/126a1168e3bcffb43c706822b3aa28695840310f

commit 126a1168e3bcffb43c706822b3aa28695840310f
Author: Max Rebuschatis <lincolnfrog@chromium.org>
Date: Tue Nov 06 00:47:06 2018

Make XR FrameData and Environment mojo associated

The XRFrameDataProvider now returns an associated
XREnvironmentDataProvider interface so that the
two share callback queues and thus allow strict
ordering of the two interfaces. This is critical
for frame synchronization between frames and the
associated environment data.

Note: We can't just mark the interfaceptrs for
XRFrameDataProvider and
XREnvironmentIntegrationProvider as associated
in the XRSession struct. XRDevice implementations
mostly live on separate threads from the
XRFrameDataProviders, so we'd have extra thread
hopping. For the VR headsets we explicitly live
off the main thread to avoid latency, and because
we do some work that may block the thread the
XRFrameDataProvider lives on (for example waiting
for vsync, or submitting frames to headset APIs).

Bug: 867057,  876135 , 843376
Change-Id: If2fb62fcd185825209dec08e421df05f34d41c30
Reviewed-on: https://chromium-review.googlesource.com/c/1171794
Commit-Queue: Max Rebuschatis <lincolnfrog@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bill Orr <billorr@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605545}
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/chrome/browser/android/vr/arcore_device/arcore_device.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/chrome/browser/android/vr/arcore_device/arcore_device_unittest.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/chrome/browser/android/vr/gvr_scheduler_delegate.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/chrome/browser/android/vr/gvr_scheduler_delegate.h
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/chrome/browser/vr/service/xr_device_impl.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/chrome/browser/vr/service/xr_runtime_manager.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/chrome/browser/vr/service/xr_runtime_manager_unittest.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/openvr/openvr_render_loop.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/openvr/openvr_render_loop.h
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/public/mojom/isolated_xr_service.mojom
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/public/mojom/vr_service.mojom
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/vr_device_base.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/vr_display_impl.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/vr_display_impl.h
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/device/vr/vr_display_impl_unittest.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/third_party/WebKit/LayoutTests/external/wpt/resources/chromium/webxr-test.js
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/third_party/WebKit/LayoutTests/xr/ar_hittest.html
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/third_party/blink/renderer/modules/xr/xr_device.cc
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/third_party/blink/renderer/modules/xr/xr_device.h
[modify] https://crrev.com/126a1168e3bcffb43c706822b3aa28695840310f/third_party/blink/renderer/modules/xr/xr_session.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 6 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ef86cd59863eed9aa0608987c2d0deef425d09c

commit 0ef86cd59863eed9aa0608987c2d0deef425d09c
Author: Dmitry Titov <dimich@chromium.org>
Date: Tue Nov 06 01:11:41 2018

Revert "Make XR FrameData and Environment mojo associated"

This reverts commit 126a1168e3bcffb43c706822b3aa28695840310f.

Reason for revert: Broke compile: https://logs.chromium.org/logs/chromium/bb/chromium.chrome/Google_Chrome_Win/39908/+/recipes/steps/compile/0/stdout

Original change's description:
> Make XR FrameData and Environment mojo associated
> 
> The XRFrameDataProvider now returns an associated
> XREnvironmentDataProvider interface so that the
> two share callback queues and thus allow strict
> ordering of the two interfaces. This is critical
> for frame synchronization between frames and the
> associated environment data.
> 
> Note: We can't just mark the interfaceptrs for
> XRFrameDataProvider and
> XREnvironmentIntegrationProvider as associated
> in the XRSession struct. XRDevice implementations
> mostly live on separate threads from the
> XRFrameDataProviders, so we'd have extra thread
> hopping. For the VR headsets we explicitly live
> off the main thread to avoid latency, and because
> we do some work that may block the thread the
> XRFrameDataProvider lives on (for example waiting
> for vsync, or submitting frames to headset APIs).
> 
> Bug: 867057,  876135 , 843376
> Change-Id: If2fb62fcd185825209dec08e421df05f34d41c30
> Reviewed-on: https://chromium-review.googlesource.com/c/1171794
> Commit-Queue: Max Rebuschatis <lincolnfrog@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Bill Orr <billorr@chromium.org>
> Reviewed-by: Klaus Weidner <klausw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605545}

TBR=dcheng@chromium.org,klausw@chromium.org,billorr@chromium.org,lincolnfrog@chromium.org

Change-Id: I7a2408629460f456bf1189acdb0837f7d22a7919
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 867057,  876135 , 843376
Reviewed-on: https://chromium-review.googlesource.com/c/1318805
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605551}
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/chrome/browser/android/vr/arcore_device/arcore_device.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/chrome/browser/android/vr/arcore_device/arcore_device_unittest.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/chrome/browser/android/vr/gvr_scheduler_delegate.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/chrome/browser/android/vr/gvr_scheduler_delegate.h
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/chrome/browser/vr/service/xr_device_impl.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/chrome/browser/vr/service/xr_runtime_manager.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/chrome/browser/vr/service/xr_runtime_manager_unittest.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/openvr/openvr_render_loop.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/openvr/openvr_render_loop.h
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/public/mojom/isolated_xr_service.mojom
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/public/mojom/vr_service.mojom
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/vr_device_base.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/vr_display_impl.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/vr_display_impl.h
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/device/vr/vr_display_impl_unittest.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/third_party/WebKit/LayoutTests/external/wpt/resources/chromium/webxr-test.js
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/third_party/WebKit/LayoutTests/xr/ar_hittest.html
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/third_party/blink/renderer/modules/xr/xr_device.cc
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/third_party/blink/renderer/modules/xr/xr_device.h
[modify] https://crrev.com/0ef86cd59863eed9aa0608987c2d0deef425d09c/third_party/blink/renderer/modules/xr/xr_session.cc

Comment 10 by lincolnfrog@chromium.org, Nov 6

Status: Fixed (was: Started)

Comment 11 by ddorwin@google.com, Nov 6

Status: Started (was: Fixed)
The CL was reverted and not landed?
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 8 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210

commit fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210
Author: Max Rebuschatis <lincolnfrog@chromium.org>
Date: Thu Nov 08 23:11:12 2018

Reland "Make XR FrameData and Environment mojo associated"

This is a reland of 126a1168e3bcffb43c706822b3aa28695840310f

The fix in this change addresses compile errors on the
Oculus platform.

TBR=dcheng@chromium.org
dcheng@chromium.org: please review, though there are no
new changes to vr_service.mojom

Original change's description:
> Make XR FrameData and Environment mojo associated
>
> The XRFrameDataProvider now returns an associated
> XREnvironmentDataProvider interface so that the
> two share callback queues and thus allow strict
> ordering of the two interfaces. This is critical
> for frame synchronization between frames and the
> associated environment data.
>
> Note: We can't just mark the interfaceptrs for
> XRFrameDataProvider and
> XREnvironmentIntegrationProvider as associated
> in the XRSession struct. XRDevice implementations
> mostly live on separate threads from the
> XRFrameDataProviders, so we'd have extra thread
> hopping. For the VR headsets we explicitly live
> off the main thread to avoid latency, and because
> we do some work that may block the thread the
> XRFrameDataProvider lives on (for example waiting
> for vsync, or submitting frames to headset APIs).
>
> Bug: 867057,  876135 , 843376
> Change-Id: If2fb62fcd185825209dec08e421df05f34d41c30
> Reviewed-on: https://chromium-review.googlesource.com/c/1171794
> Commit-Queue: Max Rebuschatis <lincolnfrog@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Bill Orr <billorr@chromium.org>
> Reviewed-by: Klaus Weidner <klausw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605545}

Bug: 867057,  876135 , 843376
Change-Id: If76578ebabbb40d03a21f6f557c5d4c27f69fc38
Reviewed-on: https://chromium-review.googlesource.com/c/1324089
Commit-Queue: Max Rebuschatis <lincolnfrog@chromium.org>
Reviewed-by: Bill Orr <billorr@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606643}
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/chrome/browser/android/vr/arcore_device/arcore_device.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/chrome/browser/android/vr/arcore_device/arcore_device_unittest.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/chrome/browser/android/vr/gvr_scheduler_delegate.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/chrome/browser/android/vr/gvr_scheduler_delegate.h
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/chrome/browser/vr/service/xr_device_impl.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/chrome/browser/vr/service/xr_runtime_manager.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/chrome/browser/vr/service/xr_runtime_manager_unittest.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/oculus/oculus_render_loop.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/oculus/oculus_render_loop.h
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/openvr/openvr_render_loop.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/openvr/openvr_render_loop.h
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/public/mojom/isolated_xr_service.mojom
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/public/mojom/vr_service.mojom
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/vr_device_base.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/vr_display_impl.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/vr_display_impl.h
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/device/vr/vr_display_impl_unittest.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/third_party/WebKit/LayoutTests/external/wpt/resources/chromium/webxr-test.js
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/third_party/WebKit/LayoutTests/xr/ar_hittest.html
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/third_party/blink/renderer/modules/xr/xr_device.cc
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/third_party/blink/renderer/modules/xr/xr_device.h
[modify] https://crrev.com/fee9f1985d7d8cc2f51bc6dddd4c63aa93cc9210/third_party/blink/renderer/modules/xr/xr_session.cc
►

Sign in to add a comment