Handle custom/bitmap cursors in ws2.
Go through CursorFactoryOzone to load the PlatformCursor (which
confusingly could be CursorData, but for now is X11CursorOzone). Also
set fields on the cross-platform Cursor type so that code which relies
on it such as CursorWindowController::UpdateCursorImage will work.
That code path can be exercised with --ash-enable-cursor-motion-blur
although there is currently no use of custom cursors with ws2.
Bug: 837705
Change-Id: I9f725c5a5350e28cf8bded6b53bf48759cb644f8
Reviewed-on: https://chromium-review.googlesource.com/1105562
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571213}diff --git a/ash/cursor_unittest.cc b/ash/cursor_unittest.cc
index c251828..27356b1 100644
--- a/ash/cursor_unittest.cc
+++ b/ash/cursor_unittest.cc
@@ -13,6 +13,7 @@
#include "ui/events/event.h"
#include "ui/events/event_constants.h"
#include "ui/events/test/event_generator.h"
+#include "ui/gfx/image/image_unittest_util.h"
namespace ash {
@@ -85,4 +86,28 @@
ash::Shell::Get()->cursor_manager()->GetCursor().native_type());
}
+TEST_F(CursorTest, Custom) {
+ // Create and hover a window.
+ std::unique_ptr<aura::Window> window =
+ CreateTestWindow(gfx::Rect(0, 0, 100, 100));
+ EXPECT_EQ(0U, GetTestWindowTreeClient()->input_events().size());
+ ui::test::EventGenerator generator(window.get());
+ generator.MoveMouseToInHost(50, 50);
+
+ // Set a custom cursor.
+ SkBitmap bitmap = gfx::test::CreateBitmap(10, 10);
+ const ui::CursorData image_cursor(gfx::Point(1, 4), {bitmap}, 1.f,
+ base::TimeDelta());
+ GetWindowTreeTestHelper()->SetCursor(window.get(), image_cursor);
+
+ // Make sure it worked.
+ EXPECT_EQ(ui::CursorType::kCustom,
+ window->delegate()->GetCursor({}).native_type());
+ aura::client::CursorClient* cursor_client =
+ aura::client::GetCursorClient(window->GetRootWindow());
+ EXPECT_EQ(ui::CursorType::kCustom, cursor_client->GetCursor().native_type());
+ EXPECT_EQ(bitmap.getGenerationID(),
+ cursor_client->GetCursor().GetBitmap().getGenerationID());
+}
+
} // namespace ash
diff --git a/services/ui/public/interfaces/cursor/cursor.mojom b/services/ui/public/interfaces/cursor/cursor.mojom
index ae4e453..0a267bc 100644
--- a/services/ui/public/interfaces/cursor/cursor.mojom
+++ b/services/ui/public/interfaces/cursor/cursor.mojom
@@ -69,7 +69,7 @@
// A description of a cursor.
struct CursorData {
- // The type of cursor. If CUSTOM, the rest of the fields are relevant.
+ // The type of cursor. If kCustom, the rest of the fields are relevant.
CursorType cursor_type;
// The delay between cursor frames.
@@ -78,7 +78,7 @@
// The hotspot in pixels in the source cursor frames.
gfx.mojom.Point hotspot_in_pixels;
- // The frames of the cursor.
+ // The frames of the cursor. Must be non-empty if |cursor_type| is kCustom.
array<skia.mojom.Bitmap?> cursor_frames;
// This is the image scale of this cursor.
diff --git a/services/ui/public/interfaces/cursor/cursor_struct_traits.cc b/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
index f3a3888..17f0aff5 100644
--- a/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
+++ b/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
@@ -334,15 +334,21 @@
gfx::Point hotspot_in_pixels;
std::vector<SkBitmap> cursor_frames;
- float scale_factor = data.scale_factor();
base::TimeDelta frame_delay;
if (!data.ReadHotspotInPixels(&hotspot_in_pixels) ||
- !data.ReadCursorFrames(&cursor_frames) ||
+ !data.ReadCursorFrames(&cursor_frames) || cursor_frames.empty() ||
!data.ReadFrameDelay(&frame_delay)) {
return false;
}
+ // Clamp the scale factor to a reasonable value. TODO(estade): do we even need
+ // this field? It doesn't appear to be used anywhere and is a property of the
+ // display, not the cursor.
+ float scale_factor = data.scale_factor();
+ if (scale_factor < 1.f || scale_factor > 3.f)
+ return false;
+
*out = ui::CursorData(hotspot_in_pixels, cursor_frames, scale_factor,
frame_delay);
return true;
diff --git a/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc b/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
index 0b26d67..f844eed 100644
--- a/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
+++ b/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
@@ -114,4 +114,16 @@
EXPECT_TRUE(output.cursor_frames().front().empty());
}
+// For custom cursors, the frame image vector must be non-empty.
+TEST_F(CursorStructTraitsTest, TestMissingFrames) {
+ const base::TimeDelta kFrameDelay = base::TimeDelta::FromMilliseconds(15);
+ const gfx::Point kHotspot = gfx::Point(5, 2);
+ const float kScale = 2.0f;
+
+ ui::CursorData input(kHotspot, {}, kScale, kFrameDelay);
+
+ ui::CursorData output;
+ EXPECT_FALSE(EchoCursorData(input, &output));
+}
+
} // namespace ui
diff --git a/services/ui/ws/threaded_image_cursors.cc b/services/ui/ws/threaded_image_cursors.cc
index aaab5ca..f5178d7 100644
--- a/services/ui/ws/threaded_image_cursors.cc
+++ b/services/ui/ws/threaded_image_cursors.cc
@@ -91,10 +91,8 @@
scoped_refptr<base::SingleThreadTaskRunner> ui_service_task_runner_,
base::WeakPtr<ThreadedImageCursors> threaded_image_cursors_weak_ptr) {
if (image_cursors_weak_ptr) {
- ui::PlatformCursor platform_cursor = cursor_factory->CreateAnimatedCursor(
- cursor_data->cursor_frames(), cursor_data->hotspot_in_pixels(),
- cursor_data->frame_delay().InMilliseconds(),
- cursor_data->scale_factor());
+ ui::PlatformCursor platform_cursor =
+ cursor_data->ToNativeCursor().platform();
// |platform_window| is owned by the UI Service thread, so setting the
// cursor on it also needs to happen on that thread.
ui_service_task_runner_->PostTask(
diff --git a/services/ui/ws2/window_tree.cc b/services/ui/ws2/window_tree.cc
index 1ddc3f7d..4e91f5f0 100644
--- a/services/ui/ws2/window_tree.cc
+++ b/services/ui/ws2/window_tree.cc
@@ -947,18 +947,13 @@
return false;
}
if (!IsClientCreatedWindow(window) && !IsClientRootWindow(window)) {
- DVLOG(1) << "SerCursor failed (access denied)";
+ DVLOG(1) << "SetCursor failed (access denied)";
return false;
}
auto* server_window = ServerWindow::GetMayBeNull(window);
- // Convert from CursorData to Cursor. TODO(estade): remove this conversion.
- // See class level comment on CursorData. Also support kCustom, i.e. image
- // cursors.
- ui::Cursor old_cursor_type(cursor.cursor_type());
- if (cursor.cursor_type() == ui::CursorType::kCustom)
- NOTIMPLEMENTED_LOG_ONCE();
+ ui::Cursor old_cursor_type = cursor.ToNativeCursor();
// Ask our delegate to set the cursor. This will save the cursor for toplevels
// and also update the active cursor if appropriate (i.e. if |window| is the
diff --git a/ui/base/cursor/cursor_data.cc b/ui/base/cursor/cursor_data.cc
index 6c39f5a..27847f17 100644
--- a/ui/base/cursor/cursor_data.cc
+++ b/ui/base/cursor/cursor_data.cc
@@ -5,7 +5,15 @@
#include "ui/base/cursor/cursor_data.h"
#include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/base/cursor/cursor_type.h"
+
+#if defined(USE_AURA)
#include "ui/base/cursor/cursor.h"
+#endif
+
+#if defined(USE_OZONE)
+#include "ui/ozone/public/cursor_factory_ozone.h"
+#endif
namespace ui {
@@ -48,4 +56,34 @@
generator_ids_ == rhs.generator_ids_;
}
+#if defined(USE_AURA)
+gfx::NativeCursor CursorData::ToNativeCursor() const {
+ Cursor cursor(cursor_type_);
+
+#if defined(USE_OZONE)
+ PlatformCursor platform;
+ auto* factory = CursorFactoryOzone::GetInstance();
+ if (cursor_type_ != CursorType::kCustom) {
+ platform = factory->GetDefaultCursor(cursor_type_);
+ } else if (cursor_frames_.size() > 1U) {
+ platform = factory->CreateAnimatedCursor(
+ cursor_frames_, hotspot_, frame_delay_.InMilliseconds(), scale_factor_);
+ } else {
+ DCHECK_EQ(1U, cursor_frames_.size());
+ platform =
+ factory->CreateImageCursor(cursor_frames_[0], hotspot_, scale_factor_);
+ }
+ cursor.SetPlatformCursor(platform);
+#else
+ NOTIMPLEMENTED();
+#endif
+
+ if (!cursor_frames_.empty()) {
+ cursor.set_custom_bitmap(cursor_frames_[0]);
+ cursor.set_custom_hotspot(hotspot_);
+ }
+ return cursor;
+}
+#endif
+
} // namespace ui
diff --git a/ui/base/cursor/cursor_data.h b/ui/base/cursor/cursor_data.h
index 6b3f5d2..ada9dd6 100644
--- a/ui/base/cursor/cursor_data.h
+++ b/ui/base/cursor/cursor_data.h
@@ -11,6 +11,7 @@
#include "build/build_config.h"
#include "ui/base/ui_base_export.h"
#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/native_widget_types.h"
class SkBitmap;
@@ -72,6 +73,11 @@
// data, but different generation ids.
bool IsSameAs(const CursorData& rhs) const;
+ // Until CursorData replaces Cursor, it's necessary to convert back to Cursor.
+ // Some code such as CursorWindowController still uses Cursor (and not its
+ // PlatformCursor).
+ gfx::NativeCursor ToNativeCursor() const;
+
private:
// A native type constant from cursor.h.
CursorType cursor_type_;
