Minor nits below.

Otherwise, for the series, Reviewed-by: Brian Paul <bri...@vmware.com>


On 02/08/2018 06:18 PM, Marek Olšák wrote:
From: Marek Olšák <marek.ol...@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);


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to