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

Issue 782395 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

Introduce class name to UiElement so it can be used to query element without name

Project Member Reported by bshe@chromium.org, Nov 7 2017

Issue description

For example, a Button might be composited by a Rect and VectorIcon. It would not scale well if we need to give each of these child elements a unique name when create a Button. Instead, we could name them as kNone and use button's name and class name to uniquely identify them.
 

Comment 1 by bshe@chromium.org, Nov 7 2017

Cc: vollick@chromium.org
+cc vollick explicitly
Ian, please correct me if my understanding was not correct about class name.
Thanks for logging this.

My hope for class is that it is a way of referring to swaths of elements, but it may not end up being unique, even if paired with a name.

For example, we might have a class for all button backgrounds (and we can have a default class for UiElements set in the ctor).
Owner: vollick@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 27 2017

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

commit d18d0e56358dde5a2ad0e58b561375ec5fd6ad65
Author: Ian Vollick <vollick@chromium.org>
Date: Mon Nov 27 18:45:52 2017

[vr] Add a depth/scale adjustment element

This change introduces a ScaledDepthAdjuster element whose role is to
both position its descendants at the correct depth and ensure that they
inherit a scale that permits their units to be expressed in standard
DMM.

Bug:  782395 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Iec5481edcf0a60f78fd0a16c35a32c981ee84b6f
Reviewed-on: https://chromium-review.googlesource.com/789436
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519348}
[modify] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/BUILD.gn
[modify] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/button.cc
[add] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/scaled_depth_adjuster.cc
[add] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/scaled_depth_adjuster.h
[add] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/scaled_depth_adjuster_unittest.cc
[modify] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/ui_element.cc
[modify] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/ui_element.h
[modify] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/ui_element_name.cc
[add] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/ui_element_type.cc
[add] https://crrev.com/d18d0e56358dde5a2ad0e58b561375ec5fd6ad65/chrome/browser/vr/elements/ui_element_type.h

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 28 2017

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

commit 4face97457dc574a442cf5b4a6ed87f6df3e2bb6
Author: Ian Vollick <vollick@chromium.org>
Date: Tue Nov 28 01:16:08 2017

[vr] Switch to exclusive visibility checks in the UI unittests

Previously, there were many spots in the UI unittests where we checked
that a set of UI elements (and only that set of UI elements) was
visible. This had regressed, however, and in the interim, elements had
been added that were not accounted for in the test expectations.

In this CL, I have switched back to exclusive visibility checks, and
have updated the appropriate expectations. I have also introduced the
notion of an owner elements. This both permits unit tests to refer to
unnamed sub-elements meaningfully and allows for better debug output
(the hierarchy dumps are much more meaningful).

This also permitted including these unnamed sub-elements in the
stacking unit test. I have also moved the remaining stacking unit
tests into the UI unittests.

I have also updated the backplane click test to simulate the response
to the click so we can check that the visuals update as we expect.

Bug:  782395 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I8f4f0fb92df18dc6b8d7a8e086768d4a97136898
Reviewed-on: https://chromium-review.googlesource.com/789440
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519503}
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/BUILD.gn
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/elements/button.cc
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/elements/button.h
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/elements/button_unittest.cc
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/elements/ui_element.cc
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/elements/ui_element.h
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/test/ui_test.cc
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/test/ui_test.h
[delete] https://crrev.com/383af21a3652791eba7326b3c9c7fc0a573f0ca7/chrome/browser/vr/ui_renderer_unittest.cc
[modify] https://crrev.com/4face97457dc574a442cf5b4a6ed87f6df3e2bb6/chrome/browser/vr/ui_unittest.cc

Labels: M-64 Test-Complete

Sign in to add a comment