Closed Bug 888781 Opened 8 years ago Closed 5 years ago
DPI] Custom mouse cursors are too small
58 bytes, text/x-review-board-request
58 bytes, text/x-review-board-request
11.83 KB, application/zip
6.79 KB, text/plain
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.
Component: Untriaged → Widget: Win32
Product: Firefox → Core
Hardware: x86_64 → All
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
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).
(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 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 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+
Pushed by firstname.lastname@example.org: 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
Is this something we want to uplift to 51?
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).
(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.
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+
(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.
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?
I'm attaching the about:support info from Windows 7. Maybe it helps.
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.
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.