Re: winex11.drv: Implement FIXME in AlphaBlend

[Roderick Colenbrander]

Hi Peter,

I didn't see this patch before because I was away at that time. The
SourceConstantAlpha part looks correct to me. It might make sense to
calculate blendfn.SourceConstantAlpha / 255 before the loop, so that
it isn't recalculated each time.

The other half of your patch is a separate change and it would require
a separate patch. The part looks suspicious though. I don't understand
why you need special locking there.

Roderick

On Mon, Oct 26, 2009 at 4:39 PM, Peter Urbanec  wrote:
> commit df78de34a06e32be9f37a3b34fe68e5b088a8578
> Author: Peter Urbanec 
> Date:   Mon Oct 26 21:29:00 2009 +1100
>
> Implement source alpha plus SourceConstantAlpha combined transparency.
>    Add DIB locking to prevent node icon rendering errors in Fusion composition flow views.
>
> diff --git a/dlls/winex11.drv/xrender.c b/dlls/winex11.drv/xrender.c
> index 90503bf..26dc262 100644
> --- a/dlls/winex11.drv/xrender.c
> +++ b/dlls/winex11.drv/xrender.c
> @@ -1956,9 +1956,6 @@ BOOL CDECL X11DRV_AlphaBlend(X11DRV_PDEVICE *devDst, INT xDst, INT yDst, INT wid
>         return FALSE;
>     }
> - if ((blendfn.AlphaFormat & AC_SRC_ALPHA) && blendfn.SourceConstantAlpha != 0xff)
> - FIXME("Ignoring SourceConstantAlpha %d for AC_SRC_ALPHA\n", blendfn.SourceConstantAlpha);
> -
>
> if(dib.dsBm.bmBitsPixel != 32) {
>         FIXME("not a 32 bpp dibsection\n");
>         return FALSE;
> @@ -1979,11 +1976,35 @@ BOOL CDECL X11DRV_AlphaBlend(X11DRV_PDEVICE *devDst, INT xDst, INT yDst, INT wid
>
> if (blendfn.AlphaFormat & AC_SRC_ALPHA)
>     {
> - for(; y >= y2; y--)
> - if (blendfn.SourceConstantAlpha == 0xff)
>
> {
> - memcpy(dstbits, (char *)dib.dsBm.bmBits + y * dib.dsBm.bmWidthBytes + xSrc * 4,
> - for(; y >= y2; y--)
> - {
> - memcpy(dstbits, (char *)dib.dsBm.bmBits + y * dib.dsBm.bmWidthBytes + xSrc * 4,
>
> widthSrc * 4);
> - dstbits += (top_down ? -1 : 1) * widthSrc;
> - dstbits += (top_down ? -1 : 1) * widthSrc;
> - }
> - }
> - else
> - {
> - /* Source alpha plus SourceConstantAlpha */
> - for(; y >= y2; y--)
> - {
> - int x;
> - DWORD *srcbits = (DWORD *)((char *)dib.dsBm.bmBits + y * dib.dsBm.bmWidthBytes) + xSrc;
> - for (x = 0; x < widthSrc; x++)
> - {
> - DWORD argb = *srcbits++;
> - BYTE *s = (BYTE *) &argb;
> - s[0] = (s[0] * blendfn.SourceConstantAlpha) / 255;
> - s[1] = (s[1] * blendfn.SourceConstantAlpha) / 255;
> - s[2] = (s[2] * blendfn.SourceConstantAlpha) / 255;
> - s[3] = (s[3] * blendfn.SourceConstantAlpha) / 255;
> - *dstbits++ = argb;
> - }
> - if (top_down)  /* we traversed the row forward so we should go back by two rows */
> - dstbits -= 2 * widthSrc;
> - }
>
> }
>     }
>     else
> @@ -2006,6 +2027,7 @@ BOOL CDECL X11DRV_AlphaBlend(X11DRV_PDEVICE *devDst, INT xDst, INT yDst, INT wid
>
> }
> - X11DRV_LockDIBSection( devDst, DIB_Status_GdiMod );
>
> dst_pict = get_xrender_picture(devDst);
>
> wine_tsx11_lock();
> @@ -2017,6 +2039,8 @@ BOOL CDECL X11DRV_AlphaBlend(X11DRV_PDEVICE *devDst, INT xDst, INT yDst, INT wid
>     if(!src_format)
>     {
>         WARN("Unable to find a picture format supporting alpha, make sure X is running at 24-bit\n");
> - wine_tsx11_unlock();
> - X11DRV_UnlockDIBSection( devDst, TRUE );
>
> return FALSE;
>     }
>
> @@ -2055,6 +2079,7 @@ BOOL CDECL X11DRV_AlphaBlend(X11DRV_PDEVICE *devDst, INT xDst, INT yDst, INT wid
>     XDestroyImage(image);
>
> wine_tsx11_unlock();
> - X11DRV_UnlockDIBSection( devDst, TRUE );
>
> HeapFree(GetProcessHeap(), 0, data);
>     return TRUE;
>  }