[Mesa-dev] [RFC 05/24] nvc0: compute correct image dimensions based on the target

[Samuel Pitoiset]

On 04/13/2016 10:23 PM, Ilia Mirkin wrote:
> On Tue, Apr 12, 2016 at 7:56 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> Loosely based on softpipe but with some changes, this computes correct
>> width, height and depth of different targets to help in processing
>> coordinates from the codegen part.
>>
>> This fixes a ton of dEQP/piglit tests.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 52 +++++++++++++++++++++++++++--
>>   1 file changed, 49 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>> index 7cac31d..fa89324 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>> @@ -743,6 +743,49 @@ static const uint8_t nve4_su_format_map[PIPE_FORMAT_COUNT];
>>   static const uint16_t nve4_su_format_aux_map[PIPE_FORMAT_COUNT];
>>   static const uint16_t nve4_suldp_lib_offset[PIPE_FORMAT_COUNT];
>>
>> +static void
>> +nvc0_get_surface_dims(struct pipe_image_view *view, int *width, int *height,
>> +                      int *depth)
>> +{
>> +   struct nv04_resource *res = nv04_resource(view->resource);
>> +   int level;
>> +
>> +   *width = *height = *depth = 1;
>> +   if (res->base.target == PIPE_BUFFER) {
>> +      *width = view->u.buf.last_element - view->u.buf.first_element + 1;
>> +      return;
>> +   }
>> +
>> +   level = view->u.tex.level;
>> +   *width = u_minify(view->resource->width0, level);
>
> You can safely do
>
> *height = u_minify(view->resource->height0, level);
>
> here and be just fine. height0 will be 1 for 1d textures. This should
> simplify the switch below as well.

Right, I think this will work.

>
>> +   switch (res->base.target) {
>> +   case PIPE_TEXTURE_1D_ARRAY:
>> +      *depth = view->u.tex.last_layer - view->u.tex.first_layer + 1;
>> +      /* fallthrough */
>> +   case PIPE_TEXTURE_1D:
>> +      break;
>> +   case PIPE_TEXTURE_CUBE:
>> +   case PIPE_TEXTURE_2D_ARRAY:
>> +      *depth = view->u.tex.last_layer - view->u.tex.first_layer + 1;
>> +      /* fallthrough */
>> +   case PIPE_TEXTURE_2D:
>> +   case PIPE_TEXTURE_RECT:
>> +      *height = u_minify(view->resource->height0, level);
>> +      break;
>> +   case PIPE_TEXTURE_3D:
>> +      *height = u_minify(view->resource->height0, level);
>> +      *depth  = u_minify(view->resource->depth0, level);
>> +      break;
>> +   case PIPE_TEXTURE_CUBE_ARRAY:
>> +      *height = u_minify(view->resource->height0, level);
>> +      *depth  = (view->u.tex.last_layer - view->u.tex.first_layer + 1) / 6;
>
> Are you sure? A cube array will be presented as, effectively, a 2d
> array... Same with cube above. I think for those, depth should be 6...

It works like this but I will double check. Cube arrays are not super 
tested.
>
>
>> +      break;
>> +   default:
>> +      assert(!"unexpected texture target");
>> +      break;
>> +   }
>> +}
>> +
>>   void
>>   nve4_set_surface_info(struct nouveau_pushbuf *push,
>>                         struct pipe_image_view *view,
>> @@ -772,9 +815,8 @@ nve4_set_surface_info(struct nouveau_pushbuf *push,
>>
>>      address = res->address;
>>
>> -   width = u_minify(view->resource->width0, view->u.tex.level);
>> -   height = u_minify(view->resource->height0, view->u.tex.level);
>> -   depth = u_minify(view->resource->depth0, view->u.tex.level);
>> +   /* get surface dimensions based on the target. */
>> +   nvc0_get_surface_dims(view, &width, &height, &depth);
>>
>>      info[8] = width;
>>      info[9] = height;
>> @@ -848,6 +890,10 @@ nve4_set_surface_info(struct nouveau_pushbuf *push,
>>               address += mt->layer_stride * z;
>>            }
>>         }
>> +
>> +      if (res->base.target == PIPE_TEXTURE_CUBE_ARRAY)
>> +         depth = view->u.tex.last_layer - view->u.tex.first_layer + 1;
>
> You partially fix this up here, but I think that depth should be / 6
> in the one place that needs that. Most places will want the full layer
> "span".
>
> Also note that the resource's target is irrelevant with texture views.
> I might have created a 2d array, and cast it as a cube array. I don't
> think anything's different as far as images are concerned.

Oh, really? That sounds bad.
Well, yeah this patch needs some changes.

>
>    -ilia
>
>> +
>>         info[0]  = address >> 8;
>>         info[2]  = width - 1;
>>         /* NOTE: this is really important: */
>> --
>> 2.8.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev