Skip to content

Commit

Permalink
Make a new constructor for custom cursors
Browse files Browse the repository at this point in the history
Add a new constructor for `ui::Cursor` that accepts all parameters
needed to build a custom cursor, e.g. bitmap, hotspot and scale
factor.

This is a precursor CL for removing `content::WebCursor`. In a follow-up
CL, the constraints checked at `content::WebCursor::SetCursor` will be
performed in `ui::Cursor` and the setters for the custom cursor
parameters will be removed, making `ui::Cursor` an immutable class
(except for the platform cursor). This will ensure that the constraints
cannot be broken once a cursor is created.

Bug: 1149906
Change-Id: I21779964257452105892e0d56d7c8571fe89f991
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4217753
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1108640}
  • Loading branch information
hferreiro authored and Chromium LUCI CQ committed Feb 22, 2023
1 parent 98446a4 commit 2203613
Show file tree
Hide file tree
Showing 18 changed files with 173 additions and 197 deletions.
11 changes: 5 additions & 6 deletions ash/capture_mode/capture_mode_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "ash/capture_mode/capture_mode_session.h"

#include <tuple>
#include <utility>

#include "ash/accessibility/accessibility_controller_impl.h"
#include "ash/accessibility/magnifier/magnifier_glass.h"
Expand Down Expand Up @@ -228,7 +229,6 @@ views::Widget::InitParams CreateWidgetParams(aura::Window* parent,
// image icon if we're capturing image, or a video record image icon if we're
// capturing video.
ui::Cursor GetCursorForFullscreenOrWindowCapture(bool capture_image) {
ui::Cursor cursor(ui::mojom::CursorType::kCustom);
const display::Display display =
display::Screen::GetScreen()->GetDisplayNearestWindow(
capture_mode_util::GetPreferredRootWindow());
Expand All @@ -240,11 +240,10 @@ ui::Cursor GetCursorForFullscreenOrWindowCapture(bool capture_image) {
gfx::Point hotspot(bitmap.width() / 2, bitmap.height() / 2);
wm::ScaleAndRotateCursorBitmapAndHotpoint(
device_scale_factor, display.panel_rotation(), &bitmap, &hotspot);
auto* cursor_factory = ui::CursorFactory::GetInstance();
cursor.SetPlatformCursor(
cursor_factory->CreateImageCursor(cursor.type(), bitmap, hotspot));
cursor.set_custom_bitmap(bitmap);
cursor.set_custom_hotspot(hotspot);
ui::Cursor cursor = ui::Cursor::NewCustom(
std::move(bitmap), std::move(hotspot), device_scale_factor);
cursor.SetPlatformCursor(ui::CursorFactory::GetInstance()->CreateImageCursor(
cursor.type(), cursor.custom_bitmap(), cursor.custom_hotspot()));

return cursor;
}
Expand Down
16 changes: 6 additions & 10 deletions ash/capture_mode/capture_mode_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@
#include "ui/message_center/message_center_observer.h"
#include "ui/message_center/public/cpp/notification.h"
#include "ui/message_center/public/cpp/notification_delegate.h"
#include "ui/ozone/public/ozone_platform.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/view.h"
#include "ui/views/widget/widget.h"
Expand Down Expand Up @@ -5130,18 +5129,15 @@ TEST_F(CaptureModeCursorOverlayTest, OverlayBoundsAccountForCursorScaleFactor) {
auto* cursor_manager = Shell::Get()->cursor_manager();
auto set_cursor = [cursor_manager](const gfx::Size& cursor_image_size,
float cursor_image_scale_factor) {
const auto cursor_type = CursorType::kCustom;
gfx::NativeCursor cursor{cursor_type};
SkBitmap cursor_image;
cursor_image.allocN32Pixels(cursor_image_size.width(),
cursor_image_size.height());
cursor.set_image_scale_factor(cursor_image_scale_factor);
cursor.set_custom_bitmap(cursor_image);
auto* platform_cursor_factory =
ui::OzonePlatform::GetInstance()->GetCursorFactory();
cursor.SetPlatformCursor(platform_cursor_factory->CreateImageCursor(
cursor_type, cursor_image, cursor.custom_hotspot()));
cursor_manager->SetCursor(cursor);
ui::Cursor cursor = ui::Cursor::NewCustom(
std::move(cursor_image), gfx::Point(), cursor_image_scale_factor);
cursor.SetPlatformCursor(
ui::CursorFactory::GetInstance()->CreateImageCursor(
cursor.type(), cursor.custom_bitmap(), cursor.custom_hotspot()));
cursor_manager->SetCursor(std::move(cursor));
};

struct {
Expand Down
12 changes: 5 additions & 7 deletions ash/wm/cursor_manager_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,19 @@ void CursorManager::Init() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceShowCursor)) {
// Set a custom cursor so users know that the switch is turned on.
gfx::NativeCursor cursor(ui::mojom::CursorType::kCustom);
const gfx::ImageSkia custom_icon =
gfx::CreateVectorIcon(kTouchIndicatorIcon, SK_ColorBLACK);
const float dsf =
display::Screen::GetScreen()->GetPrimaryDisplay().device_scale_factor();
SkBitmap bitmap = custom_icon.GetRepresentation(dsf).GetBitmap();
gfx::Point hotspot(bitmap.width() / 2, bitmap.height() / 2);
auto* cursor_factory = ui::CursorFactory::GetInstance();
ui::Cursor cursor =
ui::Cursor::NewCustom(std::move(bitmap), std::move(hotspot), dsf);
cursor.SetPlatformCursor(
cursor_factory->CreateImageCursor(cursor.type(), bitmap, hotspot));
cursor.set_custom_bitmap(bitmap);
cursor.set_custom_hotspot(hotspot);
cursor.set_image_scale_factor(dsf);
ui::CursorFactory::GetInstance()->CreateImageCursor(
cursor.type(), cursor.custom_bitmap(), cursor.custom_hotspot()));

SetCursor(cursor);
SetCursor(std::move(cursor));
LockCursor();
return;
}
Expand Down
15 changes: 7 additions & 8 deletions chrome/browser/devtools/devtools_eye_dropper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ void DevToolsEyeDropper::AttachToHost(content::RenderFrameHost* frame_host) {

void DevToolsEyeDropper::DetachFromHost() {
host_->RemoveMouseEventCallback(mouse_event_callback_);
ui::Cursor cursor(ui::mojom::CursorType::kPointer);
host_->SetCursor(cursor);
host_->SetCursor(ui::mojom::CursorType::kPointer);
video_capturer_.reset();
host_ = nullptr;
}
Expand Down Expand Up @@ -276,12 +275,12 @@ void DevToolsEyeDropper::UpdateCursor() {
paint.setAntiAlias(true);
canvas.drawCircle(kCursorSize / 2, kCursorSize / 2, kDiameter / 2, paint);

ui::Cursor cursor(ui::mojom::CursorType::kCustom);
cursor.set_image_scale_factor(device_scale_factor);
cursor.set_custom_bitmap(result);
cursor.set_custom_hotspot(gfx::Point(kHotspotOffset * device_scale_factor,
kHotspotOffset * device_scale_factor));
host_->SetCursor(cursor);
ui::Cursor cursor =
ui::Cursor::NewCustom(std::move(result),
gfx::Point(kHotspotOffset * device_scale_factor,
kHotspotOffset * device_scale_factor),
device_scale_factor);
host_->SetCursor(std::move(cursor));
}

void DevToolsEyeDropper::OnFrameCaptured(
Expand Down
15 changes: 7 additions & 8 deletions components/exo/pointer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -937,10 +937,9 @@ void Pointer::UpdateCursor() {

// Scaling bitmap to match the corresponding supported scale factor of ash.
const display::Display& display = cursor_client->GetDisplay();
float scale =
ui::GetScaleForResourceScaleFactor(ui::GetSupportedResourceScaleFactor(
display.device_scale_factor())) /
capture_scale_;
const float resource_scale_factor = ui::GetScaleForResourceScaleFactor(
ui::GetSupportedResourceScaleFactor(display.device_scale_factor()));
const float scale = resource_scale_factor / capture_scale_;

// Use panel_rotation() rather than "natural" rotation, as it actually
// relates to the hardware you're about to draw the cursor bitmap on.
Expand All @@ -950,11 +949,11 @@ void Pointer::UpdateCursor() {
// TODO(reveman): Add interface for creating cursors from GpuMemoryBuffers
// and use that here instead of the current bitmap API.
// https://crbug.com/686600
cursor_ = ui::Cursor::NewCustom(std::move(bitmap), std::move(hotspot),
resource_scale_factor);
cursor_.SetPlatformCursor(
ui::CursorFactory::GetInstance()->CreateImageCursor(cursor_.type(),
bitmap, hotspot));
cursor_.set_custom_bitmap(bitmap);
cursor_.set_custom_hotspot(hotspot);
ui::CursorFactory::GetInstance()->CreateImageCursor(
cursor_.type(), cursor_.custom_bitmap(), cursor_.custom_hotspot()));
}

// When pointer capture is broken, use the standard system cursor instead of
Expand Down
8 changes: 2 additions & 6 deletions content/browser/renderer_host/input/touch_emulator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,8 @@ ui::Cursor TouchEmulator::InitCursorFromResource(int resource_id) {
content::GetContentClient()->GetNativeImageNamed(resource_id);
SkBitmap bitmap = cursor_image.AsBitmap();
gfx::Point hotspot(bitmap.width() / 2, bitmap.height() / 2);

ui::Cursor cursor(ui::mojom::CursorType::kCustom);
cursor.set_custom_bitmap(std::move(bitmap));
cursor.set_custom_hotspot(std::move(hotspot));
cursor.set_image_scale_factor(cursor_scale_factor_);
return cursor;
return ui::Cursor::NewCustom(std::move(bitmap), std::move(hotspot),
cursor_scale_factor_);
}

bool TouchEmulator::HandleMouseEvent(const WebMouseEvent& mouse_event,
Expand Down
6 changes: 3 additions & 3 deletions content/browser/renderer_host/render_widget_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <memory>
#include <tuple>
#include <utility>

#include "base/command_line.h"
#include "base/functional/bind.h"
Expand Down Expand Up @@ -2423,13 +2424,12 @@ TEST_F(RenderWidgetHostTest, OnVerticalScrollDirectionChanged) {
}

TEST_F(RenderWidgetHostTest, SetCursorWithBitmap) {
ui::Cursor cursor(ui::mojom::CursorType::kCustom);

SkBitmap bitmap;
bitmap.allocN32Pixels(1, 1);
bitmap.eraseColor(SK_ColorGREEN);
cursor.set_custom_bitmap(bitmap);

const ui::Cursor cursor =
ui::Cursor::NewCustom(std::move(bitmap), gfx::Point());
host_->SetCursor(cursor);
EXPECT_EQ(cursor, view_->last_cursor());
}
Expand Down
21 changes: 12 additions & 9 deletions content/common/cursors/webcursor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,18 @@ bool WebCursor::SetCursor(const ui::Cursor& cursor) {
#if defined(USE_AURA)
custom_cursor_.reset();
#endif
cursor_ = cursor;

// Clamp the hotspot to the custom image's dimensions.
if (cursor_.type() == ui::mojom::CursorType::kCustom) {
cursor_.set_custom_hotspot(
gfx::Point(std::max(0, std::min(cursor_.custom_bitmap().width() - 1,
cursor_.custom_hotspot().x())),
std::max(0, std::min(cursor_.custom_bitmap().height() - 1,
cursor_.custom_hotspot().y()))));

if (cursor.type() == ui::mojom::CursorType::kCustom) {
cursor_ = ui::Cursor::NewCustom(
cursor.custom_bitmap(),
// Clamp the hotspot to the custom image's dimensions.
gfx::Point(std::max(0, std::min(cursor.custom_bitmap().width() - 1,
cursor.custom_hotspot().x())),
std::max(0, std::min(cursor.custom_bitmap().height() - 1,
cursor.custom_hotspot().y()))),
cursor.image_scale_factor());
} else {
cursor_ = cursor;
}

return true;
Expand Down
9 changes: 4 additions & 5 deletions content/common/cursors/webcursor_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,17 @@ namespace content {
gfx::NativeCursor WebCursor::GetNativeCursor() {
if (cursor_.type() == ui::mojom::CursorType::kCustom) {
if (!custom_cursor_) {
custom_cursor_.emplace(ui::mojom::CursorType::kCustom);
SkBitmap bitmap = cursor_.custom_bitmap();
gfx::Point hotspot = cursor_.custom_hotspot();
float scale = GetCursorScaleFactor(&bitmap);
wm::ScaleAndRotateCursorBitmapAndHotpoint(scale, rotation_, &bitmap,
&hotspot);
custom_cursor_->set_custom_bitmap(bitmap);
custom_cursor_->set_custom_hotspot(hotspot);
custom_cursor_->set_image_scale_factor(device_scale_factor_);
custom_cursor_ = ui::Cursor::NewCustom(
std::move(bitmap), std::move(hotspot), device_scale_factor_);
custom_cursor_->SetPlatformCursor(
ui::CursorFactory::GetInstance()->CreateImageCursor(
ui::mojom::CursorType::kCustom, bitmap, hotspot));
custom_cursor_->type(), custom_cursor_->custom_bitmap(),
custom_cursor_->custom_hotspot()));
}
return *custom_cursor_;
}
Expand Down
54 changes: 21 additions & 33 deletions content/common/cursors/webcursor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ TEST(WebCursorTest, WebCursorCursorConstructor) {
}

TEST(WebCursorTest, WebCursorCursorConstructorCustom) {
ui::Cursor cursor(ui::mojom::CursorType::kCustom);
cursor.set_custom_bitmap(CreateTestBitmap(32, 32));
cursor.set_custom_hotspot(gfx::Point(10, 20));
cursor.set_image_scale_factor(2.f);
const ui::Cursor cursor =
ui::Cursor::NewCustom(CreateTestBitmap(32, 32), gfx::Point(10, 20), 2.0f);
WebCursor webcursor(cursor);
EXPECT_EQ(cursor, webcursor.cursor());

Expand Down Expand Up @@ -87,9 +85,8 @@ TEST(WebCursorTest, WebCursorCursorConstructorCustom) {

TEST(WebCursorTest, ClampHotspot) {
// Initialize a cursor with an invalid hotspot; it should be clamped.
ui::Cursor cursor(ui::mojom::CursorType::kCustom);
cursor.set_custom_hotspot(gfx::Point(100, 100));
cursor.set_custom_bitmap(CreateTestBitmap(5, 7));
ui::Cursor cursor =
ui::Cursor::NewCustom(CreateTestBitmap(5, 7), gfx::Point(100, 100));
WebCursor webcursor(cursor);
EXPECT_EQ(gfx::Point(4, 6), webcursor.cursor().custom_hotspot());
// SetCursor should also clamp the hotspot.
Expand All @@ -101,48 +98,44 @@ TEST(WebCursorTest, SetCursor) {
WebCursor webcursor;
EXPECT_TRUE(webcursor.SetCursor(ui::Cursor()));
EXPECT_TRUE(webcursor.SetCursor(ui::Cursor(ui::mojom::CursorType::kHand)));
EXPECT_TRUE(webcursor.SetCursor(ui::Cursor(ui::mojom::CursorType::kCustom)));
EXPECT_TRUE(
webcursor.SetCursor(ui::Cursor::NewCustom(SkBitmap(), gfx::Point())));

ui::Cursor cursor(ui::mojom::CursorType::kCustom);
cursor.set_custom_bitmap(CreateTestBitmap(32, 32));
cursor.set_custom_hotspot(gfx::Point(10, 20));
cursor.set_image_scale_factor(1.5f);
const SkBitmap kBitmap = CreateTestBitmap(32, 32);
const gfx::Point kHotspot = gfx::Point(10, 20);
ui::Cursor cursor = ui::Cursor::NewCustom(kBitmap, kHotspot, 1.5f);
EXPECT_TRUE(webcursor.SetCursor(cursor));

// SetCursor should return false when the scale factor is too small.
cursor.set_image_scale_factor(0.001f);
cursor = ui::Cursor::NewCustom(kBitmap, kHotspot, 0.001f);
EXPECT_FALSE(webcursor.SetCursor(cursor));

// SetCursor should return false when the scale factor is too large.
cursor.set_image_scale_factor(1000.f);
cursor = ui::Cursor::NewCustom(kBitmap, kHotspot, 1000.0f);
EXPECT_FALSE(webcursor.SetCursor(cursor));

// SetCursor should return false when the unscaled bitmap width is too large.
cursor.set_image_scale_factor(10.f);
cursor.set_custom_bitmap(CreateTestBitmap(1025, 5));
cursor = ui::Cursor::NewCustom(CreateTestBitmap(1025, 5), kHotspot, 10.0f);
EXPECT_FALSE(webcursor.SetCursor(cursor));

// SetCursor should return false when the unscaled bitmap height is too large.
cursor.set_custom_bitmap(CreateTestBitmap(5, 1025));
cursor = ui::Cursor::NewCustom(CreateTestBitmap(5, 1025), kHotspot, 10.0f);
EXPECT_FALSE(webcursor.SetCursor(cursor));

// SetCursor should return false when the 1x scaled image width is too large.
cursor.set_image_scale_factor(1.f);
cursor.set_custom_bitmap(CreateTestBitmap(151, 3));
cursor = ui::Cursor::NewCustom(CreateTestBitmap(151, 3), kHotspot, 1.0f);
EXPECT_FALSE(webcursor.SetCursor(cursor));

// SetCursor should return false when the 1x scaled image height is too large.
cursor.set_custom_bitmap(CreateTestBitmap(3, 151));
cursor = ui::Cursor::NewCustom(CreateTestBitmap(3, 151), kHotspot, 1.0f);
EXPECT_FALSE(webcursor.SetCursor(cursor));

// SetCursor should return false when the scaled image width is too large.
cursor.set_image_scale_factor(0.02f);
cursor.set_custom_bitmap(CreateTestBitmap(50, 5));
cursor = ui::Cursor::NewCustom(CreateTestBitmap(50, 5), kHotspot, 0.02f);
EXPECT_FALSE(webcursor.SetCursor(cursor));

// SetCursor should return false when the scaled image height is too large.
cursor.set_image_scale_factor(0.1f);
cursor.set_custom_bitmap(CreateTestBitmap(5, 20));
cursor = ui::Cursor::NewCustom(CreateTestBitmap(5, 20), kHotspot, 0.1f);
EXPECT_FALSE(webcursor.SetCursor(cursor));
}

Expand All @@ -151,11 +144,8 @@ TEST(WebCursorTest, CursorScaleFactor) {
constexpr float kImageScale = 2.0f;
constexpr float kDeviceScale = 4.2f;

ui::Cursor cursor(ui::mojom::CursorType::kCustom);
cursor.set_custom_hotspot(gfx::Point(0, 1));
cursor.set_image_scale_factor(kImageScale);
cursor.set_custom_bitmap(CreateTestBitmap(128, 128));
WebCursor webcursor(cursor);
WebCursor webcursor(ui::Cursor::NewCustom(CreateTestBitmap(128, 128),
gfx::Point(0, 1), kImageScale));

display::Display display;
display.set_device_scale_factor(kDeviceScale);
Expand Down Expand Up @@ -183,10 +173,8 @@ TEST(WebCursorTest, CursorScaleFactor) {

#if BUILDFLAG(IS_WIN)
void ScaleCursor(float scale, int hotspot_x, int hotspot_y) {
ui::Cursor cursor(ui::mojom::CursorType::kCustom);
cursor.set_custom_hotspot(gfx::Point(hotspot_x, hotspot_y));
cursor.set_custom_bitmap(CreateTestBitmap(10, 10));
WebCursor webcursor(cursor);
WebCursor webcursor(ui::Cursor::NewCustom(CreateTestBitmap(10, 10),
gfx::Point(hotspot_x, hotspot_y)));

display::Display display;
display.set_device_scale_factor(scale);
Expand Down
10 changes: 3 additions & 7 deletions content/renderer/pepper/pepper_plugin_instance_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2155,21 +2155,17 @@ PP_Bool PepperPluginInstanceImpl::SetCursor(PP_Instance instance,
if (!auto_mapper.is_valid())
return PP_FALSE;

auto custom_cursor =
std::make_unique<ui::Cursor>(ui::mojom::CursorType::kCustom);
custom_cursor->set_custom_hotspot(gfx::Point(hot_spot->x, hot_spot->y));

SkBitmap bitmap(image_data->GetMappedBitmap());
// Make a deep copy, so that the cursor remains valid even after the original
// image data gets freed.
SkBitmap dst = custom_cursor->custom_bitmap();
SkBitmap dst;
if (!dst.tryAllocPixels(bitmap.info()) ||
!bitmap.readPixels(dst.info(), dst.getPixels(), dst.rowBytes(), 0, 0)) {
return PP_FALSE;
}
custom_cursor->set_custom_bitmap(dst);

DoSetCursor(std::move(custom_cursor));
DoSetCursor(std::make_unique<ui::Cursor>(ui::Cursor::NewCustom(
std::move(dst), gfx::Point(hot_spot->x, hot_spot->y))));
return PP_TRUE;
}

Expand Down
10 changes: 3 additions & 7 deletions third_party/blink/renderer/core/input/event_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -652,13 +652,9 @@ absl::optional<ui::Cursor> EventHandler::SelectCursor(
image = svg_image_holder.get();
}

ui::Cursor cursor(ui::mojom::blink::CursorType::kCustom);
cursor.set_custom_bitmap(
image->AsSkBitmapForCurrentFrame(kRespectImageOrientation));
cursor.set_custom_hotspot(
DetermineHotSpot(*image, hot_spot_specified, hot_spot));
cursor.set_image_scale_factor(scale);
return cursor;
return ui::Cursor::NewCustom(
image->AsSkBitmapForCurrentFrame(kRespectImageOrientation),
DetermineHotSpot(*image, hot_spot_specified, hot_spot), scale);
}
}

Expand Down

0 comments on commit 2203613

Please sign in to comment.