[Mesa-dev] [PATCH] llvmpipe: fix lp_resource_copy using more than one 3d slice

Jose Fonseca jfonseca at vmware.com
Tue Feb 19 01:13:39 PST 2013


Thanks for fixing this Roland.

This is definitely an improvement. I'd recommend a few tweaks (it could even be as a follow on change):

- Calling llvmpipe_flush_resource() in a loop is overkill (it will call llvmpipe_flush() to be called many times needlessly). Please refactor llvmpipe_flush_resource() and llvmpipe_is_resource_referenced() to receive start_layer, end_layer pair.

- call util_copy_box instead of util_copy_rect

Jose


----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> These used to be illegal a very long time ago, then for some more time
> nothing really emitted these so this code path wasn't hit.
> Just trivially iterate over box->depth.
> (Might be worth refactoring at some point since nowadays all the code
> doesn't really do much except for depth textures.)
> 
> This fixes https://bugs.freedesktop.org/show_bug.cgi?id=61093
> ---
>  src/gallium/drivers/llvmpipe/lp_surface.c |  170
>  +++++++++++++++--------------
>  1 file changed, 86 insertions(+), 84 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_surface.c
> b/src/gallium/drivers/llvmpipe/lp_surface.c
> index 11475fd..dbaed95 100644
> --- a/src/gallium/drivers/llvmpipe/lp_surface.c
> +++ b/src/gallium/drivers/llvmpipe/lp_surface.c
> @@ -65,7 +65,7 @@ lp_resource_copy(struct pipe_context *pipe,
>     const enum pipe_format format = src_tex->base.format;
>     unsigned width = src_box->width;
>     unsigned height = src_box->height;
> -   assert(src_box->depth == 1);
> +   unsigned z;
>  
>     /* Fallback for buffers. */
>     if (dst->target == PIPE_BUFFER && src->target == PIPE_BUFFER) {
> @@ -74,99 +74,101 @@ lp_resource_copy(struct pipe_context *pipe,
>        return;
>     }
>  
> -   llvmpipe_flush_resource(pipe,
> -                           dst, dst_level, dstz,
> -                           FALSE, /* read_only */
> -                           TRUE, /* cpu_access */
> -                           FALSE, /* do_not_block */
> -                           "blit dest");
> -
> -   llvmpipe_flush_resource(pipe,
> -                           src, src_level, src_box->z,
> -                           TRUE, /* read_only */
> -                           TRUE, /* cpu_access */
> -                           FALSE, /* do_not_block */
> -                           "blit src");
> -
> -   /*
> -   printf("surface copy from %u lvl %u to %u lvl %u: %u,%u,%u to %u,%u,%u %u
> x %u x %u\n",
> -          src_tex->id, src_level, dst_tex->id, dst_level,
> -          src_box->x, src_box->y, src_box->z, dstx, dsty, dstz,
> -          src_box->width, src_box->height, src_box->depth);
> -   */
> -
> -   /* set src tiles to linear layout */
> -   {
> -      unsigned tx, ty, tw, th;
> -      unsigned x, y;
> -
> -      adjust_to_tile_bounds(src_box->x, src_box->y, width, height,
> -                            &tx, &ty, &tw, &th);
> -
> -      for (y = 0; y < th; y += TILE_SIZE) {
> -         for (x = 0; x < tw; x += TILE_SIZE) {
> -            (void) llvmpipe_get_texture_tile_linear(src_tex,
> -                                                    src_box->z, src_level,
> -                                                    LP_TEX_USAGE_READ,
> -                                                    tx + x, ty + y);
> +   for (z = 0; z < src_box->depth; z++){
> +      llvmpipe_flush_resource(pipe,
> +                              dst, dst_level, dstz + z,
> +                              FALSE, /* read_only */
> +                              TRUE, /* cpu_access */
> +                              FALSE, /* do_not_block */
> +                              "blit dest");
> +
> +      llvmpipe_flush_resource(pipe,
> +                              src, src_level, src_box->z + z,
> +                              TRUE, /* read_only */
> +                              TRUE, /* cpu_access */
> +                              FALSE, /* do_not_block */
> +                              "blit src");
> +
> +      /*
> +      printf("surface copy from %u lvl %u to %u lvl %u: %u,%u,%u to %u,%u,%u
> %u x %u x %u\n",
> +             src_tex->id, src_level, dst_tex->id, dst_level,
> +             src_box->x, src_box->y, src_box->z, dstx, dsty, dstz,
> +             src_box->width, src_box->height, src_box->depth);
> +      */
> +
> +      /* set src tiles to linear layout */
> +      {
> +         unsigned tx, ty, tw, th;
> +         unsigned x, y;
> +
> +         adjust_to_tile_bounds(src_box->x, src_box->y, width, height,
> +                               &tx, &ty, &tw, &th);
> +
> +         for (y = 0; y < th; y += TILE_SIZE) {
> +            for (x = 0; x < tw; x += TILE_SIZE) {
> +               (void) llvmpipe_get_texture_tile_linear(src_tex,
> +                                                       src_box->z + z,
> src_level,
> +                                                       LP_TEX_USAGE_READ,
> +                                                       tx + x, ty + y);
> +            }
>           }
>        }
> -   }
> -
> -   /* set dst tiles to linear layout */
> -   {
> -      unsigned tx, ty, tw, th;
> -      unsigned x, y;
> -      enum lp_texture_usage usage;
>  
> -      adjust_to_tile_bounds(dstx, dsty, width, height, &tx, &ty, &tw, &th);
> +      /* set dst tiles to linear layout */
> +      {
> +         unsigned tx, ty, tw, th;
> +         unsigned x, y;
> +         enum lp_texture_usage usage;
>  
> -      for (y = 0; y < th; y += TILE_SIZE) {
> -         boolean contained_y = ty + y >= dsty &&
> -                               ty + y + TILE_SIZE <= dsty + height ?
> -                               TRUE : FALSE;
> +         adjust_to_tile_bounds(dstx, dsty, width, height, &tx, &ty, &tw,
> &th);
>  
> -         for (x = 0; x < tw; x += TILE_SIZE) {
> -            boolean contained_x = tx + x >= dstx &&
> -                                  tx + x + TILE_SIZE <= dstx + width ?
> +         for (y = 0; y < th; y += TILE_SIZE) {
> +            boolean contained_y = ty + y >= dsty &&
> +                                  ty + y + TILE_SIZE <= dsty + height ?
>                                    TRUE : FALSE;
>  
> -            /*
> -             * Set the usage mode to WRITE_ALL for the tiles which are
> -             * completely contained by the dest rectangle.
> -             */
> -            if (contained_y && contained_x)
> -               usage = LP_TEX_USAGE_WRITE_ALL;
> -            else
> -               usage = LP_TEX_USAGE_READ_WRITE;
> -
> -            (void) llvmpipe_get_texture_tile_linear(dst_tex,
> -                                                    dstz, dst_level,
> -                                                    usage,
> -                                                    tx + x, ty + y);
> +            for (x = 0; x < tw; x += TILE_SIZE) {
> +               boolean contained_x = tx + x >= dstx &&
> +                                     tx + x + TILE_SIZE <= dstx + width ?
> +                                     TRUE : FALSE;
> +
> +               /*
> +                * Set the usage mode to WRITE_ALL for the tiles which are
> +                * completely contained by the dest rectangle.
> +                */
> +               if (contained_y && contained_x)
> +                  usage = LP_TEX_USAGE_WRITE_ALL;
> +               else
> +                  usage = LP_TEX_USAGE_READ_WRITE;
> +
> +               (void) llvmpipe_get_texture_tile_linear(dst_tex,
> +                                                       dstz + z, dst_level,
> +                                                       usage,
> +                                                       tx + x, ty + y);
> +            }
>           }
>        }
> -   }
>  
> -   /* copy */
> -   {
> -      const ubyte *src_linear_ptr
> -         = llvmpipe_get_texture_image_address(src_tex, src_box->z,
> -                                              src_level,
> -                                              LP_TEX_LAYOUT_LINEAR);
> -      ubyte *dst_linear_ptr
> -         = llvmpipe_get_texture_image_address(dst_tex, dstz,
> -                                              dst_level,
> -                                              LP_TEX_LAYOUT_LINEAR);
> -
> -      if (dst_linear_ptr && src_linear_ptr) {
> -         util_copy_rect(dst_linear_ptr, format,
> -                        llvmpipe_resource_stride(&dst_tex->base, dst_level),
> -                        dstx, dsty,
> -                        width, height,
> -                        src_linear_ptr,
> -                        llvmpipe_resource_stride(&src_tex->base, src_level),
> -                        src_box->x, src_box->y);
> +      /* copy */
> +      {
> +         const ubyte *src_linear_ptr
> +            = llvmpipe_get_texture_image_address(src_tex, src_box->z + z,
> +                                                 src_level,
> +                                                 LP_TEX_LAYOUT_LINEAR);
> +         ubyte *dst_linear_ptr
> +            = llvmpipe_get_texture_image_address(dst_tex, dstz + z,
> +                                                 dst_level,
> +                                                 LP_TEX_LAYOUT_LINEAR);
> +
> +         if (dst_linear_ptr && src_linear_ptr) {
> +            util_copy_rect(dst_linear_ptr, format,
> +                           llvmpipe_resource_stride(&dst_tex->base,
> dst_level),
> +                           dstx, dsty,
> +                           width, height,
> +                           src_linear_ptr,
> +                           llvmpipe_resource_stride(&src_tex->base,
> src_level),
> +                           src_box->x, src_box->y);
> +         }
>        }
>     }
>  }
> --
> 1.7.9.5
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 


More information about the mesa-dev mailing list