[PATCH xserver v2] glamor: Handle bitplane in glamor_copy_fbo_cpu

[walter harms]

Am 15.08.2016 11:43, schrieb Michel Dänzer:
> From: Michel Dänzer <michel.daenzer at amd.com>
> 
> This can significantly speed up at least some CopyPlane cases, e.g.
> indirectly for stippled fills.
> 
> v2:
> * Make temporary pixmap the same size as the destination pixmap
>   (instead of the destination drawable size), and fix coordinate
>   parameters passed to fbCopyXtoX and glamor_upload_boxes. Fixes
>   incorrect rendering rendering with x11perf -copyplane* and crashes
>   with the xscreensaver phosphor hack.
> 
> Reported-by: Keith Raghubar <keith.raghubar at amd.com>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>  glamor/glamor_copy.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
> index 3501a0d..82e040a 100644
> --- a/glamor/glamor_copy.c
> +++ b/glamor/glamor_copy.c
> @@ -222,9 +222,40 @@ glamor_copy_cpu_fbo(DrawablePtr src,
>  
>      fbGetDrawable(src, src_bits, src_stride, src_bpp, src_xoff, src_yoff);
>  
> -    glamor_upload_boxes(dst_pixmap, box, nbox, src_xoff + dx, src_yoff + dy,
> -                        dst_xoff, dst_yoff,
> -                        (uint8_t *) src_bits, src_stride * sizeof (FbBits));


my i suggest to invert the question to improve readability ?

	if (!bitplane) {
	            glamor_upload_boxes(dst_pixmap, box, nbox, src_xoff + dx, src_yoff + dy,
                            dst_xoff, dst_yoff,
                            (uint8_t *) src_bits, src_stride * sizeof (FbBits));
                    glamor_finish_access(src);
	            return TRUE;
	      }

btw: i hope return TRUE is intended here

and then the rest ...

just my 2 cents

re,
 wh

> +    if (bitplane) {
> +        FbBits *tmp_bits;
> +        FbStride tmp_stride;
> +        int tmp_bpp;
> +        _X_UNUSED int tmp_xoff, tmp_yoff;
> +        PixmapPtr tmp_pix = fbCreatePixmap(screen, dst_pixmap->drawable.width,
> +                                           dst_pixmap->drawable.height,
> +                                           dst->depth, 0);
> +
> +        if (!tmp_pix) {
> +            glamor_finish_access(src);
> +            goto bail;
> +        }
> +
> +        fbGetDrawable(&tmp_pix->drawable, tmp_bits, tmp_stride, tmp_bpp, tmp_xoff,
> +                      tmp_yoff);
> +
> +        if (src->bitsPerPixel > 1)
> +            fbCopyNto1(src, &tmp_pix->drawable, gc, box, nbox,
> +                       dst_xoff + dx, dst_yoff + dy, reverse, upsidedown,
> +                       bitplane, closure);
> +        else
> +            fbCopy1toN(src, &tmp_pix->drawable, gc, box, nbox,
> +                       dst_xoff + dx, dst_yoff + dy, reverse, upsidedown,
> +                       bitplane, closure);
> +
> +        glamor_upload_boxes(dst_pixmap, box, nbox, 0, 0, 0, 0,
> +                            (uint8_t *) tmp_bits, tmp_stride * sizeof(FbBits));
> +
> +        fbDestroyPixmap(tmp_pix);
> +    } else
> +        glamor_upload_boxes(dst_pixmap, box, nbox, src_xoff + dx, src_yoff + dy,
> +                            dst_xoff, dst_yoff,
> +                            (uint8_t *) src_bits, src_stride * sizeof (FbBits));
>      glamor_finish_access(src);
>  
>      return TRUE;
> @@ -616,9 +647,9 @@ glamor_copy_gl(DrawablePtr src,
>                  return glamor_copy_fbo_fbo_draw(src, dst, gc, box, nbox, dx, dy,
>                                                  reverse, upsidedown, bitplane, closure);
>          }
> -        if (bitplane == 0)
> -            return glamor_copy_cpu_fbo(src, dst, gc, box, nbox, dx, dy,
> -                                       reverse, upsidedown, bitplane, closure);
> +
> +        return glamor_copy_cpu_fbo(src, dst, gc, box, nbox, dx, dy,
> +                                   reverse, upsidedown, bitplane, closure);
>      } else if (GLAMOR_PIXMAP_PRIV_HAS_FBO(src_priv) &&
>                 dst_priv->type != GLAMOR_DRM_ONLY &&
>                 bitplane == 0) {