[Mesa-dev] [RFC 2/3] gallium: Move nv50 clear_texture impl down to util_surface

[Ilia Mirkin]

On Thu, Dec 3, 2015 at 7:25 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 03.12.2015 um 23:48 schrieb Ilia Mirkin:
>> On Thu, Dec 3, 2015 at 4:44 AM, Edward O'Callaghan
>> <eocallaghan at alterapraxis.com> wrote:
>>> ARB_clear_texture is reasonably generic enough that it should
>>> be moved down to become part of the fallback mechanism of
>>> pipe->clear_texture.
>>>
>>> Signed-off-by: Edward O'Callaghan <eocallaghan at alterapraxis.com>
>>> ---
>>>  src/gallium/auxiliary/util/u_surface.c          | 83 +++++++++++++++++++++++++
>>>  src/gallium/auxiliary/util/u_surface.h          |  6 ++
>>>  src/gallium/drivers/nouveau/nv50/nv50_surface.c | 67 +-------------------
>>>  3 files changed, 90 insertions(+), 66 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/util/u_surface.c b/src/gallium/auxiliary/util/u_surface.c
>>> index 6aa44f9..e7ab175 100644
>>> --- a/src/gallium/auxiliary/util/u_surface.c
>>> +++ b/src/gallium/auxiliary/util/u_surface.c
>>> @@ -36,6 +36,7 @@
>>>  #include "pipe/p_screen.h"
>>>  #include "pipe/p_state.h"
>>>
>>> +#include "util/u_math.h"
>>>  #include "util/u_format.h"
>>>  #include "util/u_inlines.h"
>>>  #include "util/u_rect.h"
>>> @@ -547,6 +548,88 @@ util_clear_depth_stencil(struct pipe_context *pipe,
>>>     }
>>>  }
>>>
>>> +/**
>>> + * Fallback for pipe->clear_texture() function.
>>> + * clears a non-PIPE_BUFFER resource's specified level
>>> + * and bounding box with a clear value provided in that
>>> + * resource's native format.
>>> + *
>>> + * XXX sf->format = .. is problematic as hw need
>>> + * not nessarily support the format.
>>> + */
>>> +void
>>> +util_surface_clear_texture(struct pipe_context *pipe,
>>> +                           struct pipe_resource *res,
>>> +                           unsigned level,
>>> +                           const struct pipe_box *box,
>>> +                           const void *data)
>>> +{
>>> +   struct pipe_surface tmpl = {{0}}, *sf;
>>> +
>>> +   tmpl.format = res->format;
>>> +   tmpl.u.tex.first_layer = box->z;
>>> +   tmpl.u.tex.last_layer = box->z + box->depth - 1;
>>> +   tmpl.u.tex.level = level;
>>> +   sf = pipe->create_surface(pipe, res, &tmpl);
>>> +   if (!sf)
>>> +      return;
>>> +
>>> +   if (util_format_is_depth_or_stencil(res->format)) {
>>> +      float depth = 0;
>>> +      uint8_t stencil = 0;
>>> +      unsigned clear = 0;
>>> +      const struct util_format_description *desc =
>>> +         util_format_description(res->format);
>>> +
>>> +      if (util_format_has_depth(desc)) {
>>> +         clear |= PIPE_CLEAR_DEPTH;
>>> +         desc->unpack_z_float(&depth, 0, data, 0, 1, 1);
>>> +      }
>>> +      if (util_format_has_stencil(desc)) {
>>> +         clear |= PIPE_CLEAR_STENCIL;
>>> +         desc->unpack_s_8uint(&stencil, 0, data, 0, 1, 1);
>>> +      }
>>> +      pipe->clear_depth_stencil(pipe, sf, clear, depth, stencil,
>>> +                                box->x, box->y, box->width, box->height);
>>> +   } else {
>>> +      union pipe_color_union color;
>>> +
>>> +      switch (util_format_get_blocksizebits(res->format)) {
>>> +      case 128:
>>> +         sf->format = PIPE_FORMAT_R32G32B32A32_UINT;
>>> +         memcpy(&color.ui, data, 128 / 8);
>>> +         break;
>>> +      case 64:
>>> +         sf->format = PIPE_FORMAT_R32G32_UINT;
>>> +         memcpy(&color.ui, data, 64 / 8);
>>> +         memset(&color.ui[2], 0, 64 / 8);
>>> +         break;
>>> +      case 32:
>>> +         sf->format = PIPE_FORMAT_R32_UINT;
>>> +         memcpy(&color.ui, data, 32 / 8);
>>> +         memset(&color.ui[1], 0, 96 / 8);
>>> +         break;
>>> +      case 16:
>>> +         sf->format = PIPE_FORMAT_R16_UINT;
>>> +         color.ui[0] = util_cpu_to_le32(
>>> +            util_le16_to_cpu(*(unsigned short *)data));
>>> +         memset(&color.ui[1], 0, 96 / 8);
>>> +         break;
>>> +      case 8:
>>> +         sf->format = PIPE_FORMAT_R8_UINT;
>>> +         color.ui[0] = util_cpu_to_le32(*(unsigned char *)data);
>>> +         memset(&color.ui[1], 0, 96 / 8);
>>> +         break;
>>> +      default:
>>> +         assert(!"Unknown texel element size");
>>> +         return;
>>> +      }
>>> +
>>> +      pipe->clear_render_target(pipe, sf, &color,
>>> +                                box->x, box->y, box->width, box->height);
>>
>> I only recently realized this, but I'm fairly sure this is wrong --
>> needs to be divided by util_format_blockwidth/height, otherwise we'll
>> go way out of bounds for compressed formats. [And yes,
>> nv50_clear_texture has the same problem.]
> You can't clear compressed textures with ARB_clear_texture, so I guess
> this shouldn't be a problem.

Ah yes, indeed. That's why I didn't get any weird failurs :)

> Even if you think it should work in gallium, with the format illegally
> being changed after-the-fact (after creating the surface) it's not even
> worth thinking about what the correct dimensions should be (but yes if
> that would be fixed you'd be right)...

Yeah... that's why that stuff sat as an nv50 helper, not in generic
code. Figured it cheated too much for that... but if other drivers
want the same cheats... perhaps worth sticking somewhere common?

  -ilia