[PATCH RFC xserver] glamor: Fix pixmap offset for bitplane in glamor_copy_fbo_cpu

[Michel Dänzer]

On 04/10/16 08:28 PM, Olivier Fourdan wrote:
> Commit cba28d5 - glamor: Handle bitplane in glamor_copy_fbo_cpu
> instroduced a regression as the computed pixmap offset would not match
> the qactual coordinates and write data elsewhere in memory causing a
> segfault in fbBltOne().
> 
> Translate the pixmap coordinates so that the data is read and written at
> the correct location.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97974
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  Note: This fixes the crash apparerntly and produces the expected output,
>        as tested with xterm's contectual menu, the check boxes are drawn
>        correctly and placed where they should be.
>        Yet, I am not familiar with this code so I have no idea if ths is 
>        the correct fix for the problem, or even a fix at all.
>        But it avoids the crash and the regression here.

I looked into this a little yesterday as well, and was heading towards
pretty much the same solution.

Some cosmetic comments below, but regardless of those, this patch is

Reviewed-and-Tested-by: Michel Dänzer <michel.daenzer at amd.com>


> diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
> index 8a329d2..1fd863a 100644
> --- a/glamor/glamor_copy.c
> +++ b/glamor/glamor_copy.c
> @@ -230,19 +230,22 @@ glamor_copy_cpu_fbo(DrawablePtr src,
>              goto bail;
>          }
>  
> +        src_pix->drawable.x = - dst->x;
> +        src_pix->drawable.y = - dst->y;

I'd drop the space between the minus sign and dst->x/y.


>          if (src->bitsPerPixel > 1)
>              fbCopyNto1(src, &src_pix->drawable, gc, box, nbox,
> -                       dst_xoff + dx, dst_yoff + dy, reverse, upsidedown,
> +                       dx, dy, reverse, upsidedown,
>                         bitplane, closure);
>          else
>              fbCopy1toN(src, &src_pix->drawable, gc, box, nbox,
> -                       dst_xoff + dx, dst_yoff + dy, reverse, upsidedown,
> +                       dx, dy, reverse, upsidedown,
>                         bitplane, closure);

The fbCopy*to* calls could be reformatted to fit in two lines now.


> -        glamor_upload_boxes(dst_pixmap, box, nbox, 0, 0, 0, 0,
> +        glamor_upload_boxes(dst_pixmap, box, nbox, src_xoff, src_yoff, dst_xoff, dst_yoff,
>                              (uint8_t *) src_bits, src_stride * sizeof(FbBits));

OTOH this line is too long now, so this call should be reformatted to
span three lines.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer