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 <srol...@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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
