Split Screen mode implementation

athena/wm/bezel_controller.cc

 // Copyright 2014 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 "athena/wm/bezel_controller.h"
 
+#include "athena/wm/coordinate_conversion.h"
 #include "ui/aura/window.h"
 #include "ui/events/event_handler.h"
 
 namespace athena {
 namespace {
 
 // Using bezel swipes, the first touch that is registered is usually within
 // 5-10 pixels from the edge, but sometimes as far as 29 pixels away.
 // So setting this width fairly high for now.
 const float kBezelWidth = 20.0f;
 
-const float kScrollPositionNone = -100;
+const float kScrollDeltaNone = 0;
 
 bool ShouldProcessGesture(ui::EventType event_type) {
   return event_type == ui::ET_GESTURE_SCROLL_UPDATE ||
          event_type == ui::ET_GESTURE_SCROLL_BEGIN ||
          event_type == ui::ET_GESTURE_BEGIN ||
          event_type == ui::ET_GESTURE_END;
 }
 
 }  // namespace
 
 BezelController::BezelController(aura::Window* container)
     : container_(container),
       state_(NONE),
       scroll_bezel_(BEZEL_NONE),
       scroll_target_(NULL),
       left_right_delegate_(NULL) {
 }
 
-float BezelController::GetDistance(const gfx::PointF& position,
+float BezelController::GetDistance(const gfx::PointF& location,
+                                   aura::Window* window,
                                    BezelController::Bezel bezel) {
   DCHECK(bezel == BEZEL_LEFT || bezel == BEZEL_RIGHT);
+  // Convert location from window coordinates to screen coordinates.
+  gfx::Point point_in_screen(location.x(), location.y());

[Jun Mukai, 2014/07/25 20:21:29]

point_in_screen(location) would copy it.

[mfomitchev, 2014/07/25 23:27:38]

Doesn't look like I can pass PointF to a Point constructor...

[Jun Mukai, 2014/07/25 23:44:23]

oops, missed your point. Well, in that case, consider using a function in
ui/gfx/geometry/pointer_conversions.h

[mfomitchev, 2014/07/26 02:08:43]

Cool, will do.

[mfomitchev, 2014/08/05 19:56:57]

Done.

+  ConvertPointToScreen(window, &point_in_screen);
   return bezel == BEZEL_LEFT
-             ? position.x()
-             : position.x() - container_->GetBoundsInScreen().width();
+             ? point_in_screen.x()
+             : point_in_screen.x() - container_->GetBoundsInScreen().width();

[Jun Mukai, 2014/07/25 20:21:29]

Not the thing in this CL, but should this be GetBoundsInScreen().right()
instead?  And the previous line should be x() - GetBoundsInScreen().x()?

The container is spanning fullscreen so this makes no difference for now, but I
think that is more strict.

[mfomitchev, 2014/07/25 23:27:38]

Since this is a bezel gesture recognizer, I think  point_in_screen.x() is right,
but instead of container_->GetBoundsInScreen().width() we want to get the screen
width is not taking up the entire screen. Does this make sense? And if so -
what's a good way to get the screen width?

[Jun Mukai, 2014/07/25 23:44:23]

I don't object to point_in_screen.x(). I'm saying to use right() instead of
width(), and use x() instead of doing nothing, about
container_->GetBoundsInScreen().

That addresses the case that the container's rectangle (i.e. user's work area)
is different from the display/screen's bounds. Although right now I don't hear
any plan of doing so, it might happen eventually.

For example, ash work area originally used the entire screen width, but at some
point we introduced side shelf and docked window which shrinks work area's
width, and suffered from weird corner cases caused by implicit assumptions of
'work area width must match to the screen width'. It's better to address this
kind of thing earlier.

[mfomitchev, 2014/07/26 02:08:43]

Yes, I understand. My point is that this is a bezel controller, which is
supposed to recognize bezel gestures. Bezel gestures by definition are supposed
to start from the edge of the screen, so I think it makes sense to report the
distance from the edge of the screen in the callbacks as well. Now, the current
implementation is still wrong: the first clause (point_in_screen.x()) is okay,
but the second clause (point_in_screen.x() -
container_->GetBoundsInScreen().width()) should be changed to
(point_in_screen.x() - <screen width>).

If we want to account for the possibility that the main container is not full
screen, I think we should do it in SplitViewController: we should take
container_->x() and container_->right() into account when calculating the
windows positions. I can make that change.

[Jun Mukai, 2014/07/26 20:30:37]

+1 to use screen_width.

[mfomitchev, 2014/08/05 19:56:57]

Done.

 }
 
-void BezelController::SetState(BezelController::State state,
-                               const gfx::PointF& scroll_position) {
+void BezelController::SetState(
+    BezelController::State state, float scroll_delta) {
   if (!left_right_delegate_ || state == state_)
     return;
 
-  if (state == BEZEL_SCROLLING_TWO_FINGERS) {
-    float delta = GetDistance(scroll_position, scroll_bezel_);
-    left_right_delegate_->ScrollBegin(scroll_bezel_, delta);
-  } else if (state_ == BEZEL_SCROLLING_TWO_FINGERS) {
-    left_right_delegate_->ScrollEnd();
-  }
+  LOG(ERROR) << "\nBezelController::SetState, state=*** " << state << " ***";
+
+      if (state == BEZEL_SCROLLING_TWO_FINGERS) {
+        left_right_delegate_->ScrollBegin(scroll_bezel_, scroll_delta);
+      } else if (state_ == BEZEL_SCROLLING_TWO_FINGERS) {
+        left_right_delegate_->ScrollEnd();
+      }

[Jun Mukai, 2014/07/25 20:21:29]

remove unnecessary indents

[mfomitchev, 2014/07/25 23:27:38]

Done.

   state_ = state;
   if (state == NONE) {
     scroll_bezel_ = BEZEL_NONE;
     scroll_target_ = NULL;
   }
 }
 
 // Only implemented for LEFT and RIGHT bezels ATM.
 BezelController::Bezel BezelController::GetBezel(const gfx::PointF& location) {
   if (location.x() < kBezelWidth) {
@@ skipping 19 matching lines @@
   ui::EventType type = event->type();
   if (!ShouldProcessGesture(type))
     return;
 
   if (scroll_target_ && event->target() != scroll_target_)
     return;
 
   const gfx::PointF& event_location = event->location_f();
   const ui::GestureEventDetails& event_details = event->details();
   int num_touch_points = event_details.touch_points();
+  float scroll_delta = kScrollDeltaNone;
+  if (scroll_bezel_ != BEZEL_NONE) {
+    aura::Window* target_window = static_cast<aura::Window*> (event->target());
+    scroll_delta = GetDistance(
+        event_location, target_window, scroll_bezel_);
+  }
 
   if (type == ui::ET_GESTURE_BEGIN) {
     if (num_touch_points > 2) {
-      SetState(IGNORE_CURRENT_SCROLL,
-               gfx::Point(kScrollPositionNone, kScrollPositionNone));
+      SetState(IGNORE_CURRENT_SCROLL, kScrollDeltaNone);
       return;
     }
     BezelController::Bezel event_bezel = GetBezel(event->location_f());
     switch (state_) {
       case NONE:
         scroll_bezel_ = event_bezel;
         scroll_target_ = event->target();
-        if (event_bezel != BEZEL_LEFT && event_bezel != BEZEL_RIGHT) {
-          SetState(IGNORE_CURRENT_SCROLL,
-                   gfx::Point(kScrollPositionNone, kScrollPositionNone));
-        } else {
-          SetState(BEZEL_GESTURE_STARTED, event_location);
-        }
+        if (event_bezel != BEZEL_LEFT && event_bezel != BEZEL_RIGHT)
+          SetState(IGNORE_CURRENT_SCROLL, kScrollDeltaNone);
+        else
+          SetState(BEZEL_GESTURE_STARTED, kScrollDeltaNone);

[Jun Mukai, 2014/07/25 20:21:29]

I don't like using None all around. Can we make a struct for the state, which
has the enum and the distance.  Then declare two struct constructors: just
receiving enum (distance is None), and receiving enum and distance.

[mfomitchev, 2014/08/05 19:56:57]

I've just created an overloaded SetState method that takes one argument and
added a DCHECK there - this seems simpler and requires less changes to the
header. Let me know if you are ok with this.

[Jun Mukai, 2014/08/05 21:13:38]

SGTM

         break;
       case IGNORE_CURRENT_SCROLL:
         break;
       case BEZEL_GESTURE_STARTED:
       case BEZEL_SCROLLING_ONE_FINGER:
         DCHECK_EQ(num_touch_points, 2);
         DCHECK(scroll_target_);
         DCHECK_NE(scroll_bezel_, BEZEL_NONE);
 
         if (event_bezel != scroll_bezel_) {
-          SetState(IGNORE_CURRENT_SCROLL,
-                   gfx::Point(kScrollPositionNone, kScrollPositionNone));
+          SetState(IGNORE_CURRENT_SCROLL, kScrollDeltaNone);
           return;
         }
         if (state_ == BEZEL_SCROLLING_ONE_FINGER)
-          SetState(BEZEL_SCROLLING_TWO_FINGERS, event_location);
+          SetState(BEZEL_SCROLLING_TWO_FINGERS, scroll_delta);
         break;
       case BEZEL_SCROLLING_TWO_FINGERS:
         // Should've exited above
         NOTREACHED();
         break;
     }
   } else if (type == ui::ET_GESTURE_END) {
     if (state_ == NONE)
       return;
 
     CHECK(scroll_target_);
     if (num_touch_points == 1) {
-      SetState(NONE, gfx::Point(kScrollPositionNone, kScrollPositionNone));
+      SetState(NONE, kScrollDeltaNone);
     } else {
-      SetState(IGNORE_CURRENT_SCROLL,
-               gfx::Point(kScrollPositionNone, kScrollPositionNone));
+      SetState(IGNORE_CURRENT_SCROLL, kScrollDeltaNone);
     }
   } else if (type == ui::ET_GESTURE_SCROLL_BEGIN) {
     DCHECK(state_ == IGNORE_CURRENT_SCROLL || state_ == BEZEL_GESTURE_STARTED);
     if (state_ != BEZEL_GESTURE_STARTED)
       return;
 
     if (num_touch_points == 1) {
-      SetState(BEZEL_SCROLLING_ONE_FINGER, event_location);
+      SetState(BEZEL_SCROLLING_ONE_FINGER, scroll_delta);
       return;
     }
 
     DCHECK_EQ(num_touch_points, 2);
-    SetState(BEZEL_SCROLLING_TWO_FINGERS, event_location);
+    SetState(BEZEL_SCROLLING_TWO_FINGERS, scroll_delta);
     if (left_right_delegate_->CanScroll())
       event->SetHandled();
   } else if (type == ui::ET_GESTURE_SCROLL_UPDATE) {
     if (state_ != BEZEL_SCROLLING_TWO_FINGERS)
       return;
 
-    float scroll_delta = GetDistance(event_location, scroll_bezel_);
     left_right_delegate_->ScrollUpdate(scroll_delta);
     if (left_right_delegate_->CanScroll())
       event->SetHandled();
   }
 }
 
 }  // namespace athena