Skip to content

GitLab

T
tangled
Commit b3df3978 authored by Ian Vollick's avatar Ian Vollick Committed by Commit Bot
Browse files
Options

[vr] Draw the reticle after all elements 

This change reverts my previous attempt at sorting the reticle into the
list of UI elements. It turns out that this is more subtle than I'd
originally appreciated, and until we can come up with a robust approach
that avoids all artifacts, I'm reverting to a state where the reticle
consistently draws atop all UI elements.

Bug: 779692
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: I185b70f4c0137a6513c67765c1696fe16f996352
Reviewed-on: https://chromium-review.googlesource.com/826431Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524156}
parent b1dae1bf
Hide whitespace changes
Inline Side-by-side
Showing with 10 additions and 178 deletions
chrome/browser/vr/elements/reticle.cc
View file @ b3df3978
......@@ -76,6 +76,7 @@ static constexpr float kReticleColor[] = {1.0f, 1.0f, 1.0f, 1.0f};
Reticle::Reticle(UiScene* scene, Model* model) : scene_(scene), model_(model) {
SetName(kReticle);
SetDrawPhase(kPhaseForeground);
set_hit_testable(false);
SetVisible(true);
}
......
chrome/browser/vr/ui_scene.cc
View file @ b3df3978
......@@ -24,52 +24,13 @@ namespace vr {
namespace {
constexpr float kTolerance = 1e-5f;
// Returns true if two elements are in the same plane.
bool ArePlanar(const UiElement& e1, const UiElement& e2) {
gfx::Vector3dF n1 = e1.GetNormal();
gfx::Vector3dF n2 = e2.GetNormal();
if (!base::IsApproximatelyEqual(n1.x(), n2.x(), kTolerance) ||
!base::IsApproximatelyEqual(n1.y(), n2.y(), kTolerance) ||
!base::IsApproximatelyEqual(n1.z(), n2.z(), kTolerance)) {
return false;
}
return base::IsApproximatelyEqual(
0.0f, gfx::DotProduct(n1, e1.GetCenter() - e2.GetCenter()), kTolerance);
}
template <typename P>
UiScene::Elements GetVisibleElements(UiElement* root,
UiElement* reticle_element,
P predicate) {
Reticle* reticle = static_cast<Reticle*>(reticle_element);
UiElement* target = reticle ? reticle->TargetElement() : nullptr;
UiScene::Elements elements;
int reticle_parent_id = 0;
for (auto& element : *root) {
if (element.IsVisible() && predicate(&element)) {
if (element.IsVisible() && predicate(&element))
elements.push_back(&element);
if (target && target->id() == element.id()) {
reticle_parent_id = element.id();
// Draw the reticle after the last child that resides in the same plane
// as this element. This way, the reticle can't be partially hidden as
// it passes across portions of a composite element. By walking the
// element's subtree in reverse pre-order, we start at the last-rendered
// child and work backwards, possibly as far as the originally targeted
// element.
for (auto& child : base::Reversed(element)) {
if (predicate(&child) && ArePlanar(element, child)) {
reticle_parent_id = child.id();
break;
}
}
}
}
if (reticle_parent_id == element.id()) {
elements.push_back(reticle);
reticle->SetDrawPhase(element.draw_phase());
}
}
return elements;
}
......@@ -212,8 +173,7 @@ UiElement* UiScene::GetUiElementByName(UiElementName name) const {
UiScene::Elements UiScene::GetVisible2dBrowsingElements() const {
return GetVisibleElements(
GetUiElementByName(k2dBrowsingRoot), GetUiElementByName(kReticle),
[](UiElement* element) {
GetUiElementByName(k2dBrowsingRoot), [](UiElement* element) {
return element->draw_phase() == kPhaseForeground ||
element->draw_phase() == kPhaseFloorCeiling ||
element->draw_phase() == kPhaseBackground;
......@@ -222,8 +182,7 @@ UiScene::Elements UiScene::GetVisible2dBrowsingElements() const {
UiScene::Elements UiScene::GetVisible2dBrowsingOverlayElements() const {
return GetVisibleElements(
GetUiElementByName(k2dBrowsingRoot), GetUiElementByName(kReticle),
[](UiElement* element) {
GetUiElementByName(k2dBrowsingRoot), [](UiElement* element) {
return element->draw_phase() == kPhaseOverlayBackground ||
element->draw_phase() == kPhaseOverlayForeground;
});
......@@ -231,8 +190,7 @@ UiScene::Elements UiScene::GetVisible2dBrowsingOverlayElements() const {
UiScene::Elements UiScene::GetVisibleSplashScreenElements() const {
return GetVisibleElements(
GetUiElementByName(kSplashScreenRoot), GetUiElementByName(kReticle),
[](UiElement* element) {
GetUiElementByName(kSplashScreenRoot), [](UiElement* element) {
return element->draw_phase() == kPhaseOverlayBackground ||
element->draw_phase() == kPhaseOverlayForeground;
});
......@@ -240,36 +198,20 @@ UiScene::Elements UiScene::GetVisibleSplashScreenElements() const {
UiScene::Elements UiScene::GetVisibleWebVrOverlayForegroundElements() const {
return GetVisibleElements(
GetUiElementByName(kWebVrRoot), GetUiElementByName(kReticle),
[](UiElement* element) {
GetUiElementByName(kWebVrRoot), [](UiElement* element) {
return element->draw_phase() == kPhaseOverlayForeground;
});
}
UiScene::Elements UiScene::GetVisibleControllerElements() const {
return GetVisibleElements(
GetUiElementByName(kControllerGroup), nullptr, [](UiElement* element) {
if (element->name() == kReticle) {
Reticle* reticle = static_cast<Reticle*>(element);
// If the reticle has a non-null target element,
// it would have been positioned elsewhere.
bool need_to_add_reticle = !reticle->TargetElement();
if (need_to_add_reticle) {
// We must always update the reticle's draw phase when it is
// included in a list of elements we vend. The other controller
// elements are drawn in the foreground phase, so we will update the
// reticle to match here.
reticle->SetDrawPhase(kPhaseForeground);
}
return need_to_add_reticle;
}
return element->draw_phase() == kPhaseForeground;
});
return GetVisibleElements(GetUiElementByName(kControllerGroup),
[](UiElement* element) {
return element->draw_phase() == kPhaseForeground;
});
}
UiScene::Elements UiScene::GetVisibleKeyboardElements() const {
return GetVisibleElements(GetUiElementByName(kKeyboard),
GetUiElementByName(kReticle),
[](UiElement* element) {
return element->draw_phase() == kPhaseForeground;
});
......
chrome/browser/vr/ui_unittest.cc
View file @ b3df3978
......@@ -997,44 +997,6 @@ TEST_F(UiTest, ExitPresentAndFullscreenOnAppButtonClick) {
ui_->OnAppButtonClicked();
}
TEST(UiReticleTest, ReticleRenderedOnPlanarChildren) {
UiScene scene;
Model model;
auto reticle = base::MakeUnique<Reticle>(&scene, &model);
reticle->SetDrawPhase(kPhaseNone);
scene.root_element().AddChild(std::move(reticle));
auto element = base::MakeUnique<UiElement>();
UiElement* parent = element.get();
parent->SetDrawPhase(kPhaseForeground);
parent->SetName(k2dBrowsingRoot);
scene.root_element().AddChild(std::move(element));
model.reticle.target_element_id = parent->id();
// Add 4 children to the parent:
// - Parent (hit element, initially having reticle)
// - Planar
// - Planar but offset (should receive reticle)
// - Parallel
// - Rotated
for (int i = 0; i < 4; i++) {
element = base::MakeUnique<UiElement>();
element->SetDrawPhase(kPhaseForeground);
parent->AddChild(std::move(element));
}
parent->children()[1]->SetTranslate(1, 0, 0);
parent->children()[2]->SetTranslate(0, 0, -1);
parent->children()[3]->SetRotate(1, 0, 0, 1);
scene.OnBeginFrame(MsToTicks(0), kForwardVector);
// Sorted set should have 1 parent, 4 children and 1 reticle.
UiScene::Elements sorted = scene.GetVisible2dBrowsingElements();
EXPECT_EQ(6u, sorted.size());
// Reticle should be after the parent and first two children.
EXPECT_EQ(sorted[3]->name(), kReticle);
}
// This test ensures that the render order matches the expected tree state. All
// phases are merged, as the order is the same. Unnamed and unrenderable
// elements are skipped.
......@@ -1048,79 +1010,6 @@ TEST_F(UiTest, UiRendererSortingTest) {
}
}
TEST_F(UiTest, ReticleStacking) {
CreateScene(kNotInCct, kNotInWebVr);
UiElement* content = scene_->GetUiElementByName(kContentQuad);
EXPECT_TRUE(content);
model_->reticle.target_element_id = content->id();
auto unsorted = scene_->GetVisible2dBrowsingElements();
auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
bool saw_target = false;
for (auto* e : sorted) {
if (e == content) {
saw_target = true;
} else if (saw_target) {
EXPECT_EQ(kReticle, e->name());
break;
}
}
EXPECT_TRUE(saw_target);
auto controller_elements = scene_->GetVisibleControllerElements();
bool saw_reticle = false;
for (auto* e : controller_elements) {
if (e->name() == kReticle) {
saw_reticle = true;
}
}
EXPECT_FALSE(saw_reticle);
}
TEST_F(UiTest, ReticleStackingAtopForeground) {
CreateScene(kNotInCct, kNotInWebVr);
UiElement* element = scene_->GetUiElementByName(kContentQuad);
EXPECT_TRUE(element);
element->SetDrawPhase(kPhaseOverlayForeground);
model_->reticle.target_element_id = element->id();
auto unsorted = scene_->GetVisible2dBrowsingOverlayElements();
auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
bool saw_target = false;
for (auto* e : sorted) {
if (e == element) {
saw_target = true;
} else if (saw_target) {
EXPECT_EQ(kReticle, e->name());
break;
}
}
EXPECT_TRUE(saw_target);
auto controller_elements = scene_->GetVisibleControllerElements();
bool saw_reticle = false;
for (auto* e : controller_elements) {
if (e->name() == kReticle) {
saw_reticle = true;
}
}
EXPECT_FALSE(saw_reticle);
}
TEST_F(UiTest, ReticleStackingWithControllerElements) {
CreateScene(kNotInCct, kNotInWebVr);
UiElement* element = scene_->GetUiElementByName(kReticle);
EXPECT_TRUE(element);
element->SetDrawPhase(kPhaseBackground);
EXPECT_NE(scene_->GetUiElementByName(kLaser)->draw_phase(),
element->draw_phase());
model_->reticle.target_element_id = 0;
auto unsorted = scene_->GetVisibleControllerElements();
auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
EXPECT_EQ(element->DebugName(), sorted.back()->DebugName());
EXPECT_EQ(scene_->GetUiElementByName(kLaser)->draw_phase(),
element->draw_phase());
}
// Tests that transient elements will show, even if there is a long delay
// between when they are asked to appear and when we issue the first frame with
// them visible.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment