[Mesa-dev] [PATCH 1/2] egl: add optional plat_opt to _eglFindDisplay()

[Adam Jackson]

On Thu, 2019-04-04 at 16:25 +0100, Emil Velikov wrote:

> I'm not a huge fan of this. Yet again the other platform (x11) that uses
> these has a number of issues, see below for details. Once those are resolved
> we could try and make this more uniform.
> 
>  a) spec does not mention if a new (or the existing) EGLDislay should be
> given for same native_dpy/screen_no combo.

An EGLDisplay maps to an X11 screen. EGL_KHR_platform_x11 says:

>     On the X11 platform, an EGLDisplay refers to a specific X11 screen rather
>     than an X11 display connection.

And EGL 1.5 says (with similar text in EGL_EXT_platform_base):

> Multiple calls made to eglGetPlatformDisplay with the same parameters will
> return the same EGLDisplay handle.

EGL_KHR_platform_x11 is a little confusing on this point in one of the
issues, because it says a new display connection is created for
EGL_DEFAULT_DISPLAY; presumably that's only for the first EGLDisplay
you create though.

>  b) the implementation has number of issues:
>  - screen 0 is hardcoded when using DEFAULT_DISPLAY

False, but:

>  - screen is _ignored_ when using !DEFAULT_DISPLAY

true, but easily fixed. We definitely store the screen number in
disp->Options.Platform in _eglParseX11DisplayAttribList, and then:

> static EGLBoolean                                                              
> dri2_get_xcb_connection(_EGLDriver *drv, _EGLDisplay *disp,                    
>                         struct dri2_egl_display *dri2_dpy)                     
> {                                                                              
>    xcb_screen_iterator_t s;                                                    
>    int screen = (uintptr_t)disp->Options.Platform;                             
>    const char *msg;                                                            
>                                                                                
>    disp->DriverData = (void *) dri2_dpy;                                       
>    if (disp->PlatformDisplay == NULL) {                                        
>       dri2_dpy->conn = xcb_connect(NULL, &screen);                             
>       dri2_dpy->own_device = true;

If no display was given, we connect however xcb would, and save what it
decided was the default screen number.

>    } else {                                                                    
>       Display *dpy = disp->PlatformDisplay;                                    
>                                                                                
>       dri2_dpy->conn = XGetXCBConnection(dpy);                                 
>       screen = DefaultScreen(dpy);                                             
>    }

If not, we use what the display already had set; this is where we
ignore the user's screen number and instead use the display's default.

I think the fix is to delete that DefaultScreen line above, and in
_eglFindDisplay call the appropriate platform's vtable to check for
identical option lists (this is your _eglSameDeviceDisplay in the next
patch, just abstracted out a bit).

- ajax