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

Roland Scheidegger sroland at vmware.com
Tue Feb 19 08:49:05 PST 2013


Am 19.02.2013 10:13, schrieb Jose Fonseca:
> 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.
Actually I guess I'll just drop the layer parameter completely. It is
passed through another function however in the end it is just unused and
thrown away anyway, so it doesn't matter if we check for whole resource
or just parts (of course at some point we might want to change this but
that's how it looks for now).


> 
> - call util_copy_box instead of util_copy_rect
Ah you're right I thought it wouldn't work outside the loop but it
should (not that it makes much difference since util_copy_box will just
call util_copy_rect repeatedly but it is definitely nicer style).

Roland


> 
> 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