[Mesa-dev] Mesa (master): Revert "egl: Check if API is supported when using eglBindAPI."

[Ian Romanick]

On 06/06/2016 02:10 AM, Michel Dänzer wrote:
> On 04.06.2016 00:10, Marek Olšák wrote:
>> On Fri, Jun 3, 2016 at 4:33 PM, Dieter Nützel <Dieter at nuetzel-hh.de> wrote:
>>> Am 03.06.2016 11:47, schrieb Michel Dänzer:
>>>>
>>>> On 03.06.2016 18:34, Marek =?UNKNOWN?B?T2zFocOhaw==?= wrote:
>>>>>
>>>>> Module: Mesa
>>>>> Branch: master
>>>>> Commit: 8c361e84ad010552a42593fad4130befc58e9a6a
>>>>> URL:
>>>>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=8c361e84ad010552a42593fad4130befc58e9a6a
>>>>>
>>>>> Author: Marek Olšák <marek.olsak at amd.com>
>>>>> Date:   Fri Jun  3 11:25:19 2016 +0200
>>>>>
>>>>> Revert "egl: Check if API is supported when using eglBindAPI."
>>>>>
>>>>> This reverts commit e8b38ca202fbe8c281aeb81a4b64256983f185e0.
>>>>>
>>>>> It broke Glamor for Gallium at least.
>>>>
>>>>
>>>> It exposed a bug in glamor, which is fixed by
>>>>
>>>> https://patchwork.freedesktop.org/patch/91214/
>>>
>>>
>>> So what route should we take?
>>>
>>> Wait for the distros to catch up and enable it then, again?
>>>
>>> I was fallen in this, too.
>>>
>>> openSUSE 13.2 / Leap 42.1
>>>
>>> /usr/bin/Xorg: symbol lookup error:
>>> /usr/lib64/xorg/modules/drivers/radeon_drv.so: undefined symbol:
>>> exaGetPixmapDriverPrivate
>>
>> The commit caused Glamor to fail with:
>> (WW) glamor0: Failed to get GLSL version
>>
>> We can't just kill Glamor support with a Mesa commit.
> 
> Why do released versions of xserver/glamor have to work with unreleased
> versions of Mesa?
> 
> 
>> - keep the current eglBindAPI behavior forever
> 
> So because we haven't been following the EGL spec, allowing broken EGL
> apps to work by accident, we have to preserve that bug forever? I'm not
> buying it.

Well... there may be other options, and we should explore those.  There
are certainly other cases where we (sometimes with a driconf option)
deviate slightly from the spec to work-around application bugs.

Looking at the glamor patch and the Mesa patch... I'm not sure the
original Mesa change was correct.  Since eglBindAPI doesn't take a
display parameter, I'm not 100% sure that it's valid for it to generate
EGL_NOT_INITIALIZED.

Many EGL commands explicitly state that they can generate
EGL_NOT_INITIALIZED, and "Errors whose meanings are identical across
many functions (such as returning EGL_BAD_DISPLAY or EGL_NOT_INITIALIZED
for an unsuitable EGLDisplay argument) may not be described repeatedly."

It also says:

    An uninitialized display may be passed to the functions
    eglInitialize, eglTerminate, and in some cases eglMakeCurrent. All
    other EGL functions which take a display argument will fail and
    generate an EGL_NOT_INITIALIZED error when passed a valid but
    uninitialized display.

And, finally:

    EGL_NOT_INITIALIZED
        EGL is not initialized, or could not be initialized, for the
        specified display.  Any command may generate this error.

So... I'm not sure what is correct.  Do we have any idea what other EGL
implementations do?

> To me, the bigger issue with the change you reverted was that it caused
> the piglit test spec at egl_khr_fence_sync to hang. Has anyone looked into
> that?