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

Issue 768079 link

Starred by 2 users
Status: Fixed
Owner:
cjgrant@chromium.org
Closed: Apr 2018
Cc:
█ asimjour@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-XR
Proj-XR-VR



Sign in to add a comment

Hit testing should operate on the set of rendered elements

Project Member Reported by vollick@chromium.org, Sep 22 2017 
Currently the UiInputManager owns a UiScene and decides independently how to traverse the scene to find elements to hit.

This turns out to be error prone. If we make a mistake here and hit testing returns an element that is not in fact rendered by the UiRenderer, we can break reticle rendering (cf 767883).

We should update the hit testing code to operate on precisely the same elements that are rendered.

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

Cc: -cjgrant@chromium.org
Labels: VR-TD Proj-VR
Owner: cjgrant@chromium.org
Status: Started (was: Available)
We've essentially fixed this now - UiInputManager calls scene->GetVisibleElements().  However, it then does a sort, and a hit-test check.

We should:

- Have scene supply GetSortedHitTestableElements(), such that UiInputManager doesn't make any decisions on elements.
- Filter hit-testable elements before doing the sort.

This is easy stuff now - I'll just do it.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 25 2018 

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

commit cd8d48e7440f0656ca0b369d9df9630a791376a6
Author: Christopher Grant <cjgrant@chromium.org>
Date: Wed Apr 25 18:18:34 2018

VR: Clean up selection of hit-testable elements.

This furthers previous lifecycle framework cleanup, and fixes an
outstanding technical-debt bug in the process.

BUG= 768079 
R=ymalik

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: I6743ed68e16f437d68ccc27849f3694af347617a
Reviewed-on: https://chromium-review.googlesource.com/1015645
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553664}
[modify] https://crrev.com/cd8d48e7440f0656ca0b369d9df9630a791376a6/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/cd8d48e7440f0656ca0b369d9df9630a791376a6/chrome/browser/vr/ui_input_manager.cc
[modify] https://crrev.com/cd8d48e7440f0656ca0b369d9df9630a791376a6/chrome/browser/vr/ui_renderer.cc
[modify] https://crrev.com/cd8d48e7440f0656ca0b369d9df9630a791376a6/chrome/browser/vr/ui_scene.cc
[modify] https://crrev.com/cd8d48e7440f0656ca0b369d9df9630a791376a6/chrome/browser/vr/ui_scene.h
[modify] https://crrev.com/cd8d48e7440f0656ca0b369d9df9630a791376a6/chrome/browser/vr/ui_scene_unittest.cc

Comment 3 by cjgrant@chromium.org, Apr 25 2018

Status: Fixed (was: Started)

Comment 4 by dougman@chromium.org, May 8 2018

Labels: Test-Complete
►

Sign in to add a comment