[PATCH] user32/cursoricon: Fix CreateIcon() bitmap mask handling.

[Zhiyi Zhang]

Thanks for reviewing.

On Tue 4 3 19:58, Huw Davies wrote:
> On Fri, Mar 30, 2018 at 07:32:31PM +0800, Zhiyi Zhang wrote:
>>
>> Signed-off-by: Zhiyi Zhang <zzhang at codeweavers.com>
>> ---
>>   dlls/user32/cursoricon.c       |  80 +++++++++++----------
>>   dlls/user32/tests/cursoricon.c | 160
>> ++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 187 insertions(+), 53 deletions(-)
>>
>>
> 
> There's a lot going on in this patch and the vague commit message doesn't
> help.  I'd suggest a patch that fixes CreateIconIndirect() first and
> a second patch to fix CreateIcon(), then write more description commit
> messages.
> 
Some of the logic in CreateIconIndirect() should be in CreateIcon(), so 
there is no easy way of fixing CreateIconIndirect() alone without 
breaking CreateIcon() and the tests.

More description is a good idea.

>> diff --git a/dlls/user32/cursoricon.c b/dlls/user32/cursoricon.c
>> index 8e272f09ce..69c7a937ff 100644
>> --- a/dlls/user32/cursoricon.c
>> +++ b/dlls/user32/cursoricon.c
>> @@ -92,6 +92,9 @@ struct animated_cursoricon_object
>>       HICON                    frames[1];  /* list of animated cursor frames */
>>   };
>>   
>> +static void stretch_blt_icon(HDC hdc_dst, int dst_x, int dst_y, int dst_width, int dst_height,
>> +                             HBITMAP src, int width, int height);
>> +
>>   static HBITMAP create_color_bitmap( int width, int height )
>>   {
>>       HDC hdc = get_display_dc();
>> @@ -1561,20 +1564,38 @@ HICON WINAPI CreateIcon(
>>   {
>>       ICONINFO iinfo;
>>       HICON hIcon;
>> +    HBITMAP hbmMask_upperhalf;
>> +    HBITMAP hbmMask_lowerhalf;
>> +    HDC hdc;
>>   
>>       TRACE_(icon)("%dx%d, planes %d, bpp %d, xor %p, and %p\n",
>>                    nWidth, nHeight, bPlanes, bBitsPixel, lpXORbits, lpANDbits);
>>   
>>       iinfo.fIcon = TRUE;
>> -    iinfo.xHotspot = nWidth / 2;
>> -    iinfo.yHotspot = nHeight / 2;
> 
> So this leaves x/yHotspot uninitialized...
No, because they get initialized later in CreateIconIndirect(), so I 
think there is no need to initialize them here.

> 
>> diff --git a/dlls/user32/tests/cursoricon.c b/dlls/user32/tests/cursoricon.c
>> index 5099c08d70..e5faa0050e 100644
>> --- a/dlls/user32/tests/cursoricon.c
>> +++ b/dlls/user32/tests/cursoricon.c
>> @@ -803,23 +803,27 @@ static void test_CreateIcon(void)
>>       void *bits;
>>       UINT display_bpp;
>>       int i;
>> +    BOOL is_color;
>>   
>>       hdc = GetDC(0);
>>       display_bpp = GetDeviceCaps(hdc, BITSPIXEL);
>> +    is_color = display_bpp > 1;
>>   
> 
> Did you really try this on a 1 bpp display?
No, I don't have a 1 bpp display to test it visually. However, I covered 
1 bpp bitmap in conformance tests, tested the result image width, 
height, bpp.

I also tested the patch against a sample application downloaded online 
which creates icon with masks with CreateIconIndirect(). And after this 
patch, the application is visually fine.

I could add some visual tests in conformance test if necessary.

> 
> Huw.
> 

Thanks,
Zhiyi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ColorCursor_Demo.zip
Type: application/octet-stream
Size: 20219 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20180403/b7b8583f/attachment-0001.obj>