ff_fragment_shader: Don't do unnecessary (and dangerous) uniform setup.
authorPaul Berry <stereotype441@gmail.com>
Fri, 22 Feb 2013 23:37:41 +0000 (15:37 -0800)
committerPaul Berry <stereotype441@gmail.com>
Tue, 19 Mar 2013 23:56:56 +0000 (16:56 -0700)
Previously, right after calling _mesa_glsl_link_shader(), the fixed
function fragment shader code made several calls with the ostensible
purpose of setting up uniforms for the fragment shader it just
created.

These calls are unnecessary, since _mesa_glsl_link_shader() calls
driver->LinkShader(), which takes care of calling these functions (or
their equivalent).  Also, they are dangerous to call after
_mesa_glsl_link_shader() has returned, because on back-ends such as
i965 which do precompilation, _mesa_glsl_link_shader() may have
already cached pointers to the existing uniform structures; attempting
to set up the uniforms again invalidates those cached pointers.

It was only by sheer coincidence that this wasn't manifesting itself
as a bug.  It turns out that i965's precompile mechanism was always
setting bit 0 of brw_wm_prog_key::proj_attrib_mask to 0 for fixed
function fragment shaders, but during normal usage this bit usually
gets set to 1.  As a result, the precompiled shader (with its invalid
uniform pointers) was not being used.

I'm about to introduce some changes that cause bit 0 of
proj_attrib_mask to be set consistently between precompilation and
normal usage, so to avoid regressions I need to get rid of the
dangerous duplicate uniform setup code first.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
src/mesa/main/ff_fragment_shader.cpp

index 186988bbd7016ddc0a194b189ffde882553239b5..01a4542d747c8478de9bdb91f1bb751c70ab5d22 100644 (file)
@@ -1350,22 +1350,6 @@ create_new_program(struct gl_context *ctx, struct state_key *key)
 
    _mesa_glsl_link_shader(ctx, p.shader_program);
 
-   /* Set the sampler uniforms, and relink to get them into the linked
-    * program.
-    */
-   struct gl_shader *const fs =
-      p.shader_program->_LinkedShaders[MESA_SHADER_FRAGMENT];
-   struct gl_program *const fp = fs->Program;
-
-   _mesa_generate_parameters_list_for_uniforms(p.shader_program, fs,
-                                              fp->Parameters);
-
-   _mesa_associate_uniform_storage(ctx, p.shader_program, fp->Parameters);
-
-   _mesa_update_shader_textures_used(p.shader_program, fp);
-   if (ctx->Driver.SamplerUniformChange)
-      ctx->Driver.SamplerUniformChange(ctx, fp->Target, fp);
-
    if (!p.shader_program->LinkStatus)
       _mesa_problem(ctx, "Failed to link fixed function fragment shader: %s\n",
                    p.shader_program->InfoLog);