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

[Ilia Mirkin]

On Mon, Jan 30, 2017 at 12:26 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> 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.

https://github.com/KhronosGroup/VK-GL-CTS/blob/c9921995d8d360bd34d8672194d7c095bb376f82/external/openglcts/modules/glesext/geometry_shader/esextcGeometryShaderLayeredFBO.cpp#L1062

  -ilia