VR Controller element perpetually dirty |
||||||
Issue descriptionSince 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.
,
May 30 2018
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?)
,
May 31 2018
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.
,
May 31 2018
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.
,
May 31 2018
Maybe we need to explicitly exclude the controller from steady state checks for testing then...
,
May 31 2018
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
,
May 31 2018
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?
,
May 31 2018
Turns out https://chromium.googlesource.com/chromium/src/+/6280845b34d88c4850c2960226f3e4be8de339f2%5E%21/#F9 recently caused UpdateWorldSpaceTransform to dirty the scene.
,
Jun 1 2018
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.
,
Jun 1 2018
,
Jun 1 2018
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.
,
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
,
Jun 6 2018
This ought to be addressed by the CL in #12.
,
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
,
Jul 4
,
Aug 29
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bsheedy@chromium.org
, May 30 2018