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

[Nicolai Hähnle]

On 30.01.2017 18:23, Ilia Mirkin wrote:
> On Mon, Jan 30, 2017 at 4:36 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Mon, Jan 30, 2017 at 4:33 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>> 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?
>>
>> One of the errors listed for glFramebufferTexture is:
>>
>> """
>> An INVALID_VALUE error is generated if texture is not zero and is not the
>> name of a texture object, or if level is not a supported texture level
>> for texture
>> """
>
> Curiously for glFramebufferTexture1D/2D/3D, it also says:
>
> """
> If textarget is TEXTURE_RECTANGLE or TEXTURE_2D_MULTISAMPLE, then
> level must be zero. If textarget is TEXTURE_3D, then level must be
> greater than or equal to zero and less than or equal to log2 of the
> value of MAX_3D_TEXTURE_- SIZE. If textarget is one of the cube map
> face targets from table 8.19, then level must be greater than or equal
> to zero and less than or equal to log2 of the value of
> MAX_CUBE_MAP_TEXTURE_SIZE. For all other values of textarget, level
> must be greater than or equal to zero and no larger than log2 of the
> value of MAX_- TEXTURE_SIZE.
> """
>
> which matches the current code. I guess this patch is withdrawn...

What a coincidence, I've been staring at this for a while now and been 
getting increasingly confused. Which function does the CTS actually test 
(and possibly fail incorrectly)? Maybe it's time to open an issue 
against the CTS.

Cheers,
Nicolai

>
>   -ilia
>