888689 - Support for HiDPI custom mouse cursors with SVG images

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Ian Weiner]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130618035212

Steps to reproduce:

There does not seem to be any current way to style with a HiDPI custom mouse cursor. Using the CSS cursor property, this can be done in Webkit browsers with -webkit-image-set like this:

cursor: -webkit-image-set(
  url('cursor.png') 1x,
  url('[e-mail]') 2x
), auto;
 
I am not aware of anything comparable implemented in Gecko.


Actual results:

On Retina screens in OS X a custom cursor set with a url property is pixel-doubled and displayed at the correct size, but is noticeably blurry.


Expected results:

Perhaps there should be a way to specify the desired height and width of cursor images, similar to how display retina images are handled elsewhere in Gecko.

Comment 1

[Greg Edwards]

.cur files support multiple resolutions. How do they work at present?

Comment 2

[Ian Weiner]

(In reply to Greg Edwards from comment #1)
> .cur files support multiple resolutions. How do they work at present?

I did a quick test with Firefox 22, and it seems to take the first, low resolution image and scale it up-- same undesirable behavior as with other image types.

Comment 3

[Greg Edwards]

Attachment: Testcase, PNG and CUR cursors in various sizes

I created a more in-depth testcase to show the problem. Each cursor conveniently has a number next to it showing its actual size in pixels. On Retina OSX, Firefox should pick the 64x64 image and show it at device pixels.

Safari also manages to get this wrong in roughly the same way, except they pick the 64x64 image and show it at double size.

Comment 4

[Greg Edwards]

Also see Bug 888781 for the sister issue on Windows.

Comment 5

[Ian Weiner]

Attachment: Modified test page

Greg, I made a small modification to your very nice test case attachment. In my version the "expected" cursor image is styled with CSS to display the 64x64 cursor when viewed on a Retina screen. While perhaps not "expected", this certainly would be the desired result, I think.

Comment 6

[Evan Wallace [:evanw]]

Attachment: Patch for OS X to render SVG cursors correctly on retina displays

Here's a patch that makes custom CSS cursors with SVG images work with retina displays on OS X. The patch also fixes an existing premultiplied/postmultiplied alpha mismatch in the Firefox custom cursor handling code on OS X, so the anti-aliased edges of cursors now have the correct brightness. I added Justin Dolske as a suggested reviewer because of his patch for bug 792592.

Note that this is different from how Chrome handles HiDPI custom CSS cursors, which is by specifying multiple images using -webkit-image-set. Firefox doesn't support image-set but it already has an image pipeline that supports vector images, so adding resolution-independent cursor support is easy. As a bonus, using a single SVG cursor opens the way for automatically supporting other DPI settings in the future. For example, while OS X only supports 100% and 200%, Windows 8 supports 100%, 125%, 150%, and 200%.

A test case is available at http://jsbin.com/unikOKiz. It works correctly on a Retina MacBook Pro with OS X 10.9 and an attached non-retina display.

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 8340251 [details] [diff] [review]
Patch for OS X to render SVG cursors correctly on retina displays

Review of attachment 8340251 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great!

Just a note: The diff has only 3 lines of context, which makes it a bit hard to read. Including these settings in your ~/.hgrc should increase context to 8 (not sure if all of them are needed):

[defaults]
diff = -U 8 -p
qdiff = -U 8

[diff]
git = 1
showfunc = 1
unified = 8

::: widget/cocoa/nsChildView.mm
@@ +914,2 @@
>    nsBaseWidget::SetCursor(aCursor, aHotspotX, aHotspotY);
> +  return [[nsCursorManager sharedInstance] setCursorWithImage:aCursor hotSpotX:aHotspotX hotSpotY:aHotspotY scaleFactor:scaleFactor];

Use BackingScaleFactor() instead of a temporary scaleFactor variable. (We have a BackingScaleFactor() method right here in the nsChildView class.)

::: widget/cocoa/nsCocoaUtils.mm
@@ +282,4 @@
>                               32,
>                               stride,
>                               colorSpace,
> +                             kCGBitmapByteOrder32Host | kCGImageAlphaPremultipliedFirst,

Good catch!

Comment 8

[Evan Wallace [:evanw]]

Attachment: Updated SVG cursor patch for OSX (use BackingScaleFactor)

Changed nsChildView::SetCursor() to use BackingScaleFactor(). Thanks for the review!

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4a366445b29

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8342537 [details] [diff] [review]
Updated SVG cursor patch for OSX (use BackingScaleFactor)

Just use the checkin-needed bug keyword in the future :)

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b4a366445b29

Comment 12

[Ioana (away)]

Ian, can you please test that this is fixed in Firefox 28:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/28.0b2-candidates/build1/mac/en-US/ ?

Comment 13

[Greg Edwards]

I'm getting incorrect results with the .CUR testcase. (Comment 5) The top-right panel is double its intended size and the top panel is still picking a 32x32 cursor.
(The bottom right panel should probably be 2x zoomed since it's a PNG without any scaling factor applied.)

.CUR files always have a logical size of 32x32 Cocoa pixels regardless of their resolution.

The .SVG testcase in Comment 6 appears to be correct.

30.0a1 (2014-02-08)
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0

Comment 14

[Markus Stange [:mstange]]

The patch in this bug only fixed the SVG case. I'll file a new bug for the general case.

Comment 15

[Markus Stange [:mstange]]

I've filed bug 999404 for HiDPI raster image cursors.

Comment 16

[Evan Wallace [:evanw]]

Raster cursor images kind of work already by embedding them in SVG. While it doesn't behave like -webkit-image-set, it still works pretty well: http://jsbin.com/puneripu. Retina and non-retina resolution can be handled by using a 2x raster image, which will be automatically downsampled on a non-retina display. The only issue is that Firefox seems to be using bicubic filtering instead of bilinear filtering, which results in a lower-quality 1x image.

Comment 17

[Markus Stange [:mstange]]

Good idea! That certainly reduces the urgency of fixing bug 999404.

Comment 18

[Greg Edwards]

Did this regress? SVG cursors are fuzzy for me again, including Comment 6's testcase.

Comment 19

[Markus Stange [:mstange]]

Indeed, it looks like I regressed. I filed bug 1304098 about it.

Comment 20

[Markus Stange [:mstange]]

I mean "it regressed".