exo: Fix multi-display hardware cursor

components/exo/pointer.cc

Index: components/exo/pointer.cc
diff --git a/components/exo/pointer.cc b/components/exo/pointer.cc
index 35d1e77dfdd2999d9a937e08892cdd30a96665e2..904a4bc231317355a957f790109f0cebaee24eba 100644
--- a/components/exo/pointer.cc
+++ b/components/exo/pointer.cc
@@ -19,7 +19,7 @@
 #include "ui/display/screen.h"
 #include "ui/events/event.h"
 #include "ui/gfx/geometry/vector2d_conversions.h"
-#include "ui/gfx/transform_util.h"
+#include "ui/gfx/transform.h"
 
 #if defined(USE_OZONE)
 #include "ui/ozone/public/cursor_factory_ozone.h"
@@ -32,7 +32,13 @@
 namespace exo {
 namespace {
 
-const float kLargeCursorScale = 2.8f;
+// TODO(oshima): Some accessibility features, including large cursors, disable
+// hardware cursors. Ash does not support compositing for custom cursors, so it
+// replaces them with the default cursor. As a result, this scale has no effect
+// for now. See crbug.com/708378.
+float GetCursorScale(ui::CursorSetType cursor_set) {
+  return cursor_set == ui::CURSOR_SET_LARGE ? 2.8f : 1.0f;
+}
 
 // Synthesized events typically lack floating point precision so to avoid
 // generating mouse event jitter we consider the location of these events
@@ -51,12 +57,19 @@ bool SameLocation(const ui::LocatedEvent* event, const gfx::PointF& location) {
 
 Pointer::Pointer(PointerDelegate* delegate)
     : delegate_(delegate),
-      cursor_(ui::kCursorNull),
+      cursor_(new aura::Window(nullptr)),
       cursor_capture_source_id_(base::UnguessableToken::Create()),
       cursor_capture_weak_ptr_factory_(this) {
   auto* helper = WMHelper::GetInstance();
   helper->AddPreTargetHandler(this);
   helper->AddCursorObserver(this);
+
+  cursor_scale_ = GetCursorScale(helper->GetCursorSet());
+
+  cursor_->SetName("ExoCursor");
+  cursor_->Init(ui::LAYER_NOT_DRAWN);
+  cursor_->set_owned_by_parent(false);
+  OnCursorDisplayChanging(display::Screen::GetScreen()->GetPrimaryDisplay());
 }
 
 Pointer::~Pointer() {
@@ -88,10 +101,7 @@ void Pointer::SetCursor(Surface* surface, const gfx::Point& hotspot) {
       return;
     }
     if (surface_) {
-      surface_->window()->SetTransform(gfx::Transform());
-      WMHelper::GetInstance()
-          ->GetContainer(ash::kShellWindowId_MouseCursorContainer)
-          ->RemoveChild(surface_->window());
+      cursor_->RemoveChild(surface_->window());
       surface_->SetSurfaceDelegate(nullptr);
       surface_->RemoveSurfaceObserver(this);
     }
@@ -99,12 +109,8 @@ void Pointer::SetCursor(Surface* surface, const gfx::Point& hotspot) {
     if (surface_) {
       surface_->SetSurfaceDelegate(this);
       surface_->AddSurfaceObserver(this);
-      // Note: Surface window needs to be added to the tree so we can take a
-      // snapshot. Where in the tree is not important but we might as well use
-      // the cursor container.
-      WMHelper::GetInstance()
-          ->GetContainer(ash::kShellWindowId_MouseCursorContainer)
-          ->AddChild(surface_->window());
+      cursor_->AddChild(surface_->window());
+      surface_->window()->Show();
     }
     cursor_changed = true;
   }
@@ -125,13 +131,17 @@ void Pointer::SetCursor(Surface* surface, const gfx::Point& hotspot) {
     CaptureCursor();
   } else {
     cursor_capture_weak_ptr_factory_.InvalidateWeakPtrs();
-    cursor_ = ui::kCursorNone;
-    UpdateCursor();
+    UpdateCursor(ui::kCursorNone);
   }
 }
 
 gfx::NativeCursor Pointer::GetCursor() {
-  return cursor_;
+  if (focus_)
+    if (auto* root_window = focus_->window()->GetRootWindow())
+      if (auto* cursor_client = aura::client::GetCursorClient(root_window))
+        return cursor_client->GetCursor();
+
+  return ui::kCursorNull;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -151,7 +161,6 @@ void Pointer::OnMouseEvent(ui::MouseEvent* event) {
       // response to each OnPointerEnter() call.
       focus_->UnregisterCursorProvider(this);
       focus_ = nullptr;
-      cursor_ = ui::kCursorNull;
       cursor_capture_weak_ptr_factory_.InvalidateWeakPtrs();
     }
     // Second generate an enter event if focus moved to a new target.
@@ -226,8 +235,6 @@ void Pointer::OnMouseEvent(ui::MouseEvent* event) {
       NOTREACHED();
       break;
   }
-
-  UpdateCursorScale();
 }
 
 void Pointer::OnScrollEvent(ui::ScrollEvent* event) {
@@ -238,8 +245,26 @@ void Pointer::OnScrollEvent(ui::ScrollEvent* event) {
 // WMHelper::CursorObserver overrides:
 
 void Pointer::OnCursorSetChanged(ui::CursorSetType cursor_set) {
-  if (focus_)
-    UpdateCursorScale();
+  cursor_scale_ = GetCursorScale(cursor_set);
+  if (focus_ && surface_)
+    CaptureCursor();
+}
+
+void Pointer::OnCursorDisplayChanging(const display::Display& display) {
+  WMHelper* helper = WMHelper::GetInstance();
+  aura::Window* container = helper->GetContainer(
+      display.id(), ash::kShellWindowId_MouseCursorContainer);
+
+  // Reparent the cursor to the root window where the mouse is located.

[reveman, 2017/04/10 21:47:13]

is this re-parenting really needed?

[Dominik Laskowski, 2017/04/11 04:42:55]

Yes. Layer size is in DIPs, which depends on the DSF of the display mode. The
scaling transform is relative to the display where the mouse is located, so the
layer must be parented to the corresponding root window. Also, the layer must be
reparented when its parent root window is destroyed because the display is
removed.

[reveman, 2017/04/11 19:25:13]

Ok. Can we parent the cursor surface at the first SetCursor after an enter event
and make sure display changes generate leave/enter events. I think that would
keep all this multi display logic really simple.

[Dominik Laskowski, 2017/04/11 20:40:37]

That would break client-side dragging, i.e. we would need hacks to prevent the
window from being dropped when dragged across a display boundary.

[reveman, 2017/04/11 22:25:34]

Why would it break client side dragging? The compositors often need to generate
enter/leave events from internal state changes so being able to handle this
correctly on the client side is important I think.

+  if (container->GetRootWindow() != cursor_->GetRootWindow()) {
+    if (cursor_->parent())
+      cursor_->parent()->RemoveChild(cursor_.get());
+    container->AddChild(cursor_.get());
+  }
+
+  auto info = helper->GetDisplayInfo(display.id());
+  display_scale_ = info.GetEffectiveUIScale() * info.device_scale_factor();
+  device_scale_factor_ = display.device_scale_factor();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -283,64 +308,25 @@ Surface* Pointer::GetEffectiveTargetForEvent(ui::Event* event) const {
   return delegate_->CanAcceptPointerEventsForSurface(target) ? target : nullptr;
 }
 
-void Pointer::UpdateCursorScale() {
-  DCHECK(focus_);
-
-  display::Screen* screen = display::Screen::GetScreen();
-  WMHelper* helper = WMHelper::GetInstance();
-
-  // Update cursor scale if the effective UI scale has changed.
-  display::Display display = screen->GetDisplayNearestWindow(focus_->window());
-  float scale = helper->GetDisplayInfo(display.id()).GetEffectiveUIScale();
-
-  if (display::Display::HasInternalDisplay()) {
-    float primary_device_scale_factor =
-        screen->GetPrimaryDisplay().device_scale_factor();
-    // The size of the cursor surface is the quotient of its physical size and
-    // the DSF of the primary display. The physical size is proportional to the
-    // DSF of the internal display. For external displays (and the internal
-    // display when secondary to a display with a different DSF), scale the
-    // cursor so its physical size matches with the single display case.
-    if (!display.IsInternal() ||
-        display.device_scale_factor() != primary_device_scale_factor) {
-      scale *= primary_device_scale_factor /
-               helper->GetDisplayInfo(display::Display::InternalDisplayId())
-                   .device_scale_factor();
-    }
-  }
-
-  if (helper->GetCursorSet() == ui::CURSOR_SET_LARGE)
-    scale *= kLargeCursorScale;
-
-  if (scale != cursor_scale_) {
-    cursor_scale_ = scale;
-    if (surface_)
-      CaptureCursor();
-  }
-}
-
 void Pointer::CaptureCursor() {
   DCHECK(surface_);
   DCHECK(focus_);
 
-  // Set UI scale before submitting capture request.
-  surface_->window()->layer()->SetTransform(
-      gfx::GetScaleTransform(gfx::Point(), cursor_scale_));
+  float scale = cursor_scale_ * display_scale_;
+  float layer_scale = scale / device_scale_factor_;
 
-  float primary_device_scale_factor =
-      display::Screen::GetScreen()->GetPrimaryDisplay().device_scale_factor();
+  gfx::Transform transform;
+  transform.Scale(layer_scale, layer_scale);
+  surface_->window()->SetTransform(transform);
 
   std::unique_ptr<cc::CopyOutputRequest> request =
       cc::CopyOutputRequest::CreateBitmapRequest(
           base::Bind(&Pointer::OnCursorCaptured,
                      cursor_capture_weak_ptr_factory_.GetWeakPtr(),
-                     gfx::ScaleToFlooredPoint(
-                         hotspot_,
-                         // |hotspot_| is in surface coordinate space so apply
-                         // both device scale and UI scale.
-                         cursor_scale_ * primary_device_scale_factor)));
+                     gfx::ScaleToFlooredPoint(hotspot_, scale)));
+
   request->set_source(cursor_capture_source_id_);
-  surface_->window()->layer()->RequestCopyOfOutput(std::move(request));
+  cursor_->layer()->RequestCopyOfOutput(std::move(request));
 }
 
 void Pointer::OnCursorCaptured(const gfx::Point& hotspot,
@@ -348,7 +334,7 @@ void Pointer::OnCursorCaptured(const gfx::Point& hotspot,
   if (!focus_)
     return;
 
-  cursor_ = ui::kCursorNone;
+  gfx::NativeCursor cursor = ui::kCursorNull;
   if (!result->IsEmpty()) {
     DCHECK(result->HasBitmap());
     std::unique_ptr<SkBitmap> bitmap = result->TakeBitmap();
@@ -363,8 +349,8 @@ void Pointer::OnCursorCaptured(const gfx::Point& hotspot,
     XcursorImage* image = ui::SkBitmapToXcursorImage(bitmap.get(), hotspot);
     platform_cursor = ui::CreateReffedCustomXCursor(image);
 #endif
-    cursor_ = ui::kCursorCustom;
-    cursor_.SetPlatformCursor(platform_cursor);
+    cursor = ui::kCursorCustom;
+    cursor.SetPlatformCursor(platform_cursor);
 #if defined(USE_OZONE)
     ui::CursorFactoryOzone::GetInstance()->UnrefImageCursor(platform_cursor);
 #elif defined(USE_X11)
@@ -372,10 +358,10 @@ void Pointer::OnCursorCaptured(const gfx::Point& hotspot,
 #endif
   }
 
-  UpdateCursor();
+  UpdateCursor(cursor);
 }
 
-void Pointer::UpdateCursor() {
+void Pointer::UpdateCursor(gfx::NativeCursor cursor) {
   DCHECK(focus_);
 
   aura::Window* root_window = focus_->window()->GetRootWindow();
@@ -385,7 +371,7 @@ void Pointer::UpdateCursor() {
   aura::client::CursorClient* cursor_client =
       aura::client::GetCursorClient(root_window);
   if (cursor_client)
-    cursor_client->SetCursor(cursor_);
+    cursor_client->SetCursor(cursor);
 }
 
 }  // namespace exo