[PATCH] fb: Correctly implement CopyArea when using a window with depth 32 and 24bpp.

[Takashi Iwai]

At Fri, 6 Jun 2014 23:59:20 +1200,
Robert Ancell wrote:
> 
> This fixes gnome-terminal not showing in qemu [1] and can be triggered
> by the attached test case.
> 
> I haven't combined this with the patch in [2] because this patch is
> internally coherent with the fb logic; I still don't know if fb is
> doing the right thing here by allowing depth=32 bpp=24. The RENDER
> patch is not required for gnome-terminal (and I suspect any
> application using depth 32 visual and cairo/GTK+ to render) but
> follows the same logic.
> 
> --Robert
> 
> [1] http://lists.x.org/archives/xorg-devel/2014-June/042723.html
> [2] http://lists.x.org/archives/xorg-devel/2014-June/042721.html

I confirm that this patch fixes the problem on QEMU cirrus.

Tested-by: Takashi Iwai <tiwai at suse.de>

Can anyone review and take?  It's been a very long-standing and
annoying problem over years.


Thanks!

Takashi


> 
> On Fri, Jun 6, 2014 at 11:52 PM, Robert Ancell
> <robert.ancell at canonical.com> wrote:
> > When using the fb backend at 24bpp it allows a visual with 32 bit depth.
> > When using CopyArea from a 32bpp pixmap to a window with a 32 bit depth it would
> > read the ARGB as RGB.
> >
> > Fix is to correctly ignore the alpha channel in the pixmap when copying.
> > ---
> >  fb/fbcopy.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/fb/fbcopy.c b/fb/fbcopy.c
> > index 541ef71..5455947 100644
> > --- a/fb/fbcopy.c
> > +++ b/fb/fbcopy.c
> > @@ -242,8 +242,16 @@ fbCopyArea(DrawablePtr pSrcDrawable,
> >             int xIn, int yIn, int widthSrc, int heightSrc, int xOut, int yOut)
> >  {
> >      miCopyProc copy;
> > +    int src_bpp, dst_bpp;
> >
> > -    if (pSrcDrawable->bitsPerPixel != pDstDrawable->bitsPerPixel)
> > +    src_bpp = pSrcDrawable->bitsPerPixel;
> > +    if (src_bpp < pSrcDrawable->depth)
> > +        src_bpp = BitsPerPixel (pSrcDrawable->depth);
> > +    dst_bpp = pDstDrawable->bitsPerPixel;
> > +    if (dst_bpp < pDstDrawable->depth)
> > +        dst_bpp = BitsPerPixel (pDstDrawable->depth);
> > +
> > +    if (src_bpp != dst_bpp)
> >          copy = fb24_32CopyMtoN;
> >      else
> >          copy = fbCopyNtoN;
> > --
> > 2.0.0
> >