[Mesa-dev] [PATCH 2/2] util: fix util_clear_render_target and util_clear_depth_stencil layer handling
Marek Olšák
maraeo at gmail.com
Fri Jun 7 03:14:59 PDT 2013
On Fri, Jun 7, 2013 at 2:32 AM, <sroland at vmware.com> wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> These functions must clear all bound layers, not just the first.
> ---
> src/gallium/auxiliary/util/u_surface.c | 190 +++++++++++++++++--------------
> src/gallium/auxiliary/util/u_transfer.c | 1 +
> 2 files changed, 104 insertions(+), 87 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_surface.c b/src/gallium/auxiliary/util/u_surface.c
> index 5c3a655..77d04ba 100644
> --- a/src/gallium/auxiliary/util/u_surface.c
> +++ b/src/gallium/auxiliary/util/u_surface.c
> @@ -307,6 +307,7 @@ no_src_map:
> * cpp > 4 looks like a gross hack at best...
> * Plus can't use these transfer fallbacks when clearing
> * multisampled surfaces for instance.
> + * Clears all bound layers.
> */
> void
> util_clear_render_target(struct pipe_context *pipe,
> @@ -316,8 +317,9 @@ util_clear_render_target(struct pipe_context *pipe,
> unsigned width, unsigned height)
> {
> struct pipe_transfer *dst_trans;
> - void *dst_map;
> + ubyte *dst_map;
> union util_color uc;
> + unsigned max_layer, layer;
>
> assert(dst->texture);
> if (!dst->texture)
> @@ -332,6 +334,7 @@ util_clear_render_target(struct pipe_context *pipe,
> unsigned pixstride = util_format_get_blocksize(dst->format);
> dx = (dst->u.buf.first_element + dstx) * pixstride;
> w = width * pixstride;
> + max_layer = 0;
> dst_map = pipe_transfer_map(pipe,
> dst->texture,
> 0, 0,
> @@ -340,14 +343,13 @@ util_clear_render_target(struct pipe_context *pipe,
> &dst_trans);
> }
> else {
> - /* XXX: should handle multiple layers */
> - dst_map = pipe_transfer_map(pipe,
> - dst->texture,
> - dst->u.tex.level,
> - dst->u.tex.first_layer,
> - PIPE_TRANSFER_WRITE,
> - dstx, dsty, width, height, &dst_trans);
> -
> + max_layer = dst->u.tex.last_layer - dst->u.tex.first_layer;
> + dst_map = pipe_transfer_map_3d(pipe,
> + dst->texture,
> + dst->u.tex.level,
> + PIPE_TRANSFER_WRITE,
> + dstx, dsty, dst->u.tex.first_layer,
> + width, height, max_layer, &dst_trans);
> }
>
> assert(dst_map);
> @@ -373,9 +375,13 @@ util_clear_render_target(struct pipe_context *pipe,
> else {
> util_pack_color(color->f, dst->format, &uc);
> }
> - util_fill_rect(dst_map, dst->format,
> - dst_trans->stride,
> - 0, 0, width, height, &uc);
> +
> + for (layer = 0; layer <= max_layer; layer++) {
> + util_fill_rect(dst_map, dst->format,
> + dst_trans->stride,
> + 0, 0, width, height, &uc);
> + dst_map += dst_trans->layer_stride;
> + }
>
> pipe->transfer_unmap(pipe, dst_trans);
> }
> @@ -386,6 +392,7 @@ util_clear_render_target(struct pipe_context *pipe,
> * sw fallback doesn't look terribly useful here.
> * Plus can't use these transfer fallbacks when clearing
> * multisampled surfaces for instance.
> + * Clears all bound layers.
> */
> void
> util_clear_depth_stencil(struct pipe_context *pipe,
> @@ -400,6 +407,7 @@ util_clear_depth_stencil(struct pipe_context *pipe,
> struct pipe_transfer *dst_trans;
> ubyte *dst_map;
> boolean need_rmw = FALSE;
> + unsigned max_layer, layer;
>
> if ((clear_flags & PIPE_CLEAR_DEPTHSTENCIL) &&
> ((clear_flags & PIPE_CLEAR_DEPTHSTENCIL) != PIPE_CLEAR_DEPTHSTENCIL) &&
> @@ -409,102 +417,110 @@ util_clear_depth_stencil(struct pipe_context *pipe,
> assert(dst->texture);
> if (!dst->texture)
> return;
> - dst_map = pipe_transfer_map(pipe,
> - dst->texture,
> - dst->u.tex.level,
> - dst->u.tex.first_layer,
> - (need_rmw ? PIPE_TRANSFER_READ_WRITE :
> - PIPE_TRANSFER_WRITE),
> - dstx, dsty, width, height, &dst_trans);
> +
> + max_layer = dst->u.tex.last_layer - dst->u.tex.first_layer;
> + dst_map = pipe_transfer_map_3d(pipe,
> + dst->texture,
> + dst->u.tex.level,
> + (need_rmw ? PIPE_TRANSFER_READ_WRITE :
> + PIPE_TRANSFER_WRITE),
> + dstx, dsty, dst->u.tex.first_layer,
> + width, height, max_layer + 1, &dst_trans);
> assert(dst_map);
>
> if (dst_map) {
> unsigned dst_stride = dst_trans->stride;
> uint64_t zstencil = util_pack64_z_stencil(format,
> depth, stencil);
> + ubyte *dst_layer = dst_map;
> unsigned i, j;
> assert(dst_trans->stride > 0);
>
> - switch (util_format_get_blocksize(format)) {
> - case 1:
> - assert(format == PIPE_FORMAT_S8_UINT);
> - if(dst_stride == width)
> - memset(dst_map, (uint8_t) zstencil, height * width);
> - else {
> - for (i = 0; i < height; i++) {
> - memset(dst_map, (uint8_t) zstencil, width);
> - dst_map += dst_stride;
> - }
> - }
> - break;
> - case 2:
> - assert(format == PIPE_FORMAT_Z16_UNORM);
> - for (i = 0; i < height; i++) {
> - uint16_t *row = (uint16_t *)dst_map;
> - for (j = 0; j < width; j++)
> - *row++ = (uint16_t) zstencil;
> - dst_map += dst_stride;
> + for (layer = 0; layer <= max_layer; layer++) {
> + dst_map = dst_layer;
> +
> + switch (util_format_get_blocksize(format)) {
> + case 1:
> + assert(format == PIPE_FORMAT_S8_UINT);
> + if(dst_stride == width)
> + memset(dst_map, (uint8_t) zstencil, height * width);
> + else {
> + for (i = 0; i < height; i++) {
> + memset(dst_map, (uint8_t) zstencil, width);
> + dst_map += dst_stride;
> + }
> }
> - break;
> - case 4:
> - if (!need_rmw) {
> + break;
> + case 2:
> + assert(format == PIPE_FORMAT_Z16_UNORM);
> for (i = 0; i < height; i++) {
> - uint32_t *row = (uint32_t *)dst_map;
> + uint16_t *row = (uint16_t *)dst_map;
> for (j = 0; j < width; j++)
> - *row++ = (uint32_t) zstencil;
> + *row++ = (uint16_t) zstencil;
> dst_map += dst_stride;
> + }
> + break;
> + case 4:
> + if (!need_rmw) {
> + for (i = 0; i < height; i++) {
> + uint32_t *row = (uint32_t *)dst_map;
> + for (j = 0; j < width; j++)
> + *row++ = (uint32_t) zstencil;
> + dst_map += dst_stride;
> + }
> }
> - }
> - else {
> - uint32_t dst_mask;
> - if (format == PIPE_FORMAT_Z24_UNORM_S8_UINT)
> - dst_mask = 0x00ffffff;
> else {
> - assert(format == PIPE_FORMAT_S8_UINT_Z24_UNORM);
> - dst_mask = 0xffffff00;
> - }
> - if (clear_flags & PIPE_CLEAR_DEPTH)
> - dst_mask = ~dst_mask;
> - for (i = 0; i < height; i++) {
> - uint32_t *row = (uint32_t *)dst_map;
> - for (j = 0; j < width; j++) {
> - uint32_t tmp = *row & dst_mask;
> - *row++ = tmp | ((uint32_t) zstencil & ~dst_mask);
> + uint32_t dst_mask;
> + if (format == PIPE_FORMAT_Z24_UNORM_S8_UINT)
> + dst_mask = 0x00ffffff;
> + else {
> + assert(format == PIPE_FORMAT_S8_UINT_Z24_UNORM);
> + dst_mask = 0xffffff00;
> + }
> + if (clear_flags & PIPE_CLEAR_DEPTH)
> + dst_mask = ~dst_mask;
> + for (i = 0; i < height; i++) {
> + uint32_t *row = (uint32_t *)dst_map;
> + for (j = 0; j < width; j++) {
> + uint32_t tmp = *row & dst_mask;
> + *row++ = tmp | ((uint32_t) zstencil & ~dst_mask);
> + }
> + dst_map += dst_stride;
> }
> - dst_map += dst_stride;
> }
> - }
> - break;
> - case 8:
> - if (!need_rmw) {
> - for (i = 0; i < height; i++) {
> - uint64_t *row = (uint64_t *)dst_map;
> - for (j = 0; j < width; j++)
> - *row++ = zstencil;
> - dst_map += dst_stride;
> + break;
> + case 8:
> + if (!need_rmw) {
> + for (i = 0; i < height; i++) {
> + uint64_t *row = (uint64_t *)dst_map;
> + for (j = 0; j < width; j++)
> + *row++ = zstencil;
> + dst_map += dst_stride;
> + }
> }
> - }
> - else {
> - uint64_t src_mask;
> -
> - if (clear_flags & PIPE_CLEAR_DEPTH)
> - src_mask = 0x00000000ffffffffull;
> - else
> - src_mask = 0x000000ff00000000ull;
> -
> - for (i = 0; i < height; i++) {
> - uint64_t *row = (uint64_t *)dst_map;
> - for (j = 0; j < width; j++) {
> - uint64_t tmp = *row & ~src_mask;
> - *row++ = tmp | (zstencil & src_mask);
> + else {
> + uint64_t src_mask;
> +
> + if (clear_flags & PIPE_CLEAR_DEPTH)
> + src_mask = 0x00000000ffffffffull;
> + else
> + src_mask = 0x000000ff00000000ull;
> +
> + for (i = 0; i < height; i++) {
> + uint64_t *row = (uint64_t *)dst_map;
> + for (j = 0; j < width; j++) {
> + uint64_t tmp = *row & ~src_mask;
> + *row++ = tmp | (zstencil & src_mask);
> + }
> + dst_map += dst_stride;
> }
> - dst_map += dst_stride;
> }
> + break;
> + default:
> + assert(0);
> + break;
> }
> - break;
> - default:
> - assert(0);
> - break;
> + dst_layer += dst_trans->layer_stride;
> }
>
> pipe->transfer_unmap(pipe, dst_trans);
> diff --git a/src/gallium/auxiliary/util/u_transfer.c b/src/gallium/auxiliary/util/u_transfer.c
> index 56e059b..7804f2a 100644
> --- a/src/gallium/auxiliary/util/u_transfer.c
> +++ b/src/gallium/auxiliary/util/u_transfer.c
> @@ -25,6 +25,7 @@ void u_default_transfer_inline_write( struct pipe_context *pipe,
> usage |= PIPE_TRANSFER_WRITE;
>
> /* transfer_inline_write implicitly discards the rewritten buffer range */
> + /* XXX this looks very broken for non-buffer resources having more than one dim. */
> if (box->x == 0 && box->width == resource->width0) {
Indeed, however radeon drivers ignore the discard flags for textures
and if the transfer is write-only, they behave as if DISCARD_RANGE was
set no matter what the flags are. It's a respond to state trackers not
having used the discard flags when they should (that is: always). I
propose to standardize this behavior, i.e. if PIPE_TRANSFER_READ is
not set for texture transfers, PIPE_TRANSFER_DISCARD_RANGE is implied.
Marek
> usage |= PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE;
> } else {
> --
> 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