[Piglit] [PATCH 0/6] Cleanup and prepare for pre OpenGL 3.0 drivers

Fri Apr 1 20:11:36 UTC 2016

On 31/03/16 17:26, Emil Velikov wrote:
> On 31 March 2016 at 12:21, Jose Fonseca <jfonseca at vmware.com> wrote:
>> On 31/03/16 00:44, Emil Velikov wrote:
>>>
>>> On 30 March 2016 at 21:54, Jose Fonseca <jfonseca at vmware.com> wrote:
>>>>
>>>> On 30/03/16 11:14, Emil Velikov wrote:
>>>>>
>>>>>
>>>>> On 29 March 2016 at 23:53, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>>
>>>>>>
>>>>>> Isn't this backwards? Shouldn't waffle just work without the
>>>>>> ARB_create_context or whatever ext and just create a context and see
>>>>>> what version it is?
>>>>>>
>>>>> It isn't, imho.
>>>>>
>>>>> Waffle follows the GL winsys semantics, as such it should not do more
>>>>> (create a context, make it current, etc) unless explicitly asked. It
>>>>> translates the winsys/platform specifics, not the GL crazy. Although I
>>>>> can see how one would want both to be hidden/handled in waffle.
>>>>>
>>>>> That's my take on it, at least. If people familiar with waffle think
>>>>> I've misunderstood things, please shout.
>>>>>
>>>>> -Emil
>>>>
>>>>
>>>>
>>>> FWIW, I agree with Ilia.  Ideally Waffle should abstract away these sort
>>>> of
>>>> behavioral differences.
>>>>
>>>> On Mac, there isn't even WGL/GLX_ARB_create_context equivalent.
>>>>
>>> Indeed that's correct. I'm afraid I don't get how that's applicable here
>>> :-(
>>>
>>>> Furthermore, not setting WAFFLE_CONTEXT_DEBUG for 2.x context is IMO a
>>>> functionality regression here. Many implementations support debug for 2.x
>>>> contexts, and it can be quite useful for diagnosing issues.
>>>>
>>> Can you please elaborate - are you talking about a 3.0+ capable
>>> driver, where the user requests a debug context and version 2.x via
>>> ARB_create_context ? It does sound a bit strange, but sure.
>>
>>
>> Yes, if somebody wants to debug a 2.x OpenGL application, there's no other
>> way to do it.
>>
>> apitrace does this for the record.  By default it will enable debug context
>> on every context.  Even if it was a 1.x context.
>>
>> There's nothing strange about it.
>>
> "Strange" being - one is actually working out how to request a debug
> context and at the same time they're not using any of the 'performance
> enhancing' GL 3.0+ goodies.
>
>> Note https://www.opengl.org/registry/specs/KHR/debug.txt says the minimum
>> required is OpenGL 1.1.  Not 3.0.  Allowing debugging 1.x 2.x was a design
>> goal from the start.
>>
>>>
>>>>
>>>> So when a app requests a context <= 3.0
>>>
>>> Am I loosing my mind here or the comparison is the wrong way around ?
>>> It should be requested_context >= 3.0, right ?
>>
>>
>> No, I wrote what I meant...
>>
>> I don't think anynody is arguing what needs to be done for context > 3.0 --
>> the host should support WGL/GLX_ARB_create_context anyway and we should use
>> it.
>>
>>
>> My focus here is how to handle requests for 1.x/2.x context.  You propose
>> always use GLXCreateContext, I say that's throwing away the baby with the
>> water.
>>
> Arr there seems to be a misscomunication here - the this is exactly
> what I was thinking/talking about (a few lines below).
>
>>
>>>
>>>> : if the OpenGL implementation
>>>> supports WGL/GLX_ARB_create_context waffle should use it.  If not, it
>>>> should
>>>> request a ordinary context, and check the version matches the requested.
>>>>
>>
>>> Waffle can use ARB_create_context for 3.0+/debug/fwd compat context,
>>> although it cannot do the make_current and alike for on behalf of the
>>> user. It doesn't need to either - piglit already checks the version
>>> :-)
>>>
>>> Does this sound like a good middle ground ?
>>
>>
>> I don't really understand what you're proposing.  I suspect we're talking
>> about different things.
>>
>>
>> Let me see if I can explain this differently.  Here is what I'd expect from
>> Waffle as app developer:
>>
>> - If my app requests a 2.1 context and WGL/GLX_ARB_create_context is not
>> supported, Waffle should still create a context using
>> wglCreateContext/glxCreateContext.  (The point of using wrappers like Waffle
>> is hide away that complexity, otherwise one's better off talking the GLX/WGL
>> directly.)
>>
>> - If an app requests a 2.1 debug context, Waffle should just honour it if
>> WGL/GLX_ARB_create_context.
>>
> Minor clarification:
>
>   - If an app requests a 2.1 debug context, Waffle should just honour it if
> WGL/GLX_ARB_create_context or error otherwise.
>
> Right ?

Right.

>
> This is precisely what I was saying earlier... so I believe I
> understood you the first time ;-)
> Just in case in an alternative form:
>
> (somewhere in waffle)
>
> if (requested_version > 3.0 ||
>      requested_debug ||
>      requested_fwd_compat)
>     use_arb_create_context
> else
>     use_legacy_create_context

Sounds good!

>>
>> This is BTW, what other similar wrappers do:
>>
>> - https://github.com/raedwulf/glfw/blob/master/src/glx_context.c#L401
>>
>> - https://github.com/apitrace/apitrace/blob/master/retrace/glws_glx.cpp#L345
>>
> Afaics there is a small (but significant) difference between waffle
> and the above two. If you want a debug context and there's no way to
> create one waffle will give you an error, as opposed to silently
> ignoring you and creating a non-debug one ;-)
>
>> In fact, looking at waffle src/waffle/glx/glx_context.c , it already
>> fallbacks to glXCreateNewContext when it's not supported.
>>
>>
>> So I don't understand why is Patch 6/6 necessary at all.
>>
> Because seemingly things don't work. As mentioned previously we can
> tweak waffle and drop the patch. Does that mean that the first 5 are
> ok ?

Yes, the first 5 LGTM.  And are

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>