[Mesa-dev] [PATCH 10/10] egl: add EGL_platform_device support

[Emil Velikov]

On 29 August 2018 at 12:16, Eric Engestrom <eric.engestrom at intel.com> wrote:

>> +static int
>> +_eglCompareDeviceDisplay(_EGLDisplay *dpy, void *plat_opt)
>
> This is a true/false check, so we I think should return a bool and
> change name to reflect which "comparison" returns true, and you could
> avoid the inline cast by taking the right param type, so how about:
>
>   static bool
>   _eglSameDeviceDisplay(_EGLDisplay *dpy, _EGLDeviceDisplayOptions *plat_opt)
>
That's better indeed.


>> +
>> +         /* TODO: add the missing define to the public headers, sync our copy */
>
> Let's do that before this patch lands?
>
Yes, ideally. Khronos seems to be moving very slowly, so we might have
to keep that for a little bit.


>> +
>> +   /* TODO: Change Options.Platform under a lock, to avoid a race.
>> +    * X11 handling above need a similar fix.
>> +    */
>
> I'm assuming you will do this before landing this patch, right?
>
Was hoping to do that as a follow-up. As the comment says X11 has the
same race condition, plus in general the option handling is "broken".


>> --- a/src/egl/meson.build
>> +++ b/src/egl/meson.build
>> @@ -118,6 +118,7 @@ if not with_platform_haiku
>>    if with_platform_surfaceless
>>      files_egl += files('drivers/dri2/platform_surfaceless.c')
>>    endif
>> +  files_egl += files('drivers/dri2/platform_surfaceless.c')
>
> typo: s/surfaceless/device/
>
> and I would prefer for this to be added in the `if with_dri2` (around
> line 88) instead of in the middle here, where it gets lost in the middle
> of the `if with_platform_*` checks.
>
Ack, will do.


Thanks
Emil