286306 - [os2] implement SetCursor(imgIContainer* cursor) so CSS cursor: url(...) works

Status: VERIFIED FIXED

(Core Graveyard :: Widget: OS/2, enhancement)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

to support urls for CSS cursor properties, nsIWidget::SetCursor(imgIContainer*)
needs to be implemented for the window.

bug 38447 implemented this for windows and gtk2.

Comment 1

[Rich Walsh]

FWIW...  I'm almost done implemting this.  It would help to have additional test
cases - in particular, images with 1 bit alpha (a monochrome image wouldn't hurt
either).  Any suggestions?

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

http://biesi.damowmow.com/cursor2.html - the "transparent gif" one uses 1bit alpha

not sure if a monochrome image is of interest, since all images in mozilla use a
24bit color depth in memory...

Comment 3

[Rich Walsh]

The attached patch implements this for OS/2.  While it's based on Christian's
code for Windows, it has been substantially modified to accommodate platform
differences and to improve efficiency.  In particular, it saves the cursor it
creates - along with an owning reference to the source image - and reuses it as
needed.  When a different CSS cursor is required or the window is destroyed, the
cached cursor is destroyed and the reference is released.

To test the implementation, go to
http://e-vertise.com/warpzilla/OS2_Pointer_Test.html  It too is based on
Christian's sample page but offers a wider range of formats and sizes.

Comment 4

[Rich Walsh]

Attachment: patch to implement CSS cursors on OS/2

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

"so the hotspot is roughly in the center of the image."

please don't do that. quoting CSS3 (http://www.w3.org/TR/css3-ui/#cursor):
| If both the values are unspecific and the referenced cursor has no defined 
| hotspot, the effect is as if a value of "0 0" were specified. 

Also, (0,0) is what the other platforms use as hotspot...

>When a different CSS cursor is required or the window is destroyed, the
>cached cursor is destroyed and the reference is released.

(there's a patch somewhere to do this for windows as well)

Comment 6

[Rich Walsh]

(In reply to comment #5)
> "so the hotspot is roughly in the center of the image."
> 
> please don't do that. quoting CSS3 (http://www.w3.org/TR/css3-ui/#cursor):
> | If both the values are unspecific and the referenced cursor has no defined 
> | hotspot, the effect is as if a value of "0 0" were specified. 

Standards can be so annoying :-)  By "0 0", I trust they mean "top left corner"
("0 31" in OS/2 terms).  Will do.

> >When a different CSS cursor is required or the window is destroyed, the
> >cached cursor is destroyed and the reference is released.
> 
> (there's a patch somewhere to do this for windows as well)

Many of us OS/2 users (like me) are stodgy old farts who dislike wasting
processor cycles on frou-frou, so I couldn't imagine recreating the pointer on
every mouse move.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

yeah, "0 0" is topleft:
"The x-coordinate and y-coordinate of the position in the cursor's coordinate
system (left/top relative) which represents the precise position that is being
pointed to."

Comment 8

[Rich Walsh]

Attachment: revised patch to implement CSS cursors

default hotspot location is now the upper-left corner of image

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 180421 [details] [diff] [review]
revised patch to implement CSS cursors

Nice job
r/sr=mkaply
a=mkaply for 1.8b2

Comment 10

[Mike Kaply [:mkaply]]

checked in.

Comment 11

[Peter Weilbacher]

I couldn't test this until now. It works nicely in most cases, really cool! But
I see that some of the test cases don't work. On Rich's page everything works
(apart from mygear.gif where the URL in the source is wrong), but on Christian's
page "png w/ alpha chan" and others supposed to show the sun.png symbol don't
work and I get the hand pointer instead.

Comment 12

[Rich Walsh]

(In reply to comment #11)
> on Christian's page "png w/ alpha chan" and others supposed to show the
> sun.png symbol don't work and I get the hand pointer instead.

By design.  All OS/2 mouse pointers are 32x32.  Taking an image that's several
hundred pixels wide or high & compressing it to 32x32 doesn't provide anything
useful.  Rather than having people complain about their pointer turning into
some unrecognizable splotch, I limited acceptable images to 128x128 max.

FWIW, all images on my test page work for me except the mozilla banner which was
included as a demonstration of the above logic (it's way too big).

Comment 13

[Peter Weilbacher]

I found the size limit in the code myself in the meantime, but hadn't noticed
that the sun.png was actually more than 200px wide. Increasing the limit to
256x256px, however, still gives quite a good result I think (although the
transparent part of that particular graphic suffers a bit). Well, it's probably
more work to change it now than it's worth. Verifying fixed.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 180421 [details] [diff] [review]
revised patch to implement CSS cursors

>+  if (hotspotY == (PRUint32)-1)
>+    hotspotY = height;

I assume this should have been:
  hotspotY = height - 1;
Otherwise it would be 1 pixel outside the image, no?

>+  else
>+    hotspotY = height - hotspotY - 1;

My patch on bug 286303 will effectively correct this.  If my assumption is
wrong, please comment on bug 286303.