[Mesa-dev] [PATCH v2 1/2] egl/drm: Fix misused x and y offsets on swrast_put_image2() (v2)

[Eric Engestrom]

On Wednesday, 2017-07-19 19:25:27 +0900, Gwan-gyeong Mun wrote:
> It fixes misused x and y variables on the calculation of the memory copy regions.
> And it adds limits of the height and the width on the copy region.
> 
> v2 [Eric Engestrom]
>  - Add get_format_bpp() for handling of GBM_FORMATs. It is used for the
>    calculation of x_bytes and width_bytes.
>  - Polish the variable name.
> 
> Fixes: 8430af5ebe1ee8119e14 "Add support for swrast to the DRM EGL platform"
> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
> ---
>  src/egl/drivers/dri2/platform_drm.c | 48 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
> index 8e12aed0b3..00180deaf8 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -40,6 +40,29 @@
>  #include "egl_dri2_fallbacks.h"
>  #include "loader.h"
>  
> +static int
> +get_format_bpp(int format)

I was thinking of having that function in gbm.h (eg. `gbm_bo_get_bpp(gbm_bo)`),
but we can just move it in a later patch.

> +{
> +   int bpp;
> +
> +   switch (format) {
> +   case GBM_FORMAT_XRGB2101010:
> +   case GBM_FORMAT_ARGB2101010:
> +   case GBM_FORMAT_XRGB8888:
> +   case GBM_FORMAT_ARGB8888:
> +      bpp = 4;
> +      break;

No need for a temporary variable here, just `return 4;`.
R-b still stands, with this fixed (don't re-send just for this, I'll
amend locally when applying).

I'll leave the series sitting for a day or two for Emil, DanielS or
Giovanni to chime in in case I'm missing something :)

> +   case GBM_FORMAT_RGB565:
> +      bpp = 2;
> +      break;
> +   default:
> +      bpp = 0;
> +      break;
> +   }
> +
> +   return bpp;
> +}
> +
>  static struct gbm_bo *
>  lock_front_buffer(struct gbm_surface *_surf)
>  {
> @@ -525,6 +548,9 @@ swrast_put_image2(__DRIdrawable *driDrawable,
>     struct dri2_egl_surface *dri2_surf = loaderPrivate;
>     int internal_stride;
>     struct gbm_dri_bo *bo;
> +   int bpp;
> +   int x_bytes, width_bytes;
> +   char *src, *dst;
>  
>     if (op != __DRI_SWRAST_IMAGE_OP_DRAW &&
>         op != __DRI_SWRAST_IMAGE_OP_SWAP)
> @@ -534,14 +560,32 @@ swrast_put_image2(__DRIdrawable *driDrawable,
>        return;
>  
>     bo = gbm_dri_bo(dri2_surf->current->bo);
> +
> +   bpp = get_format_bpp(bo->base.format);
> +   if (bpp == 0)
> +      return;
> +
> +   x_bytes = x * bpp;
> +   width_bytes = width * bpp;
> +
>     if (gbm_dri_bo_map_dumb(bo) == NULL)
>        return;
>  
>     internal_stride = bo->base.stride;
>  
> +   if (height > dri2_surf->base.Height - y)
> +      height = dri2_surf->base.Height - y;
> +
> +   if (width_bytes > internal_stride - x_bytes)
> +      width_bytes = internal_stride - x_bytes;
> +
> +   dst = bo->map + x_bytes + (y * internal_stride);
> +   src = data;
> +
>     for (int i = 0; i < height; i++) {
> -      memcpy(bo->map + (x + i) * internal_stride + y,
> -             data + i * stride, stride);
> +      memcpy(dst, src, width_bytes);
> +      dst += internal_stride;
> +      src += stride;
>     }
>  
>     gbm_dri_bo_unmap_dumb(bo);
> -- 
> 2.13.3
>