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

[Ilia Mirkin]

On Mon, Jan 30, 2017 at 1:06 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> 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

A thought occurred to me... whereas glFramebufferTexture1D/etc talk
about binding points, glFramebufferTexture talks about a specific
texture object. When are the bindings resolved? at
glFramebufferTexture2D time, or at draw time? If the latter, then the
spec has no choice but to just check the maxima for the binding point
ones...

  -ilia