Closed
Bug 888781
Opened 11 years ago
Closed 8 years ago
[HiDPI] Custom mouse cursors are too small
Categories
(Core :: Widget: Win32, defect, P4)
Tracking
()
VERIFIED
FIXED
mozilla52
People
(Reporter: edwardsgreg, Assigned: xidorn)
References
(Blocks 1 open bug)
Details
(Whiteboard: tpi:+,parity-chrome)
Attachments
(4 files)
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•11 years ago
|
Reporter | ||
Comment 1•11 years ago
|
||
I created Attachment 769535 [details] in the OSX bug (Bug 888689) to use as a testcase for cursor size problems.
Comment 2•9 years ago
|
||
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
Comment 3•9 years ago
|
||
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...
Updated•9 years ago
|
Summary: [HiDPI] Mouse cursors are too small → [HiDPI] Custom mouse cursors are too small
Updated•8 years ago
|
Priority: -- → P4
Whiteboard: tpi:+
Comment 4•8 years ago
|
||
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).
status-firefox48:
--- → affected
status-firefox49:
--- → affected
status-firefox50:
--- → affected
status-firefox51:
--- → affected
OS: Windows 7 → Windows
Whiteboard: tpi:+ → tpi:+,parity-chrome
Assignee | ||
Comment 5•8 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 hidden (mozreview-request) |
Comment 8•8 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•8 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•8 years ago
|
Assignee: nobody → xidorn+moz
Comment 10•8 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•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b4a82f0cb935
https://hg.mozilla.org/mozilla-central/rev/ff28b1510763
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 12•8 years ago
|
||
Is this something we want to uplift to 51?
Reporter | ||
Comment 13•8 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•8 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•8 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 16•8 years ago
|
||
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+
Updated•8 years ago
|
Flags: qe-verify+
Comment 17•8 years ago
|
||
(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.
Comment 18•8 years ago
|
||
bugherder uplift |
Updated•8 years ago
|
Assignee | ||
Updated•8 years ago
|
Comment 19•8 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•8 years ago
|
||
I'm attaching the about:support info from Windows 7. Maybe it helps.
Assignee | ||
Comment 21•8 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•8 years ago
|
||
Thanks for clarifications.
Then I will mark this as verified both for Aurora 51 and Nightly 52 (Windows 8.1 & Windows 10).
You need to log in
before you can comment on or make changes to this bug.
Description
•