VR Shell: Allow UI elements to determine hit testing.

chrome/browser/android/vr_shell/ui_elements/close_button.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/vr_shell/ui_elements/close_button.h"
 
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/android/vr_shell/textures/close_button_texture.h"
 
 namespace vr_shell {
 
 CloseButton::CloseButton(base::Callback<void()> click_handler)
     : TexturedElement(256),
       texture_(base::MakeUnique<CloseButtonTexture>()),
       click_handler_(click_handler) {}
 
 CloseButton::~CloseButton() = default;
 
 void CloseButton::OnHoverEnter(gfx::PointF position) {
-  hover_ = true;
-  OnStateUpdated();
+  OnStateUpdated(position);
 }
 
 void CloseButton::OnHoverLeave() {
-  hover_ = false;
-  OnStateUpdated();
+  OnStateUpdated(gfx::PointF(-1000, -1000));

[cjgrant, 2017/05/15 16:55:09]

Should probably use language numeric limits here, for correctness and
readability.

[mthiesse, 2017/05/15 21:11:27]

I didn't want the code to have to think about infinities or NaNs. This is just
some position that's outside of the element's hit-testable region. Sure NaN is
technically more correct, but having to check isNaN everywhere would just be
tedious.

[cjgrant, 2017/05/15 21:51:52]

I just meant to use the modern equivalent of INT_MAX (or whatever) instead of
-1000, not to use isNan for checks.

[mthiesse, 2017/05/15 22:23:32]

Done.

+}
+
+void CloseButton::OnMove(gfx::PointF position) {
+  OnStateUpdated(position);
 }
 
 void CloseButton::OnButtonDown(gfx::PointF position) {
   down_ = true;
-  OnStateUpdated();
+  OnStateUpdated(position);
 }
 
 void CloseButton::OnButtonUp(gfx::PointF position) {
   down_ = false;
-  OnStateUpdated();
-  if (position.x() < 0 || position.x() > 1.0f)
-    return;
-  if (position.y() < 0 || position.y() > 1.0f)
-    return;
-  click_handler_.Run();
+  OnStateUpdated(position);
+  if (HitTest(position))

[cjgrant, 2017/05/15 16:55:09]

At first glance, I'd have thought the common code would do the hit test, and
only call OnButtonUp if we're on the button.  But this is our way of working
around OnButtonDownCancel(), I guess.  The fact that HitTest needs to be used
both in and out of this class is weird to me.

[mthiesse, 2017/05/15 21:11:26]

I think this will make more sense once you see the URL bar? HitTest determines
whether the element should be handling input at all, which happens to overlap
with the clickable area in the case of this button. With the URL bar HitTest
isn't the same as the check in ButtonUp.

+    click_handler_.Run();
+}
+
+bool CloseButton::HitTest(gfx::PointF point) const {
+  return (point - gfx::PointF(0.5, 0.5)).LengthSquared() < 0.25;
 }
 
 UiTexture* CloseButton::GetTexture() const {
   return texture_.get();
 }
 
-void CloseButton::OnStateUpdated() {
-  int flags = hover_ ? CloseButtonTexture::FLAG_HOVER : 0;
-  flags |= (down_ && hover_) ? CloseButtonTexture::FLAG_DOWN : 0;
+void CloseButton::OnStateUpdated(gfx::PointF position) {
+  bool hitting = HitTest(position);
+  bool down = hitting ? down_ : false;
+
+  int flags = hitting ? CloseButtonTexture::FLAG_HOVER : 0;
+  flags |= down ? CloseButtonTexture::FLAG_DOWN : 0;
   if (!texture_->SetDrawFlags(flags))
     return;
   UpdateTexture();
 }
 
 }  // namespace vr_shell