[Mesa-dev] [PATCH 1/9] mesa/es3.1: Allow binding GL_DRAW_INDIRECT_BUFFER with gles 3.1

[Emil Velikov]

On 11 May 2015 at 20:09, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, May 11, 2015 at 3:02 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 05/11/2015 08:23 AM, Ilia Mirkin wrote:
>>> On Mon, May 11, 2015 at 9:03 AM, Marta Lofstedt
>>> <marta.lofstedt at linux.intel.com> wrote:
>>>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>>>
>>>> Signed-off-by: Marta Lofstedt <marta.lofstedt at intel.com>
>>>> ---
>>>>  src/mesa/main/bufferobj.c | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
>>>> index 66dee68..07f82cd 100644
>>>> --- a/src/mesa/main/bufferobj.c
>>>> +++ b/src/mesa/main/bufferobj.c
>>>> @@ -91,8 +91,9 @@ get_buffer_target(struct gl_context *ctx, GLenum target)
>>>>     case GL_COPY_WRITE_BUFFER:
>>>>        return &ctx->CopyWriteBuffer;
>>>>     case GL_DRAW_INDIRECT_BUFFER:
>>>> -      if (ctx->API == API_OPENGL_CORE &&
>>>> -          ctx->Extensions.ARB_draw_indirect) {
>>>> +      if ((ctx->API == API_OPENGL_CORE &&
>>>> +           ctx->Extensions.ARB_draw_indirect) ||
>>>> +           _mesa_is_gles31(ctx)) {
>>>
>>> Similar to my comment on the other patch (and if this occurs in the
>>> other patches, I'd have the same comment there again). I think it's
>>> confusing, the way you're mixing things. Also it'll lead to backend
>>> drivers potentially receiving things they're not ready for. IMHO this
>>> should become
>>>
>>> if ((ctx->API == API_OPENGL_CORE || _mesa_is_gles31(ctx)) &&
>>> ctx->Extensions.ARB_draw_indirect)
>>
>> Before these patches were sent out for review, they were written in this
>> way.  I had suggested changing it to the current method.
>> GL_ARB_draw_indirect isn't an ES extension, so checking that case seemed
>> weird to me.
>
> Yeah, but we don't have GL vs GLES vs GLES3.1 driver API's. We just
> have a single API, and the enable bit for "I can do indirect draws" is
> that you set Extensions.ARB_draw_indirect to true. In a desktop GL
> context, this also means that GL_ARB_draw_indirect is reported in the
> extension string. I could easily imagine a hypothetical GLES ext that
> could be enabled by the same bit (although there is none afaik).
>
> Basically the question is what do we want to have happen when you have
> a driver that doesn't quite support GL (ES) version X, but you use a
> version override, and then try to use one of the features that version
> X provides but the driver isn't quite ready for. Do you get a
> (potential) library crash/otherwise inconsistent state? Or do you just
> get an error at the API level (which the spec doesn't allow for, since
> the feature is supposed to exist)?
>
A while back I was under the strange impression - the extension
booleans are set per API. Thankfully Marek and/or Brian, did point out
that they are meant as a capability flags, and the state-tracker
can/should check them accordingly. Perhaps one can even envision a way
to use GL_ARB_foo to implement a short cut internally :-)

For the purposes of glGetString(GL_EXTENSIONS) everything is handled
nicely (in src/mesa/main/extensions.c), and we never expose the
incorrect string to the user.

TLDR: I'm in favour of Ilia's suggestion.

-Emil