[Mesa-dev] [PATCH v2] st/mesa: directly compute level=0 texture size in st_finalize_texture

Nicolai Hähnle nicolai.haehnle at amd.com
Tue Jun 7 23:07:22 UTC 2016


On 08.06.2016 01:00, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> The width0/height0/depth0 on stObj may not have been set at this point.
> Observed in a trace that set up levels 2..9 of a 2d texture, and set the base
> level to 2, with height 1. This made the guess logic always bail.
>
> Originally investigated by Ilia Mirkin, this patch gets rid of the somewhat
> redundant storage of width0/height0/depth0 and makes sure we always compute
> pipe texture sizes that are compatible with the base level image of the
> GL texture.
>
> Fixes the gl-1.2-texture-base-level piglit test provided by Brian Paul.
>
> v2:
> - try to re-use an existing pipe texture when possible
> - handle a corner case where the base level is not level 0 and it is of
>    size 1x1x1

Note how this patch reverses the relationship between the old stObj->pt 
size and the guess. Previously, the old size was used when the guess 
failed (incorrectly, since there might not have been an old size). Now, 
the old size is _preferred_.

I understood the relevance of this when looking at piglit's levelclamp 
test. st_finalize_texture is called after a base level change -- now 
imagine that the level 0 image provided by the user has an odd size, and 
then the user sets the base level to > 0. The old code ended up 
needlessly guessing the (incorrect) level 0 size in this situation and 
therefore re-allocated the texture needlessly.

Cheers,
Nicolai

>
> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
> --
> Tested with Piglit on radeonsi, no change except for the
> gl-1.2-texture-base-level test.
> ---
>   src/mesa/state_tracker/st_cb_eglimage.c |  3 --
>   src/mesa/state_tracker/st_cb_texture.c  | 86 ++++++++++++++++++---------------
>   src/mesa/state_tracker/st_manager.c     |  3 --
>   src/mesa/state_tracker/st_texture.h     |  6 ---
>   src/mesa/state_tracker/st_vdpau.c       |  3 --
>   5 files changed, 48 insertions(+), 53 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_eglimage.c b/src/mesa/state_tracker/st_cb_eglimage.c
> index 8531afb..1782d15 100644
> --- a/src/mesa/state_tracker/st_cb_eglimage.c
> +++ b/src/mesa/state_tracker/st_cb_eglimage.c
> @@ -128,9 +128,6 @@ st_bind_surface(struct gl_context *ctx, GLenum target,
>      st_texture_release_all_sampler_views(st, stObj);
>      pipe_resource_reference(&stImage->pt, stObj->pt);
>
> -   stObj->width0 = ps->width;
> -   stObj->height0 = ps->height;
> -   stObj->depth0 = 1;
>      stObj->surface_format = ps->format;
>
>      _mesa_dirty_texobj(ctx, texObj);
> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
> index 44e21b1..3a3d6b9 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -493,7 +493,6 @@ guess_and_alloc_texture(struct st_context *st,
>
>      if (!guessed_box) {
>         /* we can't determine the image size at level=0 */
> -      stObj->width0 = stObj->height0 = stObj->depth0 = 0;
>         /* this is not an out of memory error */
>         return GL_TRUE;
>      }
> @@ -518,11 +517,6 @@ guess_and_alloc_texture(struct st_context *st,
>         lastLevel = 0;
>      }
>
> -   /* Save the level=0 dimensions */
> -   stObj->width0 = width;
> -   stObj->height0 = height;
> -   stObj->depth0 = depth;
> -
>      fmt = st_mesa_format_to_pipe_format(st, stImage->base.TexFormat);
>
>      bindings = default_bindings(st, fmt);
> @@ -2443,9 +2437,6 @@ st_finalize_texture(struct gl_context *ctx,
>         if (st_obj->buffer != stObj->pt) {
>            pipe_resource_reference(&stObj->pt, st_obj->buffer);
>            st_texture_release_all_sampler_views(st, stObj);
> -         stObj->width0 = stObj->pt->width0 / _mesa_get_format_bytes(tObj->_BufferObjectFormat);
> -         stObj->height0 = 1;
> -         stObj->depth0 = 1;
>         }
>         return GL_TRUE;
>
> @@ -2478,25 +2469,44 @@ st_finalize_texture(struct gl_context *ctx,
>      /* Find size of level=0 Gallium mipmap image, plus number of texture layers */
>      {
>         GLuint width, height, depth;
> -      if (!guess_base_level_size(stObj->base.Target,
> -                                 firstImage->base.Width2,
> -                                 firstImage->base.Height2,
> -                                 firstImage->base.Depth2,
> -                                 firstImage->base.Level,
> -                                 &width, &height, &depth)) {
> -         width = stObj->width0;
> -         height = stObj->height0;
> -         depth = stObj->depth0;
> +
> +      st_gl_texture_dims_to_pipe_dims(stObj->base.Target,
> +                                      firstImage->base.Width2,
> +                                      firstImage->base.Height2,
> +                                      firstImage->base.Depth2,
> +                                      &width, &height, &depth, &ptLayers);
> +
> +      /* If we previously allocated a pipe texture and its sizes are
> +       * compatible, use them.
> +       */
> +      if (stObj->pt &&
> +          u_minify(stObj->pt->width0, firstImage->base.Level) == width &&
> +          u_minify(stObj->pt->height0, firstImage->base.Level) == height &&
> +          u_minify(stObj->pt->depth0, firstImage->base.Level) == depth) {
> +         ptWidth = stObj->pt->width0;
> +         ptHeight = stObj->pt->height0;
> +         ptDepth = stObj->pt->depth0;
>         } else {
> -         /* The width/height/depth may have been previously reset in
> -          * guess_and_alloc_texture. */
> -         stObj->width0 = width;
> -         stObj->height0 = height;
> -         stObj->depth0 = depth;
> +         /* Otherwise, compute a new level=0 size that is compatible with the
> +          * base level image.
> +          */
> +         ptWidth = width > 1 ? width << firstImage->base.Level : 1;
> +         ptHeight = height > 1 ? height << firstImage->base.Level : 1;
> +         ptDepth = depth > 1 ? depth << firstImage->base.Level : 1;
> +
> +         /* If the base level image is 1x1x1, we still need to ensure that the
> +          * resulting pipe texture ends up with the required number of levels
> +          * in total.
> +          */
> +         if (ptWidth == 1 && ptHeight == 1 && ptDepth == 1) {
> +            ptWidth <<= firstImage->base.Level;
> +
> +            if (stObj->base.Target == GL_TEXTURE_CUBE_MAP ||
> +                stObj->base.Target == GL_TEXTURE_CUBE_MAP_ARRAY)
> +               ptHeight <<= firstImage->base.Level;
> +         }
>         }
> -      /* convert GL dims to Gallium dims */
> -      st_gl_texture_dims_to_pipe_dims(stObj->base.Target, width, height, depth,
> -                                      &ptWidth, &ptHeight, &ptDepth, &ptLayers);
> +
>         ptNumSamples = firstImage->base.NumSamples;
>      }
>
> @@ -2554,16 +2564,23 @@ st_finalize_texture(struct gl_context *ctx,
>            /* Need to import images in main memory or held in other textures.
>             */
>            if (stImage && stObj->pt != stImage->pt) {
> -            GLuint height = stObj->height0;
> -            GLuint depth = stObj->depth0;
> +            GLuint height;
> +            GLuint depth;
>
>               if (stObj->base.Target != GL_TEXTURE_1D_ARRAY)
> -               height = u_minify(height, level);
> +               height = u_minify(ptHeight, level);
> +            else
> +               height = ptLayers;
> +
>               if (stObj->base.Target == GL_TEXTURE_3D)
> -               depth = u_minify(depth, level);
> +               depth = u_minify(ptDepth, level);
> +            else if (stObj->base.Target == GL_TEXTURE_CUBE_MAP)
> +               depth = 1;
> +            else
> +               depth = ptLayers;
>
>               if (level == 0 ||
> -                (stImage->base.Width == u_minify(stObj->width0, level) &&
> +                (stImage->base.Width == u_minify(ptWidth, level) &&
>                    stImage->base.Height == height &&
>                    stImage->base.Depth == depth)) {
>                  /* src image fits expected dest mipmap level size */
> @@ -2599,10 +2616,6 @@ st_AllocTextureStorage(struct gl_context *ctx,
>
>      assert(levels > 0);
>
> -   /* Save the level=0 dimensions */
> -   stObj->width0 = width;
> -   stObj->height0 = height;
> -   stObj->depth0 = depth;
>      stObj->lastLevel = levels - 1;
>
>      fmt = st_mesa_format_to_pipe_format(st, texImage->TexFormat);
> @@ -2740,9 +2753,6 @@ st_TextureView(struct gl_context *ctx,
>      tex->surface_format =
>         st_mesa_format_to_pipe_format(st_context(ctx), image->TexFormat);
>
> -   tex->width0 = image->Width;
> -   tex->height0 = image->Height;
> -   tex->depth0 = image->Depth;
>      tex->lastLevel = numLevels - 1;
>
>      return GL_TRUE;
> diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
> index a983d64..33f56ea 100644
> --- a/src/mesa/state_tracker/st_manager.c
> +++ b/src/mesa/state_tracker/st_manager.c
> @@ -587,9 +587,6 @@ st_context_teximage(struct st_context_iface *stctxi,
>      }
>
>      pipe_resource_reference(&stImage->pt, tex);
> -   stObj->width0 = width;
> -   stObj->height0 = height;
> -   stObj->depth0 = depth;
>      stObj->surface_format = pipe_format;
>
>      _mesa_dirty_texobj(ctx, texObj);
> diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
> index d8cd7c7..ae9e2b4 100644
> --- a/src/mesa/state_tracker/st_texture.h
> +++ b/src/mesa/state_tracker/st_texture.h
> @@ -79,12 +79,6 @@ struct st_texture_object
>       */
>      GLuint lastLevel;
>
> -   /** The size of the level=0 mipmap image.
> -    * Note that the number of 1D array layers will be in height0 and the
> -    * number of 2D array layers will be in depth0, as in GL.
> -    */
> -   GLuint width0, height0, depth0;
> -
>      /* On validation any active images held in main memory or in other
>       * textures will be copied to this texture and the old storage freed.
>       */
> diff --git a/src/mesa/state_tracker/st_vdpau.c b/src/mesa/state_tracker/st_vdpau.c
> index 08f2553..dffa52f 100644
> --- a/src/mesa/state_tracker/st_vdpau.c
> +++ b/src/mesa/state_tracker/st_vdpau.c
> @@ -238,9 +238,6 @@ st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum access,
>      sampler_view = st_texture_get_sampler_view(st, stObj);
>      *sampler_view = st->pipe->create_sampler_view(st->pipe, res, &templ);
>
> -   stObj->width0 = res->width0;
> -   stObj->height0 = res->height0;
> -   stObj->depth0 = 1;
>      stObj->surface_format = res->format;
>
>      _mesa_dirty_texobj(ctx, texObj);
>


More information about the mesa-dev mailing list