[Mesa-dev] [PATCH] mesa: implement GL_ARB_texture_buffer_range v5

[Christoph Bumiller]

On 28.01.2013 19:54, Ian Romanick wrote:
> On 01/27/2013 12:19 PM, Eric Anholt wrote:
>> Christoph Bumiller <e0425955 at student.tuwien.ac.at> writes:
>>
>>> On 27.01.2013 00:58, Eric Anholt wrote:
>>>> Christoph Bumiller <e0425955 at student.tuwien.ac.at> writes:
>>>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>>>> index 31a559e..e71f6e1 100644
>>>>> --- a/src/mesa/main/teximage.c
>>>>> +++ b/src/mesa/main/teximage.c
>>>>> +/** GL_ARB_texture_buffer_object */
>>>>> +void GLAPIENTRY
>>>>> +_mesa_TexBuffer(GLenum target, GLenum internalFormat, GLuint buffer)
>>>>> +{
>>>>> +   struct gl_buffer_object *bufObj;
>>>>> +
>>>>> +   GET_CURRENT_CONTEXT(ctx);
>>>>> +
>>>>> +   if (!ctx->Extensions.ARB_texture_buffer_object) {
>>>>> +      _mesa_error(ctx, GL_INVALID_OPERATION, "glTexBuffer");
>>>>> +      return;
>>>>> +   }
>>>> The check for also ctx->API == API_OPENGL_CORE here has been
>>>> dropped, so
>>>> I think the i965 driver would start accepting this function in compat
>>>> contexts when it shouldn't.
>>>>
>>>> If that check gets re-added, this is
>>>> Reviewed-by: Eric Anholt <eric at anholt.net>
>>>
>>> To quote Ian on a reply to an earlier version of this, where I had the
>>> check in the _mesa_TexBufferRange function also:
>>>
>>> "
>>> if (!(ctx->API == API_OPENGL_CORE &&
>>>
>>> This check /shouldn't/ be necessary.  Since Paul's rework of the
>>> dispatch table code, the TexBufferRange function should only be put in
>>> the dispatch table if the context is the correct profile.  A trivial
>>> piglit test will verify that: check that GL_INVALID_OPERATION is
>>> generated if GL_ARB_texture_buffer_range is not supported and
>>>
>>>      glTexBufferRange(GL_TEXTURE_BUFFER, GL_RGBA, 0, 0, 0);
>>>
>>> is called.
>>> "
>>>
>>> Why would this work for TexBufferRange and not TexBuffer non-Range ?
>>
>> Oh, I forgot Ian ruled that Intel can expose it on non-core as well, so
>> the code before was just broken (we never noticed, since the test
>> requires 1.40, which we don't have for compat).
>>
>> (the thing Ian was probably thinking of, though, was the "deprecated"
>> flag in the XML for leaving things on in compat and removing them from
>> core, not the other way around)
>
> I just talked to Paul, and it turns out I was wrong.  We only leave
> out functions that are marked as deprecated.  Sorry about that. :(
>

So do I have to re-add the check now, in both TexBuffer and TexBufferRange ?
"that Intel can expose it on non-core as well, so the code before was
just broken" sounds like the check should be gone anyway ...

(Also, it looks hackish; using the dispatch table like you thought was
the case seems much nicer.
And why does calling unsupported API entry points have to have defined
behaviour ... a crash would be much for verbose ;)