Re: [Mesa-dev] [PATCH 2/2] st/mesa: simplify st_generate_mipmap()

[Roland Scheidegger]

Am 26.03.2016 um 18:21 schrieb Brian Paul:
> The whole st_generate_mipmap() function was overly complicated.  Now
> we just call the new _mesa_prepare_mipmap_levels() function to prepare
> the texture mipmap memory, then call the generate function which fills
> in the texture images.
> 
> This fixes a failed assertion in llvmpipe/softpipe which is hit with the
> new piglit generatemipmap-base-change test.  Also fixes some device errors
> (format mismatches) with the VMware svga driver.
> ---
>  src/mesa/state_tracker/st_gen_mipmap.c | 102 
> -
>  1 file changed, 24 insertions(+), 78 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_gen_mipmap.c 
> b/src/mesa/state_tracker/st_gen_mipmap.c
> index c4b3492..d854e20 100644
> --- a/src/mesa/state_tracker/st_gen_mipmap.c
> +++ b/src/mesa/state_tracker/st_gen_mipmap.c
> @@ -82,7 +82,6 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
> const uint baseLevel = texObj->BaseLevel;
> enum pipe_format format;
> uint lastLevel, first_layer, last_layer;
> -   uint dstLevel;
>  
> if (!pt)
>return;
> @@ -103,42 +102,33 @@ st_generate_mipmap(struct gl_context *ctx, GLenum 
> target,
> stObj->lastLevel = lastLevel;
>  
> if (!texObj->Immutable) {
> -  if (pt->last_level < lastLevel) {
> - /* The current gallium texture doesn't have space for all the
> - * mipmap levels we need to generate.  So allocate a new texture.
> - */
> - struct pipe_resource *oldTex = stObj->pt;
> -
> - /* create new texture with space for more levels */
> - stObj->pt = st_texture_create(st,
> -   oldTex->target,
> -   oldTex->format,
> -   lastLevel,
> -   oldTex->width0,
> -   oldTex->height0,
> -   oldTex->depth0,
> -   oldTex->array_size,
> -   0,
> -   oldTex->bind);
> -
> - /* This will copy the old texture's base image into the new texture
> - * which we just allocated.
> - */
> - st_finalize_texture(ctx, st->pipe, texObj);
> -
> - /* release the old tex (will likely be freed too) */
> - pipe_resource_reference(, NULL);
> - st_texture_release_all_sampler_views(st, stObj);
> -  }
> -  else {
> - /* Make sure that the base texture image data is present in the
> - * texture buffer.
> - */
> - st_finalize_texture(ctx, st->pipe, texObj);
> -  }
> +  const GLboolean genSave = texObj->GenerateMipmap;
> +
> +  /* Temporarily set GenerateMipmap to true so that 
> allocate_full_mipmap()
> +   * makes the right decision about full mipmap allocation.
> +   */
> +  texObj->GenerateMipmap = GL_TRUE;
> +
> +  _mesa_prepare_mipmap_levels(ctx, texObj, baseLevel, lastLevel);
> +
> +  texObj->GenerateMipmap = genSave;
> +
> +  /* At this point, memory for all the texture levels has been
> +   * allocated.  However, the base level image may be in one resource
> +   * while the subsequent/smaller levels may be in another resource.
> +   * Finalizing the texture will copy the base images from the former
> +   * resource to the later.
> +   *
> +   * After this, we'll have all mipmap levels in one resource.
> +   */
> +  st_finalize_texture(ctx, st->pipe, texObj);
> }
>  
> pt = stObj->pt;
> +   if (!pt) {
> +  _mesa_error(ctx, GL_OUT_OF_MEMORY, "mipmap generation");
> +  return;
> +   }
>  
> assert(pt->last_level >= lastLevel);
>  
> @@ -169,48 +159,4 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
>   _mesa_generate_mipmap(ctx, target, texObj);
>}
> }
> -
> -   /* Fill in the Mesa gl_texture_image fields */
> -   for (dstLevel = baseLevel + 1; dstLevel <= lastLevel; dstLevel++) {
> -  const uint srcLevel = dstLevel - 1;
> -  const struct gl_texture_image *srcImage
> - = _mesa_get_tex_image(ctx, texObj, target, srcLevel);
> -  struct gl_texture_image *dstImage;
> -  struct st_texture_image *stImage;
> -  uint border = srcImage->Border;
> -  uint dstWidth, dstHeight, dstDepth;
> -
> -  dstWidth = u_minify(pt->width0, dstLevel);
> -  if (texObj->Target == GL_TEXTURE_1D_ARRAY) {
> - dstHeight = pt->array_size;
> -  }
> -  else {
> - dstHeight = u_minify(pt->height0, dstLevel);
> -  }
> -  if (texObj->Target == GL_TEXTURE_2D_ARRAY ||
> -  texObj->Target == GL_TEXTURE_CUBE_MAP_ARRAY) {
> - dstDepth = pt->array_size;
> -  }
> -  else {
> - dstDepth = u_minify(pt->depth0, dstLevel);
> -  }
> -
> -  dstImage = _mesa_get_tex_image(ctx, texObj, 

Re: [Mesa-dev] [PATCH 2/2] st/mesa: simplify st_generate_mipmap()

[Jose Fonseca]

On 26/03/16 17:21, Brian Paul wrote:

The whole st_generate_mipmap() function was overly complicated.  Now
we just call the new _mesa_prepare_mipmap_levels() function to prepare
the texture mipmap memory, then call the generate function which fills
in the texture images.

This fixes a failed assertion in llvmpipe/softpipe which is hit with the
new piglit generatemipmap-base-change test.  Also fixes some device errors
(format mismatches) with the VMware svga driver.
---
  src/mesa/state_tracker/st_gen_mipmap.c | 102 -
  1 file changed, 24 insertions(+), 78 deletions(-)

diff --git a/src/mesa/state_tracker/st_gen_mipmap.c 
b/src/mesa/state_tracker/st_gen_mipmap.c
index c4b3492..d854e20 100644
--- a/src/mesa/state_tracker/st_gen_mipmap.c
+++ b/src/mesa/state_tracker/st_gen_mipmap.c
@@ -82,7 +82,6 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
 const uint baseLevel = texObj->BaseLevel;
 enum pipe_format format;
 uint lastLevel, first_layer, last_layer;
-   uint dstLevel;

 if (!pt)
return;
@@ -103,42 +102,33 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
 stObj->lastLevel = lastLevel;

 if (!texObj->Immutable) {
-  if (pt->last_level < lastLevel) {
- /* The current gallium texture doesn't have space for all the
- * mipmap levels we need to generate.  So allocate a new texture.
- */
- struct pipe_resource *oldTex = stObj->pt;
-
- /* create new texture with space for more levels */
- stObj->pt = st_texture_create(st,
-   oldTex->target,
-   oldTex->format,
-   lastLevel,
-   oldTex->width0,
-   oldTex->height0,
-   oldTex->depth0,
-   oldTex->array_size,
-   0,
-   oldTex->bind);
-
- /* This will copy the old texture's base image into the new texture
- * which we just allocated.
- */
- st_finalize_texture(ctx, st->pipe, texObj);
-
- /* release the old tex (will likely be freed too) */
- pipe_resource_reference(, NULL);
- st_texture_release_all_sampler_views(st, stObj);
-  }
-  else {
- /* Make sure that the base texture image data is present in the
- * texture buffer.
- */
- st_finalize_texture(ctx, st->pipe, texObj);
-  }
+  const GLboolean genSave = texObj->GenerateMipmap;
+
+  /* Temporarily set GenerateMipmap to true so that allocate_full_mipmap()
+   * makes the right decision about full mipmap allocation.
+   */
+  texObj->GenerateMipmap = GL_TRUE;


I wonder if there's any assertion we could add somewhere to ensure it 
did indeed make the right decision.


Either way, series is:

Reviewed-by: Jose Fonseca 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] st/mesa: simplify st_generate_mipmap()

[Sinclair Yeh]

One typo below otherwise:  Reviewed-by: Sinclair Yeh 

On Sat, Mar 26, 2016 at 11:21:19AM -0600, Brian Paul wrote:
> The whole st_generate_mipmap() function was overly complicated.  Now
> we just call the new _mesa_prepare_mipmap_levels() function to prepare
> the texture mipmap memory, then call the generate function which fills
> in the texture images.
> 
> This fixes a failed assertion in llvmpipe/softpipe which is hit with the
> new piglit generatemipmap-base-change test.  Also fixes some device errors
> (format mismatches) with the VMware svga driver.
> ---
>  src/mesa/state_tracker/st_gen_mipmap.c | 102 
> -
>  1 file changed, 24 insertions(+), 78 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_gen_mipmap.c 
> b/src/mesa/state_tracker/st_gen_mipmap.c
> index c4b3492..d854e20 100644
> --- a/src/mesa/state_tracker/st_gen_mipmap.c
> +++ b/src/mesa/state_tracker/st_gen_mipmap.c
> @@ -82,7 +82,6 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
> const uint baseLevel = texObj->BaseLevel;
> enum pipe_format format;
> uint lastLevel, first_layer, last_layer;
> -   uint dstLevel;
>  
> if (!pt)
>return;
> @@ -103,42 +102,33 @@ st_generate_mipmap(struct gl_context *ctx, GLenum 
> target,
> stObj->lastLevel = lastLevel;
>  
> if (!texObj->Immutable) {
> -  if (pt->last_level < lastLevel) {
> - /* The current gallium texture doesn't have space for all the
> - * mipmap levels we need to generate.  So allocate a new texture.
> - */
> - struct pipe_resource *oldTex = stObj->pt;
> -
> - /* create new texture with space for more levels */
> - stObj->pt = st_texture_create(st,
> -   oldTex->target,
> -   oldTex->format,
> -   lastLevel,
> -   oldTex->width0,
> -   oldTex->height0,
> -   oldTex->depth0,
> -   oldTex->array_size,
> -   0,
> -   oldTex->bind);
> -
> - /* This will copy the old texture's base image into the new texture
> - * which we just allocated.
> - */
> - st_finalize_texture(ctx, st->pipe, texObj);
> -
> - /* release the old tex (will likely be freed too) */
> - pipe_resource_reference(, NULL);
> - st_texture_release_all_sampler_views(st, stObj);
> -  }
> -  else {
> - /* Make sure that the base texture image data is present in the
> - * texture buffer.
> - */
> - st_finalize_texture(ctx, st->pipe, texObj);
> -  }
> +  const GLboolean genSave = texObj->GenerateMipmap;
> +
> +  /* Temporarily set GenerateMipmap to true so that 
> allocate_full_mipmap()
> +   * makes the right decision about full mipmap allocation.
> +   */
> +  texObj->GenerateMipmap = GL_TRUE;
> +
> +  _mesa_prepare_mipmap_levels(ctx, texObj, baseLevel, lastLevel);
> +
> +  texObj->GenerateMipmap = genSave;
> +
> +  /* At this point, memory for all the texture levels has been
> +   * allocated.  However, the base level image may be in one resource
> +   * while the subsequent/smaller levels may be in another resource.
> +   * Finalizing the texture will copy the base images from the former
> +   * resource to the later.
latter.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] st/mesa: simplify st_generate_mipmap()

[Brian Paul]

The whole st_generate_mipmap() function was overly complicated.  Now
we just call the new _mesa_prepare_mipmap_levels() function to prepare
the texture mipmap memory, then call the generate function which fills
in the texture images.

This fixes a failed assertion in llvmpipe/softpipe which is hit with the
new piglit generatemipmap-base-change test.  Also fixes some device errors
(format mismatches) with the VMware svga driver.
---
 src/mesa/state_tracker/st_gen_mipmap.c | 102 -
 1 file changed, 24 insertions(+), 78 deletions(-)

diff --git a/src/mesa/state_tracker/st_gen_mipmap.c 
b/src/mesa/state_tracker/st_gen_mipmap.c
index c4b3492..d854e20 100644
--- a/src/mesa/state_tracker/st_gen_mipmap.c
+++ b/src/mesa/state_tracker/st_gen_mipmap.c
@@ -82,7 +82,6 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
const uint baseLevel = texObj->BaseLevel;
enum pipe_format format;
uint lastLevel, first_layer, last_layer;
-   uint dstLevel;
 
if (!pt)
   return;
@@ -103,42 +102,33 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
stObj->lastLevel = lastLevel;
 
if (!texObj->Immutable) {
-  if (pt->last_level < lastLevel) {
- /* The current gallium texture doesn't have space for all the
- * mipmap levels we need to generate.  So allocate a new texture.
- */
- struct pipe_resource *oldTex = stObj->pt;
-
- /* create new texture with space for more levels */
- stObj->pt = st_texture_create(st,
-   oldTex->target,
-   oldTex->format,
-   lastLevel,
-   oldTex->width0,
-   oldTex->height0,
-   oldTex->depth0,
-   oldTex->array_size,
-   0,
-   oldTex->bind);
-
- /* This will copy the old texture's base image into the new texture
- * which we just allocated.
- */
- st_finalize_texture(ctx, st->pipe, texObj);
-
- /* release the old tex (will likely be freed too) */
- pipe_resource_reference(, NULL);
- st_texture_release_all_sampler_views(st, stObj);
-  }
-  else {
- /* Make sure that the base texture image data is present in the
- * texture buffer.
- */
- st_finalize_texture(ctx, st->pipe, texObj);
-  }
+  const GLboolean genSave = texObj->GenerateMipmap;
+
+  /* Temporarily set GenerateMipmap to true so that allocate_full_mipmap()
+   * makes the right decision about full mipmap allocation.
+   */
+  texObj->GenerateMipmap = GL_TRUE;
+
+  _mesa_prepare_mipmap_levels(ctx, texObj, baseLevel, lastLevel);
+
+  texObj->GenerateMipmap = genSave;
+
+  /* At this point, memory for all the texture levels has been
+   * allocated.  However, the base level image may be in one resource
+   * while the subsequent/smaller levels may be in another resource.
+   * Finalizing the texture will copy the base images from the former
+   * resource to the later.
+   *
+   * After this, we'll have all mipmap levels in one resource.
+   */
+  st_finalize_texture(ctx, st->pipe, texObj);
}
 
pt = stObj->pt;
+   if (!pt) {
+  _mesa_error(ctx, GL_OUT_OF_MEMORY, "mipmap generation");
+  return;
+   }
 
assert(pt->last_level >= lastLevel);
 
@@ -169,48 +159,4 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
  _mesa_generate_mipmap(ctx, target, texObj);
   }
}
-
-   /* Fill in the Mesa gl_texture_image fields */
-   for (dstLevel = baseLevel + 1; dstLevel <= lastLevel; dstLevel++) {
-  const uint srcLevel = dstLevel - 1;
-  const struct gl_texture_image *srcImage
- = _mesa_get_tex_image(ctx, texObj, target, srcLevel);
-  struct gl_texture_image *dstImage;
-  struct st_texture_image *stImage;
-  uint border = srcImage->Border;
-  uint dstWidth, dstHeight, dstDepth;
-
-  dstWidth = u_minify(pt->width0, dstLevel);
-  if (texObj->Target == GL_TEXTURE_1D_ARRAY) {
- dstHeight = pt->array_size;
-  }
-  else {
- dstHeight = u_minify(pt->height0, dstLevel);
-  }
-  if (texObj->Target == GL_TEXTURE_2D_ARRAY ||
-  texObj->Target == GL_TEXTURE_CUBE_MAP_ARRAY) {
- dstDepth = pt->array_size;
-  }
-  else {
- dstDepth = u_minify(pt->depth0, dstLevel);
-  }
-
-  dstImage = _mesa_get_tex_image(ctx, texObj, target, dstLevel);
-  if (!dstImage) {
- _mesa_error(ctx, GL_OUT_OF_MEMORY, "generating mipmaps");
- return;
-  }
-
-  /* Free old image data */
-  ctx->Driver.FreeTextureImageBuffer(ctx, dstImage);
-
-  /* initialize new image */
-