[Mesa-dev] [PATCH] mesa: validate sampler uniforms during gluniform calls
Tapani
tapani.palli at intel.com
Tue Oct 14 07:06:35 PDT 2014
On 10/14/2014 03:35 PM, Francisco Jerez wrote:
> Tapani Pälli <tapani.palli at intel.com> writes:
>
>> Patch fixes 'glsl-2types-of-textures-on-same-unit' in WebGL conformance
>> test suite, no Piglit regressions.
>>
>> To avoid adding potentially heavy check during draw (valid_to_render),
>> check is done during uniform updates by inspecting TexturesUsed mask.
>>
>> A new boolean variable is introduced to cache validation state.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>> src/mesa/main/context.c | 11 +++++++++++
>> src/mesa/main/mtypes.h | 1 +
>> src/mesa/main/uniform_query.cpp | 44 +++++++++--------------------------------
>> src/mesa/main/uniforms.c | 14 +++++++++++++
>> 4 files changed, 35 insertions(+), 35 deletions(-)
>>
>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>> index 5a8f718..8848338 100644
>> --- a/src/mesa/main/context.c
>> +++ b/src/mesa/main/context.c
>> @@ -133,6 +133,7 @@
>> #include "program/prog_print.h"
>> #include "math/m_matrix.h"
>> #include "main/dispatch.h" /* for _gloffset_COUNT */
>> +#include "uniforms.h"
>>
>> #ifdef USE_SPARC_ASM
>> #include "sparc/sparc.h"
>> @@ -1949,6 +1950,16 @@ _mesa_valid_to_render(struct gl_context *ctx, const char *where)
>> }
>> }
>>
>> + /* If a program is active, check if validation of samplers succeeded. */
>> + if (ctx->_Shader->ActiveProgram) {
>> + char errMsg[100];
>> + if (!_mesa_sampler_uniforms_are_valid(ctx->_Shader->ActiveProgram,
>> + errMsg, 100)) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s", errMsg);
>> + return GL_FALSE;
>> + }
>> + }
>> +
>> if (ctx->DrawBuffer->_Status != GL_FRAMEBUFFER_COMPLETE_EXT) {
>> _mesa_error(ctx, GL_INVALID_FRAMEBUFFER_OPERATION_EXT,
>> "%s(incomplete framebuffer)", where);
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 5e9453b..27670bd 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -2873,6 +2873,7 @@ struct gl_shader_program
>> GLboolean LinkStatus; /**< GL_LINK_STATUS */
>> GLboolean Validated;
>> GLboolean _Used; /**< Ever used for drawing? */
>> + GLboolean SamplersValidated; /**< Samplers validated against texture units? */
>> GLchar *InfoLog;
>>
>> unsigned Version; /**< GLSL version used for linking */
>> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
>> index 1592c9b..c2776c0 100644
>> --- a/src/mesa/main/uniform_query.cpp
>> +++ b/src/mesa/main/uniform_query.cpp
>> @@ -1064,42 +1064,16 @@ extern "C" bool
>> _mesa_sampler_uniforms_are_valid(const struct gl_shader_program *shProg,
>> char *errMsg, size_t errMsgLength)
>> {
>> - const glsl_type *unit_types[MAX_COMBINED_TEXTURE_IMAGE_UNITS];
>> -
>> - memset(unit_types, 0, sizeof(unit_types));
>> -
>> - for (unsigned i = 0; i < shProg->NumUserUniformStorage; i++) {
>> - const struct gl_uniform_storage *const storage =
>> - &shProg->UniformStorage[i];
>> - const glsl_type *const t = (storage->type->is_array())
>> - ? storage->type->fields.array : storage->type;
>> -
>> - if (!t->is_sampler())
>> - continue;
>> -
>> - const unsigned count = MAX2(1, storage->type->array_size());
>> - for (unsigned j = 0; j < count; j++) {
>> - const unsigned unit = storage->storage[j].i;
>> -
>> - /* The types of the samplers associated with a particular texture
>> - * unit must be an exact match. Page 74 (page 89 of the PDF) of the
>> - * OpenGL 3.3 core spec says:
>> - *
>> - * "It is not allowed to have variables of different sampler
>> - * types pointing to the same texture image unit within a program
>> - * object."
>> - */
>> - if (unit_types[unit] == NULL) {
>> - unit_types[unit] = t;
>> - } else if (unit_types[unit] != t) {
>> - _mesa_snprintf(errMsg, errMsgLength,
>> - "Texture unit %d is accessed both as %s and %s",
>> - unit, unit_types[unit]->name, t->name);
>> - return false;
>> - }
>> - }
>> + /* Shader does not have samplers. */
>> + if (shProg->NumUserUniformStorage == 0)
>> + return true;
>> +
>> + if (!shProg->SamplersValidated) {
>> + _mesa_snprintf(errMsg, errMsgLength,
>> + "active samplers with a different type "
>> + "refer to the same texture image unit");
>> + return false;
>> }
>> -
>> return true;
>> }
>>
>> diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
>> index 0d0cbf5..af7a822 100644
>> --- a/src/mesa/main/uniforms.c
>> +++ b/src/mesa/main/uniforms.c
>> @@ -75,12 +75,26 @@ _mesa_update_shader_textures_used(struct gl_shader_program *shProg,
>> memcpy(prog->SamplerUnits, shader->SamplerUnits, sizeof(prog->SamplerUnits));
>> memset(prog->TexturesUsed, 0, sizeof(prog->TexturesUsed));
>>
>> + shProg->SamplersValidated = GL_TRUE;
>> +
>> for (s = 0; s < MAX_SAMPLERS; s++) {
>> if (prog->SamplersUsed & (1 << s)) {
>> GLuint unit = shader->SamplerUnits[s];
>> GLuint tgt = shader->SamplerTargets[s];
>> assert(unit < Elements(prog->TexturesUsed));
>> assert(tgt < NUM_TEXTURE_TARGETS);
>> +
>> + /* The types of the samplers associated with a particular texture
>> + * unit must be an exact match. Page 74 (page 89 of the PDF) of the
>> + * OpenGL 3.3 core spec says:
>> + *
>> + * "It is not allowed to have variables of different sampler
>> + * types pointing to the same texture image unit within a program
>> + * object."
>> + */
>> + if (prog->TexturesUsed[unit] != 0)
> Hi Tapani, isn't this going to give a false positive when there are
> several sampler uniforms of the same type bound to the same unit? Maybe
> "TexturesUsed[unit] & ~(1 << tgt)" is what you meant?
Ah right, this is true.
> And most likely we want to cross-validate sampler uniform types from all
> shader stages bound to the program pipeline, as we discussed earlier.
Yep, I thought existing tests might cover this but will check. If not
I'll make a new test for this and also for the situation above and fix
things.
>> + shProg->SamplersValidated = GL_FALSE;
>> +
>> prog->TexturesUsed[unit] |= (1 << tgt);
>> }
>> }
>> --
>> 1.9.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list