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

[Ilia Mirkin]

Wait, so are we saying that my original patch is good? I've
beyond-lost context on all this, but as I recall my original patch had
some issues.

As I don't have Khronos access to the bug in question, I'm not sure
what the right thing is. The original patch applies this logic to
gl[Named]FramebufferTexture[Layer|1D|2D|3D]

Is that correct? Is there some sort of late-binding thing that should
happen (e.g. if you do glFramebufferTexture2D(), does it use the
currently-bound TEXTURE_2D, or is that resolved later? (I haven't
checked.)

  -ilia

On Fri, Mar 24, 2017 at 8:17 AM, Antía Puentes <apuentes at igalia.com> wrote:
> Reviewed-by: Antia Puentes <apuentes at igalia.com>
>
>
> On jue, 2017-01-26 at 00:47 -0500, 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.
>>
>>  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;
>>     }
>>