[Mesa-dev] [PATCH 04/13] mesa: decrease the array size of ctx->Texture.FixedFuncUnit to 8
Brian Paul
brianp at vmware.com
Mon Feb 12 22:56:34 UTC 2018
Minor nits below.
Otherwise, for the series, Reviewed-by: Brian Paul <brianp at vmware.com>
On 02/08/2018 06:18 PM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> GL allows doing glTexEnv on 192 texture units, while in reality,
> only MaxTextureCoordUnits units are used by fixed-func shaders.
>
> There is a piglit patch that adjusts piglits/texunits to check only
> MaxTextureCoordUnits units.
> ---
> 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 4c5f9dc..f811ab5 100644
> --- a/src/mesa/main/enable.c
> +++ b/src/mesa/main/enable.c
> @@ -202,20 +202,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;
return GL_FALSE since the return type is GLboolean. Or change the
function to use bool/true/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;
> @@ -1286,20 +1288,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;
again.
> +
> 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 562fb17..3a4fdb5 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1363,21 +1363,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 2b05630..f9f50a3 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 cf3f7e5..c0b73b1 100644
> --- a/src/mesa/main/texstate.h
> +++ b/src/mesa/main/texstate.h
> @@ -58,20 +58,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