Icons as cursors patch

[Krzysztof Drewniak]

This is my first patch. It allows icons to be loaded as cursors because
it defaults hotspots to x = 0 and y = 0 if the hotspot isn't found.
Pasting patch into body of message. This is against the latest git .

--------------------------------------------------------------------------------------------------------------------------

--- cursoricon.c.orig    2008-11-23 15:07:32.000000000 -0600
+++ cursoricon.c    2008-11-23 15:13:22.000000000 -0600
@@ -932,7 +932,10 @@ static HICON CURSORICON_LoadFromFile( LP
         entry = CURSORICON_FindBestIconFile( dir, width, height, colors );

if ( !entry )
-        goto end;
+        /* this is my change. If there is no hotspot, default to 0, 0. */
+        hotspot.x = 0;
+        hotspot.y = 0;
+        goto iconskip;

/* check that we don't run off the end of the file */
     if ( entry->dwDIBOffset > filesize )
@@ -942,6 +945,7 @@ static HICON CURSORICON_LoadFromFile( LP

hotspot.x = entry->xHotspot;
     hotspot.y = entry->yHotspot;
+iconskip:
     hIcon = CURSORICON_CreateIconFromBMI( (BITMAPINFO
*)&bits[entry->dwDIBOffset],
                       hotspot, !fCursor, 0x00030000,
                       width, height, loadflags );

--------------------------------------------------------------------------------------------------------------------------

Please look over the code.
Krzys D.

[Stefan Dösinger]

Hi,
> This is my first patch. It allows icons to be loaded as cursors because
> it defaults hotspots to x = 0 and y = 0 if the hotspot isn't found.
> Pasting patch into body of message. This is against the latest git .

I recommend to write a test case to show that Windows behaves in the same
way.

[Krzysztof Drewniak]

I know this isn't a coded test, bot The Sims 2 behave in this way. This
was previously stated WAY back on this list.

Stefan Dösinger wrote:
> Hi,
> > This is my first patch. It allows icons to be loaded as cursors because
> > it defaults hotspots to x = 0 and y = 0 if the hotspot isn't found.
> > Pasting patch into body of message. This is against the latest git .
>
> I recommend to write a test case to show that Windows behaves in the same
> way.

[Roderick Colenbrander]

A real small test app if possible as a wine test is needed to demonstrate it is really correct. Sorry it can be a pain but it is to prevent issues in other apps which might be very hard to debug if an incorrect patch enters Wine.

Roderick
> I know this isn't a coded test, bot The Sims 2 behave in this way. This
> was previously stated WAY back on this list.
>
> Stefan Dösinger wrote:
> > Hi,
> > > This is my first patch. It allows icons to be loaded as cursors because
> > > it defaults hotspots to x = 0 and y = 0 if the hotspot isn't found.
> > > Pasting patch into body of message. This is against the latest git .
> >
> > I recommend to write a test case to show that Windows behaves in the
>
> same
> > way.

[Stefan Dösinger]

> I know this isn't a coded test, bot The Sims 2 behave in this way. This

was > previously stated WAY back on this list.
Yes, but do you know if Sims 2 behaves that way on Windows? There could be
some other bug triggering this behavior in Sims.

That's where a test helps even in these cases: If yout test shows that your
change is correct, it confirms the Sims behavior. If your test shows that
your change is wrong, you know that you found the wrong bug.

[Krzysztof Drewniak]

Isn't this already covered in the cursoricon tests?

Stefan Dösinger wrote:
> > I know this isn't a coded test, bot The Sims 2 behave in this way. This
>
> was > previously stated WAY back on this list.
> Yes, but do you know if Sims 2 behaves that way on Windows? There could be
> some other bug triggering this behavior in Sims.
>
> That's where a test helps even in these cases: If yout test shows that your
> change is correct, it confirms the Sims behavior. If your test shows that
> your change is wrong, you know that you found the wrong bug.

[Stefan Dösinger]

> Isn't this already covered in the cursoricon tests?

I don't think so. That test would be marked TODO_WINE in that case, so your
patch would cause a test failure(unexpected success). If that test wasn't
marked todo the behavior your patch adds would be implemented already.

[Andrew Riedi]

On Tue, Nov 25, 2008 at 4:04 AM, Stefan Dösinger  wrote:
> > Isn't this already covered in the cursoricon tests?
>
> I don't think so. That test would be marked TODO_WINE in that case, so your
> patch would cause a test failure(unexpected success). If that test wasn't
> marked todo the behavior your patch adds would be implemented already.

A patch I sent in recently (attached for convenience) should let Wine
know that it is dealing with a cursor loaded from an .ico.  After this
gets commited, then we can see where the hotspot should be for such a
cursor (after Wine knows about it internally.)  If I recall correctly,
GetIconInfo() returns (width/2, height/2) for the hotspot of all
.ico's.  I will write a test for this.

Even so, the cursor hotspot returned by GetIconInfo() might very will
be different from what Win32 actually uses when drawing the cursor, so
I will have to write another interactive test for this so I can see
what Win32 actually does.  The interactive test, unless specifically
asked for, I probably won't send in because it is impossible to
automate.  (Just check manually by clicking on stuff where the hotspot
of a cursor loaded from a .ico is. - Hopefully this will match
GetIconInfo() behavior, or at least use (0,0).)

Krzysztof: Please test the attached patch against your program/game to
see what, if anything, it does to fix this issue.