[Bug 18145] New: FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

Summary: FillRect: Undocumented feature
           Product: Wine
           Version: unspecified
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: trivial
          Priority: P2
         Component: user32
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=20608)
 --> (http://bugs.winehq.org/attachment.cgi?id=20608)
Test that show chess board under WinXP and gray field under wine.

I found that FillRect function have undocumented feature. It can receive 0 in
`hbrush' argument. In that case it use brush selected before by SelectObject.
I'm attached test program that can prove such behaviour.

This feature is used in some proprietary programs.

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |testcase

--- Comment #1 from Austin English   2009-04-22 11:22:56 ---
Bonus points if you can add that as a conformance test to the wine test suite:
http://wiki.winehq.org/ConformanceTests

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20608|application/octet-stream    |text/plain
          mime type|                            |

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

--- Comment #2 from Dmitry Timoshkov   2009-04-29 00:49:13 ---
Probably what happens is that FillRect() ignores an error of SelectObject()
when an invalid brush handle is passed. That needs a test case.

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

--- Comment #3 from Marintsev Miron   2009-04-29 05:18:28 ---
(In reply to comment #2)
> Probably what happens is that FillRect() ignores an error of SelectObject()
> when an invalid brush handle is passed. That needs a test case.

#1. FillRect && GetSysColorBrush
FillRect code:
...
if ( hbrush <= (HBRUSH) (COLOR_MAX + 1)) hbrush = GetSysColorBrush(
HandleToULong(hbrush) - 1 );
...

GetSysColorBrush( (unsigned) -1 ) returns NULL under windows, but wine returns
GetStockObject( LTGRAY_BRUSH ).

So it's necessary to add such code to dlls/user32/sysparams.c/GetSysColorBrush:
if( (unsigned int) -1 == index ) return NULL;

MSDN says:

Return Value

The return value identifies a logical brush if the nIndex parameter is
supported by the current platform. Otherwise, it returns NULL.

#2. SelectObject( hdc, 0 )
There is no problem with SelectObject( hdc, 0 ). It returns 0 and do nothing
both under Windows and wine.

#3. FillRect && SelectObject( hdc, 0 )

dlls/user32/uitools.c/FillRect code:

...
if ( !(prevBrush = SelectObject( hdc, hbrush ))) return 0;
PatBlt( hdc, rect->left, rect->top,
              rect->right - rect->left, rect->bottom - rect->top, PATCOPY );
SelectObject( hdc, prevBrush );
...

SelectObject( hdc, 0 ) will return 0, so prevBrush will be 0 and FillRect
returns 0, before drawing using PatBlt. It's improper.

According to item #2 it's safely to use SelectObject( hdc, 0 ) before and after
PatBlt, but lacks performance.

So this is my vision of FillRect reconstruction:
if ( hbrush && !(prevBrush = SelectObject( hdc, hbrush ))) return 0;
PatBlt( hdc, rect->left, rect->top,
              rect->right - rect->left, rect->bottom - rect->top, PATCOPY );
if( hbrush ) SelectObject( hdc, prevBrush );

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

--- Comment #4 from Austin English   2009-10-29 15:26:24 ---
Is this still an issue in current (1.1.32 or newer) wine?

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

--- Comment #5 from Marintsev Miron   2009-11-03 19:05:33 ---
(In reply to comment #4)
> Is this still an issue in current (1.1.32 or newer) wine?

Still yet:
wine-1.1.32-260-gf222a16

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

--- Comment #6 from Nikolay Sivov   2009-12-29 07:05:51 ---
(In reply to comment #3)
> (In reply to comment #2)
> > Probably what happens is that FillRect() ignores an error of SelectObject()
> > when an invalid brush handle is passed. That needs a test case.
>
> #1. FillRect && GetSysColorBrush
> FillRect code:
> ...
> if ( hbrush <= (HBRUSH) (COLOR_MAX + 1)) hbrush = GetSysColorBrush(
> HandleToULong(hbrush) - 1 );
> ...
>
> GetSysColorBrush( (unsigned) -1 ) returns NULL under windows, but wine returns
> GetStockObject( LTGRAY_BRUSH ).

I sent a test patch for GetSysColorBrush().

http://www.winehq.org/pipermail/wine-patches/2009-December/083287.html

BTW, what Wine version did you try first?

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #7 from Nikolay Sivov   2009-12-30 10:28:37 ---
This is fixed by 385b8dcb95b2853d452ec95d140ea5fa0c05aeef and
ff12594a155848ff2a12bb66e6d31ccdc16571b1.

[Bug 18145] FillRect: Undocumented feature

http://bugs.winehq.org/show_bug.cgi?id=18145

Jeff Zaroyko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #8 from Jeff Zaroyko   2010-01-09 04:48:37 ---
Closing bugs fixed in 1.1.36.