Tapani Pälli <tapani.pa...@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.pa...@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? And most likely we want to cross-validate sampler uniform types from all shader stages bound to the program pipeline, as we discussed earlier. > + shProg->SamplersValidated = GL_FALSE; > + > prog->TexturesUsed[unit] |= (1 << tgt); > } > } > -- > 1.9.3 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
pgpYTUkiXI3T7.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
