[PATCH 2/8] comctl32/button: Implement NM_CUSTOMDRAW for Push Buttons

[Gabriel Ivăncescu]

On 3/4/19 8:19 PM, Nikolay Sivov wrote:
> On 3/4/19 7:25 PM, Gabriel Ivăncescu wrote:
> 
>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=10531
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>>
>> This fixes WinXP calc perfectly, it seems it's how Windows does it (yes,
>> the frame drawing is before pre-paint notification, otherwise it won't
>> look correctly).
>>
>> Tests for all of them are at the end of the patch series, because they 
>> are
>> done in an existing loop and it would be a nightmare otherwise to add 
>> todos
>> only for some buttons but not others. The MSDN documentation is wrong and
>> a mess for some CDRF_* values and many other info online gets it wrong,
>> as the tests demonstrate.
>>
>>   dlls/comctl32/button.c | 105 +++++++++++++++++++++++++++++++----------
>>   1 file changed, 79 insertions(+), 26 deletions(-)
>>
>> diff --git a/dlls/comctl32/button.c b/dlls/comctl32/button.c
>> index 56a4730..25a46a4 100644
>> --- a/dlls/comctl32/button.c
>> +++ b/dlls/comctl32/button.c
>> @@ -237,6 +237,30 @@ static inline WCHAR *get_button_text( const 
>> BUTTON_INFO *infoPtr )
>>       return buffer;
>>   }
>> +static void init_custom_draw(NMCUSTOMDRAW *nmcd, const BUTTON_INFO 
>> *info, HDC hDC, const RECT *rc)
>> +{
>> +    HWND hwnd = info->hwnd;
>> +
>> +    nmcd->hdr.hwndFrom = hwnd;
>> +    nmcd->hdr.idFrom   = GetWindowLongPtrW(hwnd, GWLP_ID);
>> +    nmcd->hdr.code     = NM_CUSTOMDRAW;
>> +    nmcd->hdc          = hDC;
>> +    nmcd->rc           = *rc;
>> +    nmcd->dwDrawStage  = CDDS_PREERASE;
>> +    nmcd->dwItemSpec   = 0;
>> +    nmcd->lItemlParam  = 0;
>> +    nmcd->uItemState   = !IsWindowEnabled(hwnd) ? CDIS_DISABLED : 0;
>> +    if (info->state & BST_PUSHED)  nmcd->uItemState |= CDIS_SELECTED;
>> +    if (info->state & BST_FOCUS)   nmcd->uItemState |= CDIS_FOCUS;
>> +    if (info->state & BST_HOT)     nmcd->uItemState |= CDIS_HOT;
>> +    if (info->state & BST_INDETERMINATE)
>> +        nmcd->uItemState |= CDIS_INDETERMINATE;
>> +
>> +    /* Windows doesn't seem to send CDIS_CHECKED (it fails the tests) */
>> +    /* CDIS_SHOWKEYBOARDCUES is misleading, as the meaning is 
>> reversed */
>> +    /* FIXME: Handle it properly when we support keyboard cues? */
>> +}
> For consistency, could you make it 'infoPtr' and 'hdc'? Other minor 
> simplifications, separate 'hwnd' variable seems unnecessary,
> and uItemState condition could be flipped to get rid of negation.
> 
> 
> 
> 
> 

Hi Nikolay,

Sure I'll do that and the other changes, thanks for the fast review. The 
local var is just a habit I have to shorten the code when accessing a 
field more than a couple times.

Now I'm trying to see if there's another way to avoid the 
double-painting special case in the tests, since it doesn't happen on my 
Win7 VM (but does on the testbot, for some reason).