[Mesa-dev] [PATCH 2/2] st/mesa: remove st_finalize_textures atom

[Brian Paul]

On 07/28/2015 04:05 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> It only checks fragment textures and ignores other shaders, which makes it
> incomplete, and texture are already finalized in update_single_texture.
>
> There are no piglit regressions.
> ---
>   src/mesa/state_tracker/st_atom.c         |  1 -
>   src/mesa/state_tracker/st_atom.h         |  1 -
>   src/mesa/state_tracker/st_atom_texture.c | 45 --------------------------------
>   3 files changed, 47 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom.c b/src/mesa/state_tracker/st_atom.c
> index 5fc1a77..43dbadd 100644
> --- a/src/mesa/state_tracker/st_atom.c
> +++ b/src/mesa/state_tracker/st_atom.c
> @@ -46,7 +46,6 @@ static const struct st_tracked_state *atoms[] =
>      &st_update_depth_stencil_alpha,
>      &st_update_clip,
>
> -   &st_finalize_textures,
>      &st_update_fp,
>      &st_update_gp,
>      &st_update_tep,
> diff --git a/src/mesa/state_tracker/st_atom.h b/src/mesa/state_tracker/st_atom.h
> index 5735ca6..a24842b 100644
> --- a/src/mesa/state_tracker/st_atom.h
> +++ b/src/mesa/state_tracker/st_atom.h
> @@ -68,7 +68,6 @@ extern const struct st_tracked_state st_update_vertex_texture;
>   extern const struct st_tracked_state st_update_geometry_texture;
>   extern const struct st_tracked_state st_update_tessctrl_texture;
>   extern const struct st_tracked_state st_update_tesseval_texture;
> -extern const struct st_tracked_state st_finalize_textures;
>   extern const struct st_tracked_state st_update_fs_constants;
>   extern const struct st_tracked_state st_update_gs_constants;
>   extern const struct st_tracked_state st_update_tes_constants;
> diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c
> index 4422d9a..31e0f6b 100644
> --- a/src/mesa/state_tracker/st_atom_texture.c
> +++ b/src/mesa/state_tracker/st_atom_texture.c
> @@ -582,48 +582,3 @@ const struct st_tracked_state st_update_tesseval_texture = {
>      },
>      update_tesseval_textures				/* update */
>   };
> -
> -
> -
> -static void
> -finalize_textures(struct st_context *st)
> -{
> -   struct gl_context *ctx = st->ctx;
> -   struct gl_fragment_program *fprog = ctx->FragmentProgram._Current;
> -   const GLboolean prev_missing_textures = st->missing_textures;
> -   GLuint su;
> -
> -   st->missing_textures = GL_FALSE;
> -
> -   for (su = 0; su < ctx->Const.MaxTextureCoordUnits; su++) {
> -      if (fprog->Base.SamplersUsed & (1 << su)) {
> -         const GLuint texUnit = fprog->Base.SamplerUnits[su];
> -         struct gl_texture_object *texObj
> -            = ctx->Texture.Unit[texUnit]._Current;
> -
> -         if (texObj) {
> -            GLboolean retval;
> -
> -            retval = st_finalize_texture(ctx, st->pipe, texObj);
> -            if (!retval) {
> -               /* out of mem */
> -               st->missing_textures = GL_TRUE;
> -               continue;
> -            }
> -         }
> -      }
> -   }
> -
> -   if (prev_missing_textures != st->missing_textures)
> -      st->dirty.st |= ST_NEW_FRAGMENT_PROGRAM;
> -}
> -
> -
> -const struct st_tracked_state st_finalize_textures = {
> -   "st_finalize_textures",		/* name */
> -   {					/* dirty */
> -      _NEW_TEXTURE,			/* mesa */
> -      0,				/* st */
> -   },
> -   finalize_textures			/* update */
> -};
>

It looks like st_context::missing_textures could be removed too since 
it's no longer set anywhere.  And that would lead to removing 
get_passthrough_fs().

I remember adding the missing_textures code years ago for some 
application which failed to allocate a texture at some point and 
crashed.  It's practically impossible to check for this situation with 
piglit.  I guess one would have to hack a driver to force a 
resource_create() call to fail at some point.

After you check in this patch I could try something like that with a 
Mesa demo to see what happens.

In any case, this is a nice clean-up.

For both,
Reviewed-by: Brian Paul <brianp at vmware.com>