From: Paul Berry Date: Fri, 22 Feb 2013 23:37:41 +0000 (-0800) Subject: ff_fragment_shader: Don't do unnecessary (and dangerous) uniform setup. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=db81d3b8f78d1f5a70ce909981807825d11fc6f3;p=mesa.git ff_fragment_shader: Don't do unnecessary (and dangerous) uniform setup. 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 --- diff --git a/src/mesa/main/ff_fragment_shader.cpp b/src/mesa/main/ff_fragment_shader.cpp index 186988bbd70..01a4542d747 100644 --- a/src/mesa/main/ff_fragment_shader.cpp +++ b/src/mesa/main/ff_fragment_shader.cpp @@ -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);