exo: Fix multi-display hardware cursor

components/exo/pointer.h

 // 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.
 
 #ifndef COMPONENTS_EXO_POINTER_H_
 #define COMPONENTS_EXO_POINTER_H_
 
 #include <memory>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/unguessable_token.h"
 #include "components/exo/surface_delegate.h"
 #include "components/exo/surface_observer.h"
 #include "components/exo/wm_helper.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/cursor/cursor.h"
+#include "ui/display/display.h"
 #include "ui/events/event_constants.h"
 #include "ui/events/event_handler.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/native_widget_types.h"
 
 namespace cc {
 class CopyOutputResult;
 }
 
@@ skipping 25 matching lines @@
 
   // Returns the current cursor for the pointer.
   gfx::NativeCursor GetCursor();
 
   // Overridden from ui::EventHandler:
   void OnMouseEvent(ui::MouseEvent* event) override;
   void OnScrollEvent(ui::ScrollEvent* event) override;
 
   // Overridden from WMHelper::CursorObserver:
   void OnCursorSetChanged(ui::CursorSetType cursor_set) override;
+  void OnCursorDisplayChanging(const display::Display& display) override;
 
   // Overridden from SurfaceDelegate:
   void OnSurfaceCommit() override;
   bool IsSurfaceSynchronized() const override;
 
   // Overridden from SurfaceObserver:
   void OnSurfaceDestroying(Surface* surface) override;
 
  private:
   // Returns the effective target for |event|.
   Surface* GetEffectiveTargetForEvent(ui::Event* event) const;
 
-  // Recompute cursor scale and update cursor if scale changed.
-  void UpdateCursorScale();

[reveman, 2017/05/23 17:06:58]

Can we keep this and instead add a UpdateCursorDeviceScaleFactorAndRotation() so
the update logic stays the same as before?

[Dominik Laskowski, 2017/05/24 00:43:04]

Checking for scale/rotation changes for each mouse event is overkill, i.e. the
only events requiring a cursor update are OnCursorSetChanged and
OnCursorDisplayChanging.

[reveman, 2017/05/25 10:52:55]

Yes, let's rename this to UpdateCursor() and call that when rotation or scale
might have changed.

[Dominik Laskowski, 2017/05/31 02:06:26]

Done.

-
   // Asynchronously update the cursor by capturing a snapshot of |surface_|.
   void CaptureCursor();
 
   // Called when cursor snapshot has been captured.
-  void OnCursorCaptured(const gfx::Point& hotspot,

[reveman, 2017/05/23 17:06:57]

I think we need to have hotspot updates flow through this.

[Dominik Laskowski, 2017/05/24 00:43:04]

Good point. I've updated SetCursor to save the hotspot on capture.

-                        std::unique_ptr<cc::CopyOutputResult> result);
+  void OnCursorCaptured(std::unique_ptr<cc::CopyOutputResult> result);
 
-  // Update cursor to reflect the current value of |cursor_|.
+  // Synchronously update the cursor to the latest snapshot.
   void UpdateCursor();
 
+  void SetCursor(gfx::NativeCursor cursor);
+
   // The delegate instance that all events are dispatched to.
   PointerDelegate* const delegate_;
 
   // The current pointer surface.
   Surface* surface_ = nullptr;
 
   // The current focus surface for the pointer.
   Surface* focus_ = nullptr;
 
   // The location of the pointer in the current focus surface.
   gfx::PointF location_;
 
-  // The scale applied to the cursor to compensate for the UI scale.
+  // The display-independent scale applied to the cursor.
   float cursor_scale_ = 1.0f;
 
+  // Device scale factor of the display where the mouse is located.
+  float device_scale_factor_ = 1.0f;

[reveman, 2017/05/23 17:06:58]

nit: s/device_scale_factor_/cursor_device_scale_factor_/ and s/mouse/cursor
sprite/

[Dominik Laskowski, 2017/05/24 00:43:04]

Done.

+
+  // Rotation of the display where the mouse is located.
+  display::Display::Rotation rotation_ = display::Display::ROTATE_0;

[reveman, 2017/05/23 17:06:58]

nit: s/rotation_/cursor_rotation_/ and s/mouse/cursor sprite/

[Dominik Laskowski, 2017/05/24 00:43:04]

Done.

+
   // The position of the pointer surface relative to the pointer location.
   gfx::Point hotspot_;
 
-  // The current cursor.
-  ui::Cursor cursor_;
+  // Latest cursor snapshot.
+  SkBitmap cursor_;

[reveman, 2017/05/23 17:06:58]

Please keep this as before and add another member for the bitmap state. ie.
"SkBitmap cursor_bitmap_;". I think that will reduce the diff of the patch
significantly and make it much easier to review

[Dominik Laskowski, 2017/05/24 00:43:04]

Renamed, but the ui::Cursor is redundant, i.e. CursorClient::GetCursor is
equivalent.

 
   // Source used for cursor capture copy output requests.
   const base::UnguessableToken cursor_capture_source_id_;
 
   // Last received event type.
   ui::EventType last_event_type_ = ui::ET_UNKNOWN;
 
   // Weak pointer factory used for cursor capture callbacks.
   base::WeakPtrFactory<Pointer> cursor_capture_weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Pointer);
 };
 
 }  // namespace exo
 
 #endif  // COMPONENTS_EXO_POINTER_H_