[PATCH 2/2] comctl32/toolbar: Correctly draw disabled toolbar button which contains 32 bpp bitmap with alpha channel.

[Ziqing Hui]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=24784
Signed-off-by: Ziqing Hui <zhui(a)codeweavers.com>
---
 dlls/comctl32/tests/toolbar.c |  1 -
 dlls/comctl32/toolbar.c       | 62 ++++++++++++++++++++++++++++++++---
 2 files changed, 57 insertions(+), 6 deletions(-)

[Nikolay Sivov]

On 3/4/20 12:42 PM, Ziqing Hui wrote:
> +    has_alpha = FALSE;
> +    if (bmp.bmBitsPixel == 32)
> +    {
> +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> +        {
> +            if ((*pixel >> 24) != 0)
> +            {
> +                has_alpha = TRUE;
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (has_alpha)
> +    {
> +        /* gray the src image */
> +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> +        {
> +            DWORD alpha = (*pixel >> 24) & 0x000000ff;
> +            DWORD red   = (*pixel >> 16) & 0x000000ff;
> +            DWORD green = (*pixel >> 8)  & 0x000000ff;
> +            DWORD blue  = (*pixel)       & 0x000000ff;
> +            DWORD gray = (red * 299 + green * 587 + blue * 114) / 1000;
> +            gray = gray * alpha / 0xff;
> +            *pixel = ((alpha << 24) | (gray << 16) | (gray << 8) | gray);
> +        }
> +        bf.BlendOp = AC_SRC_OVER;
> +        bf.BlendFlags = 0;
> +        bf.SourceConstantAlpha = 0xff;
> +        bf.AlphaFormat = AC_SRC_ALPHA;
> +        GdiAlphaBlend(hdc, x, y, cx, cy, hdcImage, 0, 0, cx, cy, bf);
> +        goto done;
> +    }
> +
> I think it's possible ILS_SATURATE should be used for that. Have you 
> checked rendering on Windows with this flag?

[Ziqing Hui]

No, I didn't check it. I don't know this flag before. I'll give it a try.

On 3/4/20 6:04 PM, Nikolay Sivov wrote:
> On 3/4/20 12:42 PM, Ziqing Hui wrote:
> > +    has_alpha = FALSE;
> > +    if (bmp.bmBitsPixel == 32)
> > +    {
> > +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> > +        {
> > +            if ((*pixel >> 24) != 0)
> > +            {
> > +                has_alpha = TRUE;
> > +                break;
> > +            }
> > +        }
> > +    }
> > +
> > +    if (has_alpha)
> > +    {
> > +        /* gray the src image */
> > +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> > +        {
> > +            DWORD alpha = (*pixel >> 24) & 0x000000ff;
> > +            DWORD red   = (*pixel >> 16) & 0x000000ff;
> > +            DWORD green = (*pixel >> 8)  & 0x000000ff;
> > +            DWORD blue  = (*pixel)       & 0x000000ff;
> > +            DWORD gray = (red * 299 + green * 587 + blue * 114) / 1000;
> > +            gray = gray * alpha / 0xff;
> > +            *pixel = ((alpha << 24) | (gray << 16) | (gray << 8) | gray);
> > +        }
> > +        bf.BlendOp = AC_SRC_OVER;
> > +        bf.BlendFlags = 0;
> > +        bf.SourceConstantAlpha = 0xff;
> > +        bf.AlphaFormat = AC_SRC_ALPHA;
> > +        GdiAlphaBlend(hdc, x, y, cx, cy, hdcImage, 0, 0, cx, cy, bf);
> > +        goto done;
> > +    }
> > +
> > I think it's possible ILS_SATURATE should be used for that. Have you checked rendering on Windows with this flag?

[Ziqing Hui]

Hi Nikolay,

I tried rendering an 32-bit bitmap with ILS_SATURATE on my win10 machine, it does make the image gray as expected. So I think I should implement ILS_SATURATE for imagelist in wine first, then fix toolbar. What do you think?

Thanks,

Ziqing

On 3/4/20 6:28 PM, Ziqing Hui wrote:
> No, I didn't check it. I don't know this flag before. I'll give it a try.
>
> On 3/4/20 6:04 PM, Nikolay Sivov wrote:
> > On 3/4/20 12:42 PM, Ziqing Hui wrote:
> > > +    has_alpha = FALSE;
> > > +    if (bmp.bmBitsPixel == 32)
> > > +    {
> > > +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> > > +        {
> > > +            if ((*pixel >> 24) != 0)
> > > +            {
> > > +                has_alpha = TRUE;
> > > +                break;
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    if (has_alpha)
> > > +    {
> > > +        /* gray the src image */
> > > +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> > > +        {
> > > +            DWORD alpha = (*pixel >> 24) & 0x000000ff;
> > > +            DWORD red   = (*pixel >> 16) & 0x000000ff;
> > > +            DWORD green = (*pixel >> 8)  & 0x000000ff;
> > > +            DWORD blue  = (*pixel)       & 0x000000ff;
> > > +            DWORD gray = (red * 299 + green * 587 + blue * 114) / 1000;
> > > +            gray = gray * alpha / 0xff;
> > > +            *pixel = ((alpha << 24) | (gray << 16) | (gray << 8) | gray);
> > > +        }
> > > +        bf.BlendOp = AC_SRC_OVER;
> > > +        bf.BlendFlags = 0;
> > > +        bf.SourceConstantAlpha = 0xff;
> > > +        bf.AlphaFormat = AC_SRC_ALPHA;
> > > +        GdiAlphaBlend(hdc, x, y, cx, cy, hdcImage, 0, 0, cx, cy, bf);
> > > +        goto done;
> > > +    }
> > > +
> > > I think it's possible ILS_SATURATE should be used for that. Have you checked rendering on Windows with this flag?

[Nikolay Sivov]

On 3/4/20 6:35 PM, Ziqing Hui wrote:
> Hi Nikolay,
>
> I tried rendering an 32-bit bitmap with ILS_SATURATE on my win10 machine, it does make the image gray as expected. So I think I should implement ILS_SATURATE for imagelist in wine first, then fix toolbar. What do you think?
> Hi,

Sure, sounds good. Please add a test for it at imagelist level, small 
4x4 / 8x8 image is enough, showing that grayscale conversion happens.
> Thanks,
>
> Ziqing
>
> On 3/4/20 6:28 PM, Ziqing Hui wrote:
> > No, I didn't check it. I don't know this flag before. I'll give it a try.
> >
> > On 3/4/20 6:04 PM, Nikolay Sivov wrote:
> > > On 3/4/20 12:42 PM, Ziqing Hui wrote:
> > > > +    has_alpha = FALSE;
> > > > +    if (bmp.bmBitsPixel == 32)
> > > > +    {
> > > > +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> > > > +        {
> > > > +            if ((*pixel >> 24) != 0)
> > > > +            {
> > > > +                has_alpha = TRUE;
> > > > +                break;
> > > > +            }
> > > > +        }
> > > > +    }
> > > > +
> > > > +    if (has_alpha)
> > > > +    {
> > > > +        /* gray the src image */
> > > > +        for (pixel = bits; pixel < bits + cx * cy; pixel++)
> > > > +        {
> > > > +            DWORD alpha = (*pixel >> 24) & 0x000000ff;
> > > > +            DWORD red   = (*pixel >> 16) & 0x000000ff;
> > > > +            DWORD green = (*pixel >> 8)  & 0x000000ff;
> > > > +            DWORD blue  = (*pixel)       & 0x000000ff;
> > > > +            DWORD gray = (red * 299 + green * 587 + blue * 114) / 1000;
> > > > +            gray = gray * alpha / 0xff;
> > > > +            *pixel = ((alpha << 24) | (gray << 16) | (gray << 8) | gray);
> > > > +        }
> > > > +        bf.BlendOp = AC_SRC_OVER;
> > > > +        bf.BlendFlags = 0;
> > > > +        bf.SourceConstantAlpha = 0xff;
> > > > +        bf.AlphaFormat = AC_SRC_ALPHA;
> > > > +        GdiAlphaBlend(hdc, x, y, cx, cy, hdcImage, 0, 0, cx, cy, bf);
> > > > +        goto done;
> > > > +    }
> > > > +
> > > > I think it's possible ILS_SATURATE should be used for that. Have you checked rendering on Windows with this flag?