Christoph Bumiller <e0425...@student.tuwien.ac.at> writes:
> On 28.01.2013 19:54, Ian Romanick wrote:
>> On 01/27/2013 12:19 PM, Eric Anholt wrote:
>>> Christoph Bumiller <e0425...@student.tuwien.ac.at> writes:
>>>
>>>> On 27.01.2013 00:58, Eric Anholt wrote:
>>>>> Christoph Bumiller <e0425...@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 <e...@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 ...Agreed, it was broken. This is a behavior change slipped into another patch, but it happens to be a good one. I'd love to see it done as a patch before the refactor, since we've had a bunch of discussion on it. Either way, it's got my r-b. > (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 ;) What's worse would be some undefined behavior where the call runs to completion and doesn't throw an error. Then they never figure it out until we make a change and apps start to crash.
pgpob6kSJm7H_.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
