exo: Fix multi-display hardware cursor

components/exo/pointer.cc

 // Copyright 2015 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 "components/exo/pointer.h"
 
 #include "ash/public/cpp/shell_window_ids.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/output/copy_output_result.h"
 #include "components/exo/pointer_delegate.h"
 #include "components/exo/pointer_stylus_delegate.h"
 #include "components/exo/surface.h"
 #include "components/exo/wm_helper.h"
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/manager/managed_display_info.h"
 #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"
 #endif
 
 #if defined(USE_X11)
 #include "ui/base/cursor/cursor_loader_x11.h"
 #endif
 
 namespace exo {
 namespace {
 
-const float kLargeCursorScale = 2.8f;
+// TODO: Some accessibility features, including large cursors, disable hardware

[reveman, 2017/03/30 07:47:53]

nit: TODO(name) and please add a bug number

[Dominik Laskowski, 2017/04/05 17:59:25]

Done. TODO(oshima) to be consistent with:

https://chromium.googlesource.com/chromium/src/+/master/ash/wm/ash_native_cur...

+// 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.
+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
 // to be the same as |location| if floored values match.
 bool SameLocation(const ui::LocatedEvent* event, const gfx::PointF& location) {
   if (event->flags() & ui::EF_IS_SYNTHESIZED)
     return event->location() == gfx::ToFlooredPoint(location);
 
   return event->location_f() == location;
 }
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // Pointer, public:
 
 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() {
   delegate_->OnPointerDestroying(this);
   if (surface_)
     surface_->RemoveSurfaceObserver(this);
   if (focus_) {
     focus_->RemoveSurfaceObserver(this);
     focus_->UnregisterCursorProvider(this);
   }
@@ skipping 11 matching lines @@
   bool cursor_changed = false;
 
   // If surface is different than the current pointer surface then remove the
   // current surface and add the new surface.
   if (surface != surface_) {
     if (surface && surface->HasSurfaceDelegate()) {
       DLOG(ERROR) << "Surface has already been assigned a role";
       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);
     }
     surface_ = surface;
     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;
   }
 
   // Update hotspot.
   if (hotspot != hotspot_) {
     hotspot_ = hotspot;
     cursor_changed = true;
   }
 
   // Early out if cursor did not change.
   if (!cursor_changed)
     return;
 
   // If |surface_| is set then asynchronously capture a snapshot of cursor,
   // otherwise cancel pending capture and immediately set the cursor to "none".
   if (surface_) {
-    CaptureCursor();
+    CaptureCursor(false);
   } else {
     cursor_capture_weak_ptr_factory_.InvalidateWeakPtrs();
-    cursor_ = ui::kCursorNone;
-    UpdateCursor();
+    UpdateCursor(ui::kCursorNone, false);
   }
 }
 
 gfx::NativeCursor Pointer::GetCursor() {
-  return cursor_;

[reveman, 2017/03/30 07:47:53]

why does all this need to change? I think the previous logic was easy to
understand.

[Dominik Laskowski, 2017/04/05 17:59:25]

Keeping track of the latest cursor is unnecessary, i.e. CursorClient::GetCursor
does that already.

+  if (!focus_)
+    return ui::kCursorNull;

[reveman, 2017/03/30 07:47:53]

I think returning null cursor in this case is incorrect. It's critical that the
cursor is not changing when entering a new window until the client has had a
chance to replace it.

[Dominik Laskowski, 2017/04/05 17:59:25]

This is equivalent to the old code. Also, the cursor is captured on mouse enter
below, so the null cursor (i.e. the default pointer) would only appear while the
capture is pending.

+
+  aura::client::CursorClient* cursor_client =
+      aura::client::GetCursorClient(focus_->window()->GetRootWindow());
+  return cursor_client ? cursor_client->GetCursor() : ui::kCursorNull;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ui::EventHandler overrides:
 
 void Pointer::OnMouseEvent(ui::MouseEvent* event) {
   Surface* target = GetEffectiveTargetForEvent(event);
 
   // If target is different than the current pointer focus then we need to
   // generate enter and leave events.
   if (target != focus_) {
     // First generate a leave event if we currently have a target in focus.
     if (focus_) {
       delegate_->OnPointerLeave(focus_);
       focus_->RemoveSurfaceObserver(this);
       // Require SetCursor() to be called and cursor to be re-defined in
       // 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.
     if (target) {
       delegate_->OnPointerEnter(target, event->location_f(),
                                 event->button_flags());
       location_ = event->location_f();
       focus_ = target;
       focus_->AddSurfaceObserver(this);
       focus_->RegisterCursorProvider(this);
+
+      if (surface_)

[reveman, 2017/03/30 07:47:53]

this is incorrect I think. the client needs to provide a new cursor surface for
each enter event.

[Dominik Laskowski, 2017/04/05 17:59:25]

The client does not do that currently. Is that required by the spec?

+        CaptureCursor(false);
     }
     delegate_->OnPointerFrame();
   }
 
   if (!focus_)
     return;
 
   if (event->IsMouseEvent() && event->type() != ui::ET_MOUSE_EXITED) {
     // Generate motion event if location changed. We need to check location
     // here as mouse movement can generate both "moved" and "entered" events
@@ skipping 44 matching lines @@
     case ui::ET_MOUSE_MOVED:
     case ui::ET_MOUSE_DRAGGED:
     case ui::ET_MOUSE_ENTERED:
     case ui::ET_MOUSE_EXITED:
     case ui::ET_MOUSE_CAPTURE_CHANGED:
       break;
     default:
       NOTREACHED();
       break;
   }
-
-  UpdateCursorScale();
 }
 
 void Pointer::OnScrollEvent(ui::ScrollEvent* event) {
   OnMouseEvent(event);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // WMHelper::CursorObserver overrides:
 
 void Pointer::OnCursorSetChanged(ui::CursorSetType cursor_set) {
-  if (focus_)
-    UpdateCursorScale();
+  cursor_scale_ = GetCursorScale(cursor_set);
+  if (focus_ && surface_)
+    CaptureCursor(false);
+}
+
+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.
+  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();
+
+  if (focus_ && surface_) {
+    // Capture is asynchronous, so avoid rendering old cursor in the meantime.

[reveman, 2017/03/30 07:47:53]

I don't understand why we need to do all this. This type of special case where
we force an update makes me worried about race conditions where we incorrectly
cancel the capture of a cursor frame. It's important that cursor changes can be
pipelined properly or animated cursor will not work correctly.

[Dominik Laskowski, 2017/04/05 17:59:25]

This special case is needed when dragging across displays. While dragging, the
cursor is locked, so CursorClient::SetCursor calls are collapsed into a single
call at the end of the drag. The cursor cannot be animated in that case. When
crossing displays, we need to force a capture to update the scale, and hide the
old cursor while the capture is pending.

+    cursor_capture_weak_ptr_factory_.InvalidateWeakPtrs();
+    UpdateCursor(ui::kCursorNone, true);
+    // Force the cursor to be updated even if locked.
+    CaptureCursor(true);
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // SurfaceDelegate overrides:
 
 void Pointer::OnSurfaceCommit() {
   surface_->CheckIfSurfaceHierarchyNeedsCommitToNewSurfaces();
   surface_->CommitSurfaceHierarchy();
 
   // Capture new cursor to reflect result of commit.
   if (focus_)
-    CaptureCursor();
+    CaptureCursor(false);
 }
 
 bool Pointer::IsSurfaceSynchronized() const {
   // A pointer surface is always desynchronized.
   return false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // SurfaceObserver overrides:
 
@@ skipping 11 matching lines @@
 
 Surface* Pointer::GetEffectiveTargetForEvent(ui::Event* event) const {
   Surface* target =
       Surface::AsSurface(static_cast<aura::Window*>(event->target()));
   if (!target)
     return nullptr;
 
   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() {
+void Pointer::CaptureCursor(bool force) {
   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), force));
+
   request->set_source(cursor_capture_source_id_);
-  surface_->window()->layer()->RequestCopyOfOutput(std::move(request));
+  cursor_->layer()->RequestCopyOfOutput(std::move(request));
+
+  // Prevent subsequent requests in the same frame from aborting this capture.

[reveman, 2017/03/30 07:47:53]

why is this needed?

[Dominik Laskowski, 2017/04/05 17:59:25]

If the client happens to commit or call wl_pointer::set_cursor while the cursor
is locked, we don't want those capture requests to cancel a pending request for
a forced capture. See above for the special case where |force| is true.

+  if (force)
+    cursor_capture_source_id_ = base::UnguessableToken::Create();
 }
 
-void Pointer::OnCursorCaptured(const gfx::Point& hotspot,
+void Pointer::OnCursorCaptured(gfx::Point hotspot,
+                               bool force,
                                std::unique_ptr<cc::CopyOutputResult> result) {
   if (!focus_)
     return;
 
-  cursor_ = ui::kCursorNone;
+  gfx::NativeCursor cursor = ui::kCursorNull;
   if (!result->IsEmpty()) {
     DCHECK(result->HasBitmap());
     std::unique_ptr<SkBitmap> bitmap = result->TakeBitmap();
 
     ui::PlatformCursor platform_cursor;
 #if defined(USE_OZONE)
     // TODO(reveman): Add interface for creating cursors from GpuMemoryBuffers
     // and use that here instead of the current bitmap API. crbug.com/686600
     platform_cursor = ui::CursorFactoryOzone::GetInstance()->CreateImageCursor(
         *bitmap.get(), hotspot);
 #elif defined(USE_X11)
     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)
     ui::UnrefCustomXCursor(platform_cursor);
 #endif
   }
 
-  UpdateCursor();
+  UpdateCursor(cursor, force);
 }
 
-void Pointer::UpdateCursor() {
+void Pointer::UpdateCursor(gfx::NativeCursor cursor, bool force) {
   DCHECK(focus_);
 
   aura::client::CursorClient* cursor_client =
       aura::client::GetCursorClient(focus_->window()->GetRootWindow());
-  if (cursor_client)
-    cursor_client->SetCursor(cursor_);
+  if (!cursor_client)
+    return;
+
+  cursor_client->SetCursor(cursor);
+
+  if (force && cursor_client->IsCursorLocked())

[reveman, 2017/03/30 07:47:53]

what's the situation where this is currently causing a problem? can this
IsCursorLocked part be moved to a separate CL?

[Dominik Laskowski, 2017/04/05 17:59:25]

|force| being true implies that the cursor is locked, but the check is there for
completeness. Also, note that CursorClient::SetCursor is called as well to
update the state applied when the cursor is unlocked.

OK, I will split out that edge case.

+    WMHelper::GetInstance()->SetCursor(cursor);
 }
 
 }  // namespace exo