1) The tabs are used in the entire file. They might have got there while I was moving some of the bits of code around. I don't really use tabs for core Mesa work. I can change the tabs to spaces unless you mean I should change everything to tabs (you didn't make that clear).
2) _mesa_get_sampler_uniform_value is really called at link time, specifically at the conversion to TGSI. The only case where it might be called after link time is when a new shader variant is being generated, though i'm not really sure about this. Anyway, the function already calls linker_error above my code to report an error. I decided to do the same. _mesa_problem sounds better though. Marek On Tue, May 21, 2013 at 11:54 PM, Eric Anholt <[email protected]> wrote: > Marek Olšák <[email protected]> writes: > >> The problem is the sampler units are allocated from the same pool for all >> shader stages, so if a vertex shader uses 12 samplers (0..11), the fragment >> shader samplers start at index 12, leaving only 4 sampler units >> for the fragment shader. The main cause is probably the fact that samplers >> (texture unit -> sampler unit mapping, etc.) are tracked globally >> for an entire program object. >> >> This commit adapts the GLSL linker and core Mesa such that the sampler units >> are assigned to sampler uniforms for each shader stage separately >> (if a sampler uniform is used in all shader stages, it may occupy a different >> sampler unit in each, and vice versa, an i-th sampler unit may refer to >> a different sampler uniform in each shader stage), and the sampler-specific >> variables are moved from gl_shader_program to gl_shader. >> >> This doesn't require any driver changes, and it fixes piglit/max-samplers >> for gallium and classic swrast. It also works with any number of shader >> stages. > > I was initially uncomfortable with gl_uniform_storage getting bigger, > but it's *huge* already, and the increased overhead I thought I saw in > uniform updates isn't really there. So, just nitpicks: > >> @@ -335,8 +339,38 @@ public: >> int ubo_block_index; >> int ubo_byte_offset; >> bool ubo_row_major; >> + gl_shader_type shader_type; >> >> private: >> + void handle_samplers(const glsl_type *base_type, >> + struct gl_uniform_storage *uniform) >> + { >> + >> + if (base_type->is_sampler()) { >> + uniform->sampler[shader_type].index = this->next_sampler; >> + uniform->sampler[shader_type].active = true; >> + >> + /* Increment the sampler by 1 for non-arrays and by the number of >> + * array elements for arrays. >> + */ > > bad tab indentation > >> + this->next_sampler += >> + MAX2(1, uniform->array_elements); >> + >> + const gl_texture_index target = base_type->sampler_index(); >> + const unsigned shadow = base_type->sampler_shadow; > > tabs > >> + for (unsigned i = uniform->sampler[shader_type].index; >> + i < MIN2(this->next_sampler, MAX_SAMPLERS); >> + i++) { >> + this->targets[i] = target; >> + this->shader_samplers_used |= 1U << i; >> + this->shader_shadow_samplers |= shadow << i; >> + } > > tabs > >> @@ -388,26 +397,16 @@ private: >> base_type = type; >> } >> >> - if (base_type->is_sampler()) { >> - this->uniforms[id].sampler = this->next_sampler; >> + /* This assigns sampler uniforms to sampler units. */ >> + handle_samplers(base_type, &this->uniforms[id]); >> >> - /* Increment the sampler by 1 for non-arrays and by the number of >> - * array elements for arrays. >> - */ >> - this->next_sampler += MAX2(1, this->uniforms[id].array_elements); >> - >> - const gl_texture_index target = base_type->sampler_index(); >> - const unsigned shadow = base_type->sampler_shadow; >> - for (unsigned i = this->uniforms[id].sampler >> - ; i < MIN2(this->next_sampler, MAX_SAMPLERS) >> - ; i++) { >> - this->targets[i] = target; >> - this->shader_samplers_used |= 1U << i; >> - this->shader_shadow_samplers |= shadow << i; >> - } >> - >> - } else { >> - this->uniforms[id].sampler = ~0; >> + /* If there is already storage associated with this uniform, it means >> + * that it was set while processing an earlier shader stage. For >> + * example, we may be processing the uniform in the fragment shader, >> but >> + * the uniform was already processed in the vertex shader. >> + */ >> + if (this->uniforms[id].storage != NULL) { >> + return; > > tabs > >> @@ -119,6 +122,14 @@ _mesa_get_sampler_uniform_value(class ir_dereference >> *sampler, >> return 0; >> } >> >> - return shader_program->UniformStorage[location].sampler + getname.offset; >> -} >> + if (!shader_program->UniformStorage[location].sampler[shader].active) { >> + linker_error(shader_program, >> + "cannot return a sampler named %s, because it is not " >> + "used in this shader stage. This is a driver bug.\n", >> + getname.name); >> + return 0; >> + } > > Maybe _mesa_problem()? This isn't really called at link time, and you > want something really visible like an assert. > > Other than the style bits, > > Reviewed-by: Eric Anholt <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
