[Mesa-dev] [PATCH v2 1/2] mesa: make glFramebuffer* check immutable texture level bounds

[Nicolai Hähnle]

On 26.01.2017 06:47, Ilia Mirkin wrote:
> When a texture is immutable, we can't tack on extra levels
> after-the-fact like we could with glTexImage. So check against that
> level limit and return an error if it's surpassed.
>
> The spec is a little unclear in that it says to check if "level is not a
> supported texture level", however that is never fully defined.
>
> This fixes:
> GL45-CTS.geometry_shader.layered_fbo.fb_texture_invalid_level_number
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> v1 -> v2: use NumLevels instead of _MaxLevel.
>
> Not sure why this isn't showing up as failing in the Intel CI, but it was
> definitely failing here.

Maybe the Intel CI is running the GLCTS based on the last 4.5 release, 
and I guess you're running off what's been published on Github? The 
GLCTS on Github has a bunch of new and possibly broken tests, and may 
still have a number of regressions as well (since a lot of code was 
moved around).

Can you point out which specific place of the spec you're talking about 
in your comment?

Thanks,
Nicolai

>
>  src/mesa/main/fbobject.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 6934805..6e86248 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -3128,16 +3128,22 @@ check_layer(struct gl_context *ctx, GLenum target, GLint layer,
>   * \return true if no errors, false if errors
>   */
>  static bool
> -check_level(struct gl_context *ctx, GLenum target, GLint level,
> -            const char *caller)
> +check_level(struct gl_context *ctx, const struct gl_texture_object *texObj,
> +            GLint level, const char *caller)
>  {
>     if ((level < 0) ||
> -       (level >= _mesa_max_texture_levels(ctx, target))) {
> +       (level >= _mesa_max_texture_levels(ctx, texObj->Target))) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "%s(invalid level %d)", caller, level);
>        return false;
>     }
>
> +   if (texObj->Immutable && level >= texObj->NumLevels) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "%s(level %u >= %u)", caller, level, texObj->NumLevels);
> +      return false;
> +   }
> +
>     return true;
>  }
>
> @@ -3260,7 +3266,7 @@ framebuffer_texture_with_dims(int dims, GLenum target,
>        if ((dims == 3) && !check_layer(ctx, texObj->Target, layer, caller))
>           return;
>
> -      if (!check_level(ctx, textarget, level, caller))
> +      if (!check_level(ctx, texObj, level, caller))
>           return;
>     }
>
> @@ -3328,7 +3334,7 @@ _mesa_FramebufferTextureLayer(GLenum target, GLenum attachment,
>        if (!check_layer(ctx, texObj->Target, layer, func))
>           return;
>
> -      if (!check_level(ctx, texObj->Target, level, func))
> +      if (!check_level(ctx, texObj, level, func))
>           return;
>
>        if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
> @@ -3370,7 +3376,7 @@ _mesa_NamedFramebufferTextureLayer(GLuint framebuffer, GLenum attachment,
>        if (!check_layer(ctx, texObj->Target, layer, func))
>           return;
>
> -      if (!check_level(ctx, texObj->Target, level, func))
> +      if (!check_level(ctx, texObj, level, func))
>           return;
>
>        if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
> @@ -3419,7 +3425,7 @@ _mesa_FramebufferTexture(GLenum target, GLenum attachment,
>        if (!check_layered_texture_target(ctx, texObj->Target, func, &layered))
>           return;
>
> -      if (!check_level(ctx, texObj->Target, level, func))
> +      if (!check_level(ctx, texObj, level, func))
>           return;
>     }
>
> @@ -3459,7 +3465,7 @@ _mesa_NamedFramebufferTexture(GLuint framebuffer, GLenum attachment,
>                                          &layered))
>           return;
>
> -      if (!check_level(ctx, texObj->Target, level, func))
> +      if (!check_level(ctx, texObj, level, func))
>           return;
>     }
>
>