Keep root_location to be in pixels and display coordinates in WS.

services/ui/ws/window_manager_state.cc

 // Copyright 2016 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 "services/ui/ws/window_manager_state.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "services/service_manager/public/interfaces/connector.mojom.h"
 #include "services/ui/common/accelerator_util.h"
 #include "services/ui/ws/accelerator.h"
 #include "services/ui/ws/cursor_location_manager.h"
 #include "services/ui/ws/display.h"
 #include "services/ui/ws/display_manager.h"
 #include "services/ui/ws/platform_display.h"
 #include "services/ui/ws/server_window.h"
 #include "services/ui/ws/user_display_manager.h"
 #include "services/ui/ws/user_id_tracker.h"
 #include "services/ui/ws/window_manager_display_root.h"
 #include "services/ui/ws/window_server.h"
 #include "services/ui/ws/window_tree.h"
 #include "ui/events/event.h"
+#include "ui/gfx/geometry/dip_util.h"
 
 namespace ui {
 namespace ws {
 namespace {
 
 // Flags that matter when checking if a key event matches an accelerator.
 const int kAcceleratorEventFlags =
     EF_SHIFT_DOWN | EF_CONTROL_DOWN | EF_ALT_DOWN | EF_COMMAND_DOWN;
 
 base::TimeDelta GetDefaultAckTimerDelay() {
@@ skipping 231 matching lines @@
     if (display_root_ptr->root()->id() == id)
       return display_root_ptr->root();
   }
   return nullptr;
 }
 
 bool WindowManagerState::IsActive() const {
   return window_server()->user_id_tracker()->active_id() == user_id();
 }
 
-void WindowManagerState::Activate(const gfx::Point& mouse_location_on_screen) {
+void WindowManagerState::Activate(const gfx::Point& mouse_location_on_display,
+                                  const int64_t display_id) {
   SetAllRootWindowsVisible(true);
   event_dispatcher_.Reset();
-  event_dispatcher_.SetMousePointerScreenLocation(mouse_location_on_screen);
+  event_dispatcher_.SetMousePointerDisplayLocationAndId(
+      mouse_location_on_display, display_id);
 }
 
 void WindowManagerState::Deactivate() {
   SetAllRootWindowsVisible(false);
   event_dispatcher_.Reset();
   // The tree is no longer active, so no point in dispatching any further
   // events.
   std::queue<std::unique_ptr<QueuedEvent>> event_queue;
   event_queue.swap(event_queue_);
 }
@@ skipping 26 matching lines @@
 
   std::unique_ptr<InFlightEventDetails> details =
       std::move(in_flight_event_details_);
 
   if (result == mojom::EventResult::UNHANDLED) {
     DCHECK(details->event->IsKeyEvent());
     if (!properties.empty())
       details->event->AsKeyEvent()->SetProperties(properties);
     event_processing_display_id_ = details->display_id;
     event_dispatcher_.ProcessEvent(
-        *details->event, EventDispatcher::AcceleratorMatchPhase::POST_ONLY);
+        *details->event, event_processing_display_id_,
+        EventDispatcher::AcceleratorMatchPhase::POST_ONLY);
   } else {
     // We're not going to process the event any further, notify event observers.
     // We don't do this first to ensure we don't send an event twice to clients.
     window_server()->SendToPointerWatchers(*details->event, user_id(), nullptr,
                                            nullptr, details->display_id);
     ProcessNextEventFromQueue();
   }
 }
 
 const WindowServer* WindowManagerState::window_server() const {
@@ skipping 75 matching lines @@
   } else {
     OnEventAck(in_flight_event_details_->tree, mojom::EventResult::UNHANDLED);
   }
 }
 
 void WindowManagerState::ProcessEventImpl(const ui::Event& event,
                                           int64_t display_id) {
   // Debug accelerators are always checked and don't interfere with processing.
   ProcessDebugAccelerator(event);
   event_processing_display_id_ = display_id;
-  event_dispatcher_.ProcessEvent(event,
+  event_dispatcher_.ProcessEvent(event, event_processing_display_id_,
                                  EventDispatcher::AcceleratorMatchPhase::ANY);
 }
 
 void WindowManagerState::QueueEvent(
     const ui::Event& event,
     std::unique_ptr<ProcessedEventTarget> processed_event_target,
     int64_t display_id) {
   std::unique_ptr<QueuedEvent> queued_event(new QueuedEvent);
   queued_event->event = ui::Event::Clone(event);
   queued_event->processed_target = std::move(processed_event_target);
@@ skipping 100 matching lines @@
   const base::TimeDelta max_delay = base::debug::BeingDebugged()
                                         ? base::TimeDelta::FromDays(1)
                                         : GetDefaultAckTimerDelay();
   details->timer.Start(
       FROM_HERE, max_delay,
       base::Bind(&WindowManagerState::OnEventAckTimeout,
                  details->weak_factory.GetWeakPtr(), tree->id()));
   in_flight_event_details_ = std::move(details);
 }
 
+bool WindowManagerState::ConvertPointToScreen(gfx::Point* point,
+                                              const int64_t display_id) {
+  Display* display = display_manager()->GetDisplayById(display_id);
+  if (display) {
+    const display::Display& originated_display = display->GetDisplay();
+    *point = gfx::ConvertPointToDIP(originated_display.device_scale_factor(),
+                                    *point);
+    *point += originated_display.bounds().origin().OffsetFromOrigin();
+    return true;
+  }
+  return false;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // EventDispatcherDelegate:
 
 void WindowManagerState::OnAccelerator(uint32_t accelerator_id,
                                        const ui::Event& event,
                                        AcceleratorPhase phase) {
   DCHECK(IsActive());
   const bool needs_ack = phase == AcceleratorPhase::PRE;
   WindowTree::AcceleratorCallback ack_callback;
   if (needs_ack) {
@@ skipping 51 matching lines @@
 void WindowManagerState::UpdateNativeCursorFromDispatcher() {
   const ui::CursorData cursor = event_dispatcher_.GetCurrentMouseCursor();
   cursor_state_.SetCurrentWindowCursor(cursor);
 }
 
 void WindowManagerState::OnCaptureChanged(ServerWindow* new_capture,
                                           ServerWindow* old_capture) {
   window_server()->ProcessCaptureChanged(new_capture, old_capture);
 }
 
-void WindowManagerState::OnMouseCursorLocationChanged(const gfx::Point& point) {
+void WindowManagerState::OnMouseCursorLocationChanged(
+    const gfx::Point& point_in_display,
+    const int64_t display_id) {
+  gfx::Point point_in_screen(point_in_display);
+  ConvertPointToScreen(&point_in_screen, display_id);
   window_server()
       ->display_manager()
       ->GetCursorLocationManager(user_id())
-      ->OnMouseCursorLocationChanged(point);
+      ->OnMouseCursorLocationChanged(point_in_screen);

[sky, 2017/05/17 16:53:42]

Won't this lead to rounding errors? By that I mean should this be in float? Is
the client code expecting DIPs right now? Please also update the documentation
for CursorLocationManager to say it's in dips.

[sky, 2017/05/17 19:02:07]

Don't worry about the rounding errors for now. We can deal with that later.

[riajiang, 2017/05/19 20:45:05]

This won't introduce new rounding error? We were also converting from display
coordinates to screen coordinates before for the event root_location which is
where mouse cursor location comes from?

CursorLocationManager is expecting screen-coord and DIP (updated comment)
because this OnMouseCursorLocationChanged was taking in mouse cursor location in
screen-coord and DIP. 

[sky, 2017/05/19 22:12:19]

Before we were setting the location to pixels, so no rounding errors. Once you
convert to DIPs as ints then it is impossible for client code that sees the dip
location to convert back to pixels and keep the precision. But this can be dealt
with separately.

 }
 
 void WindowManagerState::DispatchInputEventToWindow(ServerWindow* target,
                                                     ClientSpecificId client_id,
                                                     const ui::Event& event,
                                                     Accelerator* accelerator) {
   DCHECK(IsActive());
   // TODO(sky): this needs to see if another wms has capture and if so forward
   // to it.
   if (in_flight_event_details_) {
@@ skipping 32 matching lines @@
   while (tree && tree->embedder_intercepts_events()) {
     DCHECK(tree->HasRoot(embed_root));
     tree = window_server()->GetTreeWithId(embed_root->id().client_id);
     embed_root = GetEmbedRoot(embed_root);
   }
   DCHECK(tree);
   return tree->id();
 }
 
 ServerWindow* WindowManagerState::GetRootWindowContaining(
-    gfx::Point* location) {
+    gfx::Point* location_in_display,
+    int64_t* display_id) {

[sky, 2017/05/17 16:53:42]

I'm still confused about this. I thought all events originate with a display_id
and this code shouldn't need to figure it out?

[riajiang, 2017/05/19 20:45:05]

For most events the display id for the |target_display_root| we found would be
the same as the |display_id| param. But if it's a drag-and-drop, the mouse move
event would be relative to the first display even when we have moved the mouse
to the second display. In that case, after this function call,
|location_in_display| and |display_id| would be updated to be relative to the
second display. Am I misunderstanding sth?

[sky, 2017/05/19 22:12:19]

Are you *sure* about this? Again, I believe events always originate from the
display the cursor is on. Even during drag and drop. My understanding is that
the only way for the cursor onto a different display is via warp. Have you tried
this on an actual device? If running on the desktop produces different behavior
I would say we should make the desktop give the same behavior so we don't need
to special case it. Sadrul knows more than I in this area, so I encourage you to
talk with him about how to handle this.

   if (window_manager_display_roots_.empty())
     return nullptr;
 
-  // TODO(riajiang): This is broken for HDPI because it mixes PPs and DIPs. See
-  // http://crbug.com/701036 for details.
   WindowManagerDisplayRoot* target_display_root = nullptr;
-  for (auto& display_root_ptr : window_manager_display_roots_) {
-    if (display_root_ptr->display()->GetDisplay().bounds().Contains(
-            *location)) {
-      target_display_root = display_root_ptr.get();
-      break;
+  gfx::Point location_in_screen(*location_in_display);
+  if (ConvertPointToScreen(&location_in_screen, *display_id)) {
+    for (auto& display_root_ptr : window_manager_display_roots_) {
+      if (display_root_ptr->display()->GetDisplay().bounds().Contains(
+              location_in_screen)) {
+        target_display_root = display_root_ptr.get();
+        break;
+      }
     }
   }
 
   // TODO(kylechar): Better handle locations outside the window. Overlapping X11
   // windows, dragging and touch sensors need to be handled properly.
   if (!target_display_root) {
-    DVLOG(1) << "Invalid event location " << location->ToString();
+    DVLOG(1) << "Invalid event location " << location_in_display->ToString()
+             << " / display id " << *display_id;
     target_display_root = window_manager_display_roots_.begin()->get();
   }
 
-  // Translate the location to be relative to the display instead of relative
-  // to the screen space.
-  gfx::Point origin =
-      target_display_root->display()->GetDisplay().bounds().origin();
-  *location -= origin.OffsetFromOrigin();
+  // Update |location_in_display| and |display_id| if the target display is
+  // different from the originated display, e.g. drag-and-drop.
+  if (*display_id != target_display_root->display()->GetId()) {
+    gfx::Point origin =
+        target_display_root->display()->GetDisplay().bounds().origin();
+    *location_in_display = location_in_screen - origin.OffsetFromOrigin();
+    *location_in_display = gfx::ConvertPointToPixel(
+        target_display_root->display()->GetDisplay().device_scale_factor(),
+        *location_in_display);
+    *display_id = target_display_root->display()->GetId();
+  }
+
   return target_display_root->GetClientVisibleRoot();
 }
 
 void WindowManagerState::OnEventTargetNotFound(const ui::Event& event) {
   window_server()->SendToPointerWatchers(event, user_id(), nullptr, /* window */
                                          nullptr /* ignore_tree */,
                                          event_processing_display_id_);
   if (event.IsMousePointerEvent())
     UpdateNativeCursorFromDispatcher();
 }
 
 void WindowManagerState::OnWindowEmbeddedAppDisconnected(ServerWindow* window) {
   for (auto iter = orphaned_window_manager_display_roots_.begin();
        iter != orphaned_window_manager_display_roots_.end(); ++iter) {
     if ((*iter)->root() == window) {
       window->RemoveObserver(this);
       orphaned_window_manager_display_roots_.erase(iter);
       return;
     }
   }
   NOTREACHED();
 }
 
 }  // namespace ws
 }  // namespace ui