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

Issue 831191 link

Starred by 2 users
Status: Fixed
Owner:
cjgrant@chromium.org
Closed: Apr 2018
Cc:
█ vollick@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Fix VR scene frame lifecycle integrity

Project Member Reported by cjgrant@chromium.org, Apr 10 2018 
The VR UI uses a sequence of phases to ensure that properties aren't used before they're computed.

The optimizations to early-out on tree-update work exposed deficiencies in the checks.

At very least, we need to ensure that "dirty" state is restored on every element on every frame, and fix the DCHECKs that fire as a result of reinstating this.

These checks help avoid mysterious UI anomalies, and we should fix this ASAP.

Comment 1 by cjgrant@chromium.org, Apr 10 2018 

The primary outstanding reason we can't turn these on is the reposition plane.  It's binding has a custom hit test, which in turn checks element size.  To alleviate this, we'll need another way to hit test the plane to show the visual frame.

Note that the frame needs to stay highlighted even when the reticle is on the capture indicators, making this more tricky.

Comment 2 by cjgrant@chromium.org, Apr 10 2018 

If we can sacrifice the preserving of the visual frame while on the capture indicators, then there's a CL ready to address this issue.

Comment 3 by vollick@chromium.org, Apr 10 2018 

I'd like to keep the frame visible. I don't think it'll be too bad.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 11 2018 

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

commit fb1f6483b6f735594597cf2ba04cfd765f841720
Author: Christopher Grant <cjgrant@chromium.org>
Date: Wed Apr 11 16:59:27 2018

VR: Initialize all elements to frame lifecycle dirty

Now that frame lifecycle issues have been worked out, we can initialize
each element to dirty state when a frame starts, and rely on this state
being properly updated.

Use this opportunity to set each element's timestamp earlier in the
cycle, and update the testapp dump to respect the validity of the
information it's dumping.

BUG= 831191 

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;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I2b60cc49a09ea89d3e34867445beae6d6d46e76c
Reviewed-on: https://chromium-review.googlesource.com/1007322
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549911}
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/content_element.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/content_element.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/keyboard.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/keyboard.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/repositioner.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/repositioner.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/resizer.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/resizer.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/scaled_depth_adjuster.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/scaled_depth_adjuster.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/text_input.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/text_input.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/transient_element.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/transient_element.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/transient_element_unittest.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/ui_element.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/ui_element.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/viewport_aware_root.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/elements/viewport_aware_root.h
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/ui_scene.cc
[modify] https://crrev.com/fb1f6483b6f735594597cf2ba04cfd765f841720/chrome/browser/vr/ui_scene_unittest.cc

Comment 5 by cjgrant@chromium.org, Apr 11 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment