[Mesa-dev] [PATCH 04/20] mesa: decrease the array size of ctx->Texture.FixedFuncUnit to 8
Ian Romanick
idr at freedesktop.org
Wed Nov 22 21:28:50 UTC 2017
On 11/21/2017 10:01 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> This makes piglit/texunits fail, because we can't do glTexEnv and
> glGetTexEnv with 192 texture units anymore. Not that it ever made sense.
It's just the test_texture_env subtest that fails, right? Does this
test pass on any closed-source drivers? I'm wondering if the test is
just wrong. glTexEnv and glGetTexEnv are about fragment processing.
The GL spec is pretty clear that the limit advertised by
GL_MAX_TEXTURE_IMAGE_UNITS applies to fragment shader usage. It seems
logical that fixed-function fragment processing would have the same unit
(though I haven't found anything to support this theory yet).
Further, it's quite clear that GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS is
the count of total users of all texture units. Section 11.1.3.5
(Texture Access) of the OpenGL 4.5 compatibility profile spec says:
All active shaders, and fixed-function fragment processing if no
fragment shader is active, combined cannot use more than the value
of MAX_COMBINED_TEXTURE_IMAGE_UNITS texture image units. If more
than one pipeline stage accesses the same texture image unit, each
such access counts separately against the
MAX_COMBINED_TEXTURE_IMAGE_UNITS limit.
Does that subtest pass with this change if you
s/MaxTextureCombinedUnits/MaxTextureImageUnits/ on lines 282 and 291?
> If people are OK with this, we can adjust the test to check only 8 texture
> units, so that it doesn't fail. See also code comments.
>
> gl_context: 136896 -> 75072 bytes
> ---
> src/mesa/main/enable.c | 5 +++++
> src/mesa/main/mtypes.h | 2 +-
> src/mesa/main/texenv.c | 27 +++++++++++++++++++++++++++
> src/mesa/main/texstate.c | 2 +-
> src/mesa/main/texstate.h | 3 +++
> 5 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
> index 93ffb0d..54b3b65 100644
> --- a/src/mesa/main/enable.c
> +++ b/src/mesa/main/enable.c
> @@ -213,20 +213,22 @@ get_texcoord_unit(struct gl_context *ctx)
> /**
> * Helper function to enable or disable a texture target.
> * \param bit one of the TEXTURE_x_BIT values
> * \return GL_TRUE if state is changing or GL_FALSE if no change
> */
> static GLboolean
> enable_texture(struct gl_context *ctx, GLboolean state, GLbitfield texBit)
> {
> struct gl_fixedfunc_texture_unit *texUnit =
> _mesa_get_current_fixedfunc_tex_unit(ctx);
> + if (!texUnit)
> + return false;
>
> const GLbitfield newenabled = state
> ? (texUnit->Enabled | texBit) : (texUnit->Enabled & ~texBit);
>
> if (texUnit->Enabled == newenabled)
> return GL_FALSE;
>
> FLUSH_VERTICES(ctx, _NEW_TEXTURE_STATE);
> texUnit->Enabled = newenabled;
> return GL_TRUE;
> @@ -1291,20 +1293,23 @@ _mesa_IsEnabledi( GLenum cap, GLuint index )
>
> /**
> * Helper function to determine whether a texture target is enabled.
> */
> static GLboolean
> is_texture_enabled(struct gl_context *ctx, GLbitfield bit)
> {
> const struct gl_fixedfunc_texture_unit *const texUnit =
> _mesa_get_current_fixedfunc_tex_unit(ctx);
>
> + if (!texUnit)
> + return false;
> +
> return (texUnit->Enabled & bit) ? GL_TRUE : GL_FALSE;
> }
>
>
> /**
> * Return simple enable/disable state.
> *
> * \param cap state variable to query.
> *
> * Returns the state of the specified capability from the current GL context.
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 42b9721..23abc68 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1314,21 +1314,21 @@ struct gl_texture_attrib
> /** Bitwise-OR of all Texture.Unit[i]._GenFlags */
> GLbitfield _GenFlags;
>
> /** Largest index of a texture unit with _Current != NULL. */
> GLint _MaxEnabledTexImageUnit;
>
> /** Largest index + 1 of texture units that have had any CurrentTex set. */
> GLint NumCurrentTexUsed;
>
> struct gl_texture_unit Unit[MAX_COMBINED_TEXTURE_IMAGE_UNITS];
> - struct gl_fixedfunc_texture_unit FixedFuncUnit[MAX_COMBINED_TEXTURE_IMAGE_UNITS];
> + struct gl_fixedfunc_texture_unit FixedFuncUnit[MAX_TEXTURE_COORD_UNITS];
> };
>
>
> /**
> * Data structure representing a single clip plane (e.g. one of the elements
> * of the ctx->Transform.EyeUserPlane or ctx->Transform._ClipUserPlane array).
> */
> typedef GLfloat gl_clip_plane[4];
>
>
> diff --git a/src/mesa/main/texenv.c b/src/mesa/main/texenv.c
> index 9018ce9..22fc8da 100644
> --- a/src/mesa/main/texenv.c
> +++ b/src/mesa/main/texenv.c
> @@ -393,20 +393,29 @@ _mesa_TexEnvfv( GLenum target, GLenum pname, const GLfloat *param )
> ? ctx->Const.MaxTextureCoordUnits : ctx->Const.MaxCombinedTextureImageUnits;
> if (ctx->Texture.CurrentUnit >= maxUnit) {
> _mesa_error(ctx, GL_INVALID_OPERATION, "glTexEnvfv(current unit)");
> return;
> }
>
> if (target == GL_TEXTURE_ENV) {
> struct gl_fixedfunc_texture_unit *texUnit =
> _mesa_get_current_fixedfunc_tex_unit(ctx);
>
> + /* The GL spec says that we should report an error if the unit is greater
> + * than GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, but in practice, only
> + * fixed-function units are usable. This is probably a spec bug.
> + * Ignore glTexEnv(GL_TEXTURE_ENV) calls for non-fixed-func units,
> + * because we don't want to process calls that have no effect.
> + */
> + if (!texUnit)
> + return;
> +
> switch (pname) {
> case GL_TEXTURE_ENV_MODE:
> set_env_mode(ctx, texUnit, (GLenum) iparam0);
> break;
> case GL_TEXTURE_ENV_COLOR:
> set_env_color(ctx, texUnit, param);
> break;
> case GL_COMBINE_RGB:
> case GL_COMBINE_ALPHA:
> set_combiner_mode(ctx, texUnit, pname, (GLenum) iparam0);
> @@ -642,20 +651,29 @@ _mesa_GetTexEnvfv( GLenum target, GLenum pname, GLfloat *params )
> ? ctx->Const.MaxTextureCoordUnits : ctx->Const.MaxCombinedTextureImageUnits;
> if (ctx->Texture.CurrentUnit >= maxUnit) {
> _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexEnvfv(current unit)");
> return;
> }
>
> if (target == GL_TEXTURE_ENV) {
> struct gl_fixedfunc_texture_unit *texUnit =
> _mesa_get_current_fixedfunc_tex_unit(ctx);
>
> + /* The GL spec says that we should report an error if the unit is greater
> + * than GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, but in practice, only
> + * fixed-function units are usable. This is probably a spec bug.
> + * Ignore calls for non-fixed-func units, because we don't process
> + * glTexEnv for them either.
> + */
> + if (!texUnit)
> + return;
> +
> if (pname == GL_TEXTURE_ENV_COLOR) {
> if(ctx->NewState & (_NEW_BUFFERS | _NEW_FRAG_CLAMP))
> _mesa_update_state(ctx);
> if (_mesa_get_clamp_fragment_color(ctx, ctx->DrawBuffer))
> COPY_4FV( params, texUnit->EnvColor );
> else
> COPY_4FV( params, texUnit->EnvColorUnclamped );
> }
> else {
> GLint val = get_texenvi(ctx, texUnit, pname);
> @@ -710,20 +728,29 @@ _mesa_GetTexEnviv( GLenum target, GLenum pname, GLint *params )
> ? ctx->Const.MaxTextureCoordUnits : ctx->Const.MaxCombinedTextureImageUnits;
> if (ctx->Texture.CurrentUnit >= maxUnit) {
> _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexEnviv(current unit)");
> return;
> }
>
> if (target == GL_TEXTURE_ENV) {
> struct gl_fixedfunc_texture_unit *texUnit =
> _mesa_get_current_fixedfunc_tex_unit(ctx);
>
> + /* The GL spec says that we should report an error if the unit is greater
> + * than GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, but in practice, only
> + * fixed-function units are usable. This is probably a spec bug.
> + * Ignore calls for non-fixed-func units, because we don't process
> + * glTexEnv for them either.
> + */
> + if (!texUnit)
> + return;
> +
> if (pname == GL_TEXTURE_ENV_COLOR) {
> params[0] = FLOAT_TO_INT( texUnit->EnvColor[0] );
> params[1] = FLOAT_TO_INT( texUnit->EnvColor[1] );
> params[2] = FLOAT_TO_INT( texUnit->EnvColor[2] );
> params[3] = FLOAT_TO_INT( texUnit->EnvColor[3] );
> }
> else {
> GLint val = get_texenvi(ctx, texUnit, pname);
> if (val >= 0) {
> *params = val;
> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
> index 628ac92..b0bfba5 100644
> --- a/src/mesa/main/texstate.c
> +++ b/src/mesa/main/texstate.c
> @@ -96,21 +96,21 @@ _mesa_copy_texture_state( const struct gl_context *src, struct gl_context *dst )
> if (src->Texture.Unit[u].CurrentTex[tex]) {
> dst->Texture.NumCurrentTexUsed =
> MAX2(dst->Texture.NumCurrentTexUsed, u + 1);
> }
> }
> dst->Texture.Unit[u]._BoundTextures = src->Texture.Unit[u]._BoundTextures;
> _mesa_unlock_context_textures(dst);
> }
> }
>
> - for (u = 0; u < src->Const.MaxCombinedTextureImageUnits; u++) {
> + for (u = 0; u < src->Const.MaxTextureCoordUnits; u++) {
> dst->Texture.FixedFuncUnit[u].Enabled = src->Texture.FixedFuncUnit[u].Enabled;
> dst->Texture.FixedFuncUnit[u].EnvMode = src->Texture.FixedFuncUnit[u].EnvMode;
> COPY_4V(dst->Texture.FixedFuncUnit[u].EnvColor, src->Texture.FixedFuncUnit[u].EnvColor);
> dst->Texture.FixedFuncUnit[u].TexGenEnabled = src->Texture.FixedFuncUnit[u].TexGenEnabled;
> dst->Texture.FixedFuncUnit[u].GenS = src->Texture.FixedFuncUnit[u].GenS;
> dst->Texture.FixedFuncUnit[u].GenT = src->Texture.FixedFuncUnit[u].GenT;
> dst->Texture.FixedFuncUnit[u].GenR = src->Texture.FixedFuncUnit[u].GenR;
> dst->Texture.FixedFuncUnit[u].GenQ = src->Texture.FixedFuncUnit[u].GenQ;
>
> /* GL_EXT_texture_env_combine */
> diff --git a/src/mesa/main/texstate.h b/src/mesa/main/texstate.h
> index b1f64d5..1a6d9eb 100644
> --- a/src/mesa/main/texstate.h
> +++ b/src/mesa/main/texstate.h
> @@ -59,20 +59,23 @@ _mesa_get_current_tex_unit(struct gl_context *ctx)
> /**
> * Return pointer to current fixed-func texture unit.
> * This the texture unit set by glActiveTexture(), not glClientActiveTexture().
> * \return NULL if the current unit is not a fixed-func texture unit
> */
> static inline struct gl_fixedfunc_texture_unit *
> _mesa_get_current_fixedfunc_tex_unit(struct gl_context *ctx)
> {
> unsigned unit = ctx->Texture.CurrentUnit;
>
> + if (unit >= ARRAY_SIZE(ctx->Texture.FixedFuncUnit))
> + return NULL;
> +
> return &ctx->Texture.FixedFuncUnit[unit];
> }
>
>
> static inline GLuint
> _mesa_max_tex_unit(struct gl_context *ctx)
> {
> /* See OpenGL spec for glActiveTexture: */
> return MAX2(ctx->Const.MaxCombinedTextureImageUnits,
> ctx->Const.MaxTextureCoordUnits);
>
More information about the mesa-dev
mailing list