[Mesa-dev] [PATCH 16/16] st/nine: Rework texture data allocation

[Ilia Mirkin]

On Fri, Apr 24, 2015 at 4:09 PM, Axel Davy <axel.davy at ens.fr> wrote:
> Some applications assume the memory for multilevel
> textures is allocated per continuous blocks.
>
> This patch implements that behaviour.
>
> Signed-off-by: Axel Davy <axel.davy at ens.fr>
> ---
>  src/gallium/state_trackers/nine/cubetexture9.c | 42 ++++++++++++++++------
>  src/gallium/state_trackers/nine/cubetexture9.h |  1 +
>  src/gallium/state_trackers/nine/nine_pipe.h    | 34 ++++++++++++++++++
>  src/gallium/state_trackers/nine/surface9.c     | 50 +++++++++++++-------------
>  src/gallium/state_trackers/nine/surface9.h     |  1 -
>  src/gallium/state_trackers/nine/texture9.c     | 18 ++++++++--
>  src/gallium/state_trackers/nine/texture9.h     |  1 +
>  7 files changed, 110 insertions(+), 37 deletions(-)
>
> diff --git a/src/gallium/state_trackers/nine/cubetexture9.c b/src/gallium/state_trackers/nine/cubetexture9.c
> index d81cc70..34ef4ec 100644
> --- a/src/gallium/state_trackers/nine/cubetexture9.c
> +++ b/src/gallium/state_trackers/nine/cubetexture9.c
> @@ -40,8 +40,9 @@ NineCubeTexture9_ctor( struct NineCubeTexture9 *This,
>      struct pipe_resource *info = &This->base.base.info;
>      struct pipe_screen *screen = pParams->device->screen;
>      enum pipe_format pf;
> -    unsigned i;
> +    unsigned i, l, f, offset, face_size = 0;
>      D3DSURFACE_DESC sfdesc;
> +    void *p;
>      HRESULT hr;
>
>      DBG("This=%p pParams=%p EdgeLength=%u Levels=%u Usage=%d "
> @@ -97,6 +98,14 @@ NineCubeTexture9_ctor( struct NineCubeTexture9 *This,
>          DBG("Application asked for Software Vertex Processing, "
>              "but this is unimplemented\n");
>
> +    if (Pool != D3DPOOL_DEFAULT) {
> +        face_size = nine_format_get_alloc_size(pf, EdgeLength, EdgeLength,
> +                                               info->last_level);
> +        This->managed_buffer = MALLOC(6 * face_size);
> +        if (!This->managed_buffer)
> +            return E_OUTOFMEMORY;
> +    }
> +
>      This->surfaces = CALLOC(6 * (info->last_level + 1), sizeof(*This->surfaces));
>      if (!This->surfaces)
>          return E_OUTOFMEMORY;
> @@ -117,16 +126,26 @@ NineCubeTexture9_ctor( struct NineCubeTexture9 *This,
>      sfdesc.Pool = Pool;
>      sfdesc.MultiSampleType = D3DMULTISAMPLE_NONE;
>      sfdesc.MultiSampleQuality = 0;
> -    for (i = 0; i < (info->last_level + 1) * 6; ++i) {
> -        sfdesc.Width = sfdesc.Height = u_minify(EdgeLength, i / 6);
> -
> -        hr = NineSurface9_new(This->base.base.base.device, NineUnknown(This),
> -                              This->base.base.resource, NULL, D3DRTYPE_CUBETEXTURE,
> -                              i / 6, i % 6,
> -                              &sfdesc, &This->surfaces[i]);
> -        if (FAILED(hr))
> -            return hr;
> +    /* We allocate the memory for the surfaces as continous blocks.
> +     * This is the expected behaviour, however we haven't tested for
> +     * cube textures in which order the faces/levels should be in memory
> +     */
> +    for (f = 0; f < 6; f++) {
> +        offset = f * face_size;
> +        for (l = 0; l < info->last_level + 1; l++) {

Elsewhere else you do <= info->last_level. Try to stay consistent...

> +            sfdesc.Width = sfdesc.Height = u_minify(EdgeLength, l);
> +            p = This->managed_buffer ? This->managed_buffer + offset +
> +                nine_format_get_p_offset(pf, EdgeLength, EdgeLength, l) :
> +                NULL;
> +
> +            hr = NineSurface9_new(This->base.base.base.device, NineUnknown(This),
> +                                  This->base.base.resource, p, D3DRTYPE_CUBETEXTURE,
> +                                  l, f, &sfdesc, &This->surfaces[f + 6 * l]);
> +            if (FAILED(hr))
> +                return hr;
> +        }
>      }
> +
>      for (i = 0; i < 6; ++i) /* width = 0 means empty, depth stays 1 */
>          This->dirty_rect[i].depth = 1;
>
> @@ -146,6 +165,9 @@ NineCubeTexture9_dtor( struct NineCubeTexture9 *This )
>          FREE(This->surfaces);
>      }
>
> +    if (This->managed_buffer)
> +        FREE(This->managed_buffer);
> +
>      NineBaseTexture9_dtor(&This->base);
>  }
>
> diff --git a/src/gallium/state_trackers/nine/cubetexture9.h b/src/gallium/state_trackers/nine/cubetexture9.h
> index e8594d3..ee7e275 100644
> --- a/src/gallium/state_trackers/nine/cubetexture9.h
> +++ b/src/gallium/state_trackers/nine/cubetexture9.h
> @@ -31,6 +31,7 @@ struct NineCubeTexture9
>      struct NineBaseTexture9 base;
>      struct NineSurface9 **surfaces;
>      struct pipe_box dirty_rect[6]; /* covers all mip levels */
> +    uint8_t *managed_buffer;
>  };
>  static INLINE struct NineCubeTexture9 *
>  NineCubeTexture9( void *data )
> diff --git a/src/gallium/state_trackers/nine/nine_pipe.h b/src/gallium/state_trackers/nine/nine_pipe.h
> index b8e728e..20916b7 100644
> --- a/src/gallium/state_trackers/nine/nine_pipe.h
> +++ b/src/gallium/state_trackers/nine/nine_pipe.h
> @@ -673,4 +673,38 @@ d3dtexturefiltertype_to_pipe_tex_mipfilter(D3DTEXTUREFILTERTYPE filter)
>      }
>  }
>
> +static INLINE unsigned nine_format_get_stride(enum pipe_format format,
> +                                              unsigned width)
> +{
> +    unsigned stride = util_format_get_stride(format, width);
> +
> +    return align(stride, 4);
> +}
> +
> +static INLINE unsigned nine_format_get_alloc_size(enum pipe_format format,
> +                                                  unsigned width,
> +                                                  unsigned height,
> +                                                  unsigned last_level)
> +{
> +    unsigned l, w, h, size = 0;
> +
> +    for (l = 0; l <= last_level; ++l) {
> +        w = u_minify(width, l);
> +        h = u_minify(height, l);
> +        size += nine_format_get_stride(format, w) *
> +            util_format_get_nblocksy(format, h);
> +    }
> +
> +    return size;
> +}
> +
> +static INLINE unsigned nine_format_get_p_offset(enum pipe_format format,
> +                                                unsigned width,
> +                                                unsigned height,
> +                                                unsigned level)
> +{
> +    if (!level)
> +        return 0;
> +    return nine_format_get_alloc_size(format, width, height, level-1);

This is going to be slow. Normally you just store the per-level
offsets in an array in the texture and look them up directly.

I'm having trouble understanding what you're changing here... perhaps
a better commit description could go a ways to clearing things up?
Were you allocating it a level at a time before? How did that work, I
don't see a per-level structure... I'm guessing there's a bit more
going on here.