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

Issue 848074 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-XR



Sign in to add a comment

VR Controller element perpetually dirty

Project Member Reported by bsheedy@chromium.org, May 30 2018

Issue description

Since https://chromium-review.googlesource.com/c/chromium/src/+/1058558 went in, the kController element in VR is perpetually dirty. This breaks the native UI interaction/quiescence test code since it will never think that the UI has reached quiescence.
 
Specifically, the controller's bindings are being updated every frame.
The binding that's constantly being updated is model->controller.transform => view->set_local_transform(value). Looking at the head_offset_ value that the VR controller uses, it's constantly changing in at least one axis by ~0.000001 (even on a 3DOF headset). Do we need that level of precision, or could we round to the nearest 0.00001 (or maybe apply a deadzone?)
It's changing even when the head/controller are controlled by the test APIs and stationary?

I wonder if a very tiny deadzone would also help with pixel swim issues? Worth trying I think.
The head isn't currently controlled by the test APIs, but the controller is. This is on a Pixel 2, though, so the head's position should be stationary regardless.

I added a deadzone to the head offset, which fixed the issue with the controller transform constantly getting updated. However, that exposed a number of other bindings that get updated frequently, such as:

model->controller.laser_direction => view->set_laser_direction(value)
model->controller.opacity => view->SetOpacity(value)

These don't seem to get stuck perpetually updating like the head offset was, or at least they don't do so as frequently. However, it's often enough to prevent the UI from reaching a steady state.
Cc: cjgrant@chromium.org
Maybe we need to explicitly exclude the controller from steady state checks for testing then...
I think I was mistaken about the bindings still causing issues. Digging deeper, it looks like the reason why the scene thinks it's dirty now is due to UpdateWorldSpaceTransform. The following are regularly reporting that they updated their world space transform:

k2dBrowsingRepositioner
k2dBrowsingVisibiltyHider
k2dBrowsingVisibiltyFader
kContentRepositionVisibilityToggle
k2dBrowsingForeground
k2dBrowsingContentGroup
kBackplane
kContentFrame
kContentFrameHitPlane
kContentResizer
kContentQuad
kIndicatorBackplane
kIndicatorLayout
kUrlBarPositioner
kUrlBarDmmRoot
kUrlBar
kUrlBarLayout
kUrlBarBackButton
kNone
kUrlBarLeftSeparator
kUrlBarOriginRegion
kNone
kNone
kUrlBarOriginLayout
kUrlBarSecurityButtonRegion
kUrlBarSecurityButton
kNone
kUrlBarUrlText
kUrlBarRightSeparator
kUrlBarOverflowButton
kNone
kNone
k2dBrowsingViewportAwareRoot
kOmniboxDmmRoot
kKeyboardDmmRoot
kReticle
Looking at the code, the reticle always updates its transform, and AFAIK is always visible, so the scene will always be dirtied... How did this work in the first place?
Turns out https://chromium.googlesource.com/chromium/src/+/6280845b34d88c4850c2960226f3e4be8de339f2%5E%21/#F9 recently caused UpdateWorldSpaceTransform to dirty the scene.
The reticle dirtying the scene is expected - controller quiescence would fix this, but that's currently disabled. Still no idea why so many other elements are constantly updating their world space transform, though.
Cc: vollick@chromium.org
The majority of those elements don't actually have a dirty transform - only kUrlBarRightSeparator and kUrlBarOverflowButton do. The rest seem to be updated because they're children of k2dBrowsingRepositioner, which always updates its world space transform.
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 5 2018

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

commit e11237623b9246c3ed19eb6774cbe29b93d24a01
Author: Ian Vollick <vollick@chromium.org>
Date: Tue Jun 05 19:59:40 2018

[vr] Only mark scene as dirty if transforms have actually changed

Previously, I'd assumed that if ShouldUpdateWorldSpaceTransform was
true that we would update the world space transform. It may be the case,
however, that the update results in no meaningful change (the reticle
may not have moved, for example). With this change, we only mark the
scene as dirty if the new transforms are not approximately equal to the
old transforms.

Bug:  848074 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: I9f3225b852755d8296deec58f78215838dc7b334
Reviewed-on: https://chromium-review.googlesource.com/1087170
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564622}
[modify] https://crrev.com/e11237623b9246c3ed19eb6774cbe29b93d24a01/chrome/browser/vr/elements/content_element_unittest.cc
[modify] https://crrev.com/e11237623b9246c3ed19eb6774cbe29b93d24a01/chrome/browser/vr/elements/ui_element.cc
[modify] https://crrev.com/e11237623b9246c3ed19eb6774cbe29b93d24a01/chrome/browser/vr/ui_input_manager_unittest.cc
[modify] https://crrev.com/e11237623b9246c3ed19eb6774cbe29b93d24a01/chrome/browser/vr/ui_unittest.cc

Status: Fixed (was: Assigned)
This ought to be addressed by the CL in #12.
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 6 2018

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

commit 558635267b37db79b830022c78e209ab31a68c6c
Author: bsheedy <bsheedy@chromium.org>
Date: Wed Jun 06 18:19:14 2018

Fix VrShellTransitionTest#testStartActivityTriggersDoff*

Implements two fixes that were causing testStartActivityTriggersDoff* to
fail 100% of the time.

1. Waits until the UI is quiescent after starting the activity, as
previously, clicks were being sent before the DOFF prompt was visible,
causing the test to time out.

2. Applies a small deadzone to the head offset used for repositioning
the VR controller on 6DOF devices. Even on 3DOF devices, the offset
drifted within a very tiny area, which cause the controller to be
perpetually dirty and prevent the UI from reaching quiescence.

Bug: 831589,  848074 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: Ic912dd3aa92ac2102c34e4acc330447a3ce0ca8d
Reviewed-on: https://chromium-review.googlesource.com/1087723
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564970}
[modify] https://crrev.com/558635267b37db79b830022c78e209ab31a68c6c/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTransitionTest.java
[modify] https://crrev.com/558635267b37db79b830022c78e209ab31a68c6c/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/NativeUiUtils.java
[modify] https://crrev.com/558635267b37db79b830022c78e209ab31a68c6c/chrome/browser/android/vr/vr_controller.cc
[modify] https://crrev.com/558635267b37db79b830022c78e209ab31a68c6c/chrome/browser/vr/ui.cc

Components: Internals>XR
Labels: -VR-Caught-By-Test XR-Caught-By-Test

Sign in to add a comment