[HiDPI] Custom mouse cursors are too small

VERIFIED FIXED in Firefox 51

Status

()

defect
P4
normal
VERIFIED FIXED
6 years ago
3 years ago

People

(Reporter: edwardsgreg, Assigned: xidorn)

Tracking

(Blocks 1 bug)

25 Branch
mozilla52
All
Windows
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox48 wontfix, firefox49 wontfix, firefox50 wontfix, firefox51 verified, firefox52 verified)

Details

(Whiteboard: tpi:+,parity-chrome)

Attachments

(4 attachments)

Reporter

Description

6 years ago
When running Windows in HiDPI mode, custom CSS mouse cursors are displayed too small. Despite .cur files supporting multiple resolutions, Firefox always chooses the 32x32 version or the nearest size to it and displays it at its native, device pixel size.

Instead, Firefox ought to emulate how Windows itself handles cursors:
1. Start with a resolution of 32 * window.devicePixelRatio.
2. Round down to the nearest of (32, 40, 48, 64). This is how big we draw the cursor, in device pixels.
3. Among the resolutions included in the cursor, pick the next one up or, if none exists, the largest.
4. Resize this cursor to the target size.

PNG cursors are also drawn unexpectedly small on HiDPI configurations but there doesn't seem to be a perfect way to deal with this. Maybe we should always enlarge them by window.devicePixelRatio and allow image-set on cursors when that becomes available.
Reporter

Updated

6 years ago
Blocks: win-hidpi
Component: Untriaged → Widget: Win32
Product: Firefox → Core
Hardware: x86_64 → All
Reporter

Comment 1

6 years ago
I created Attachment 769535 [details] in the OSX bug (Bug 888689) to use as a testcase for cursor size problems.
I just noticed this with the custom pointer used on Imgur albums (eg http://imgur.com/a/L3Tar) -- hovering over an image uses a custom magnifying glass image (not the standard CSS magnifier cursor).

Edge and Chrome and display a too-small icon.

We do the right thing on OSX -- the image is upscaled before handing it off to the OS.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Presumably this needs to happen in nsWindow::SetCursor(). Or in nsWindowGfx::CreateIcon()? ::SetCursor explicitly tells it not to scale, but I think that really just means "use the default size". One of these two spots should adjust for the DPI scaling factor...
Summary: [HiDPI] Mouse cursors are too small → [HiDPI] Custom mouse cursors are too small

Updated

3 years ago
Priority: -- → P4
Whiteboard: tpi:+
Gmail's custom hand cursor is too small on Windows HiDPI display.

STR:
1. Open Gmail.
2. In the email list view, hover the mouse pointer over an email's checkbox.
3. Slowly move the mouse pointer (an arrow cursor) below the checkbox.

RESULT:
The arrow cursor changes to a hand cursor, but the hand is very, very small in Firefox. In contrast to comment 2, Chrome currently scales the hand cursor correctly. In Edge, the arrow cursor does not change into a hand cursor.

My window.devicePixelRatio === 2.4000000953674316 (225% scaling factor).
OS: Windows 7 → Windows
Whiteboard: tpi:+ → tpi:+,parity-chrome
Assignee

Comment 5

3 years ago
(In reply to Justin Dolske [:Dolske] from comment #3)
> Presumably this needs to happen in nsWindow::SetCursor(). Or in
> nsWindowGfx::CreateIcon()? ::SetCursor explicitly tells it not to scale, but
> I think that really just means "use the default size". One of these two
> spots should adjust for the DPI scaling factor...

It seems to me nsWindow::SetCursor() should probably calculate the scaled size and feed it to nsWindowGfx::CreateIcon().

The comment tells not to scale was added in bug 515907 which adds the aScaledSize parameter to nsWindowGfx::CreateIcon(). I suppose that comment was just for explaining that the 0x0 size there indicates "No scaling", not because there was any reason it shouldn't scale.
Comment hidden (mozreview-request)

Comment 8

3 years ago
mozreview-review
Comment on attachment 8792355 [details]
Bug 888781 part 1 - Use cached default scale in GetDefaultScaleInternal of Windows.

https://reviewboard.mozilla.org/r/79404/#review78178

odd we weren't using that before.
Attachment #8792355 - Flags: review?(jmathies) → review+

Comment 9

3 years ago
mozreview-review
Comment on attachment 8792356 [details]
Bug 888781 part 2 - Scale cursor with the window scale on Windows.

https://reviewboard.mozilla.org/r/79406/#review78180
Attachment #8792356 - Flags: review?(jmathies) → review+
Assignee

Updated

3 years ago
Assignee: nobody → xidorn+moz

Comment 10

3 years ago
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b4a82f0cb935
part 1 - Use cached default scale in GetDefaultScaleInternal of Windows. r=jimm
https://hg.mozilla.org/integration/autoland/rev/ff28b1510763
part 2 - Scale cursor with the window scale on Windows. r=jimm

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b4a82f0cb935
https://hg.mozilla.org/mozilla-central/rev/ff28b1510763
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Is this something we want to uplift to 51?
Reporter

Comment 13

3 years ago
I'm seeing matching results between Windows and OSX now which is probably a good thing, but it's still not matching my testcase. CUR files aren't working correctly (Bug 999404) and SVGs are being upscaled.

I made an updated testcase which now covers SVG and image-set (whenever that gets implemented).
Assignee

Comment 14

3 years ago
(In reply to Greg Edwards from comment #13)
> I made an updated testcase which now covers SVG and image-set (whenever that
> gets implemented).

Oh, SVG... I guess I know how to fix it.

(In reply to Sylvestre Ledru [:sylvestre] from comment #12)
> Is this something we want to uplift to 51?

Yes, I think so.
Assignee

Comment 15

3 years ago
Comment on attachment 8792356 [details]
Bug 888781 part 2 - Scale cursor with the window scale on Windows.

Approval Request Comment
[Feature/regressing bug #]: not a regression
[User impact if declined]: may see small icon in HiDPI monitor on windows
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: not very risky because the change is simple and straightforward
[String/UUID change made/needed]: n/a

This request is ONLY for this patch. The first patch might be a bit more risky than this one. And this patch doesn't really depend on that.
Attachment #8792356 - Flags: approval-mozilla-aurora?
Comment on attachment 8792356 [details]
Bug 888781 part 2 - Scale cursor with the window scale on Windows.

This patch fixes a UI issue in HiDPI monitor on windows. Take it in aurora 51.
Attachment #8792356 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
(In reply to Jim Mathies [:jimm] from comment #8)
> Comment on attachment 8792355 [details]
> Bug 888781 part 1 - Use cached default scale in GetDefaultScaleInternal of
> Windows.
> 
> https://reviewboard.mozilla.org/r/79404/#review78178
> 
> odd we weren't using that before.

I'm a bit concerned this may have unexpected effects. IIRC, there used to be some cases where we'd miss whatever event we expect to clear the cached scale, so it can occasionally be out-of-date. GetDefaultScaleInternal worked around that by always querying the window; it just set the cached value for the benefit of code elsewhere (I forget offhand where else mDefaultScale is checked) but it doesn't really trust it to be current.

So... maybe this is fine (other fixes we've done in the meantime might have solved the related problems), but we should keep a careful eye open for regressions in behavior across mixed-resolution desktops.
Assignee

Updated

3 years ago
See Also: → 1312973

Comment 19

3 years ago
I verified the fix on Nightly 52.0a1 (Build ID 20161109030210) and Aurora 51.0a2 (Build ID 20161107004002). 

** Windows 10 and Windows 8.1:
    - using the testcase from comment 13, all works fine, except image-set that (I think) it's not yet implemented.
    - I also verified the issue using the steps to reproduce from comment 4 and it work as expected  

So, the cursor has the expected size (it is not smaller) as in FF 49 for e.g.

** Windows 7
On Windows 7 some strange things happen. I don't know if it's a Firefox issue because same happens on Chrome.
So, using the testcase:
   - first square (.cur file containing all four cursor sizes: 32, 40, 48, 64) shows as expected a cursor of size 32 (while on Windows 8.1 and 10 was a size of 64)...and when mouse over it shows 64.
   - all the options where it is expected a size of 32 are instead bigger (as 64 size). 

Do you have any thoughts on this? Should it work on Windows 7?

Comment 20

3 years ago
I'm attaching the about:support info from Windows 7. Maybe it helps.
Assignee

Comment 21

3 years ago
Windows 7 doesn't have a good support to HiDPI, and I think the majority of people who use HiDPI should have been using Windows 8.1 or up, so I wouldn't be worried about Windows 7 for this. Thanks for testing.

Comment 22

3 years ago
Thanks for clarifications.
Then I will mark this as verified both for Aurora 51 and Nightly 52 (Windows 8.1 & Windows 10).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.