[Mesa-dev] [PATCH] egl: Introduce alternative native display types

[Jakob Bornecrantz]

On Wed, May 19, 2010 at 11:39 PM, Chia-I Wu <olvaffe at gmail.com> wrote:
> On Wed, May 19, 2010 at 2:48 PM, Jakob Bornecrantz <wallbraker at gmail.com> wrote:
>> 2010/5/19 Chia-I Wu <olvaffe at gmail.com>:
>>> 2010/5/18 Kristian Høgsberg <krh at bitplanet.net>:
>>>> 2010/5/18 Chia-I Wu <olvaffe at gmail.com>:
>>>>> 2010/5/18 Kristian Høgsberg <krh at bitplanet.net>:
>>>>>> On Tue, May 18, 2010 at 12:41 PM, Chia-I Wu <olvaffe at gmail.com> wrote:
>>>>>>> 2010/5/18 Jakob Bornecrantz <wallbraker at gmail.com>:
>>>>>>>> 2010/5/17 Kristian Høgsberg <krh at bitplanet.net>:
>>>>>>>>> The EGL native platform API is determined at compile time and resolves
>>>>>>>>> to Win32, X11 or Symbian at this point.   This means that if we want to
>>>>>>>>> support XCB or a native DRM implementation, they have to be their platforms
>>>>>>>>> and result in different libEGL.so's with identical ABI but different
>>>>>>>>> entrypoint semantics.  From a distro point of view, and really, any point
>>>>>>>>> of view, this is a mess, since the different libraries can't easily co-exist
>>>>>>>>> without fiddling with linker paths.  And if you get it wrong, an application
>>>>>>>>> requiring the X11 platform libEGL.so will happily link against any other
>>>>>>>>> libEGL.so and segfault when eglGetDisplay() doesn't do what it expects.
>>>>>>>>>
>>>>>>>>> We can get around this by overloading the X11 native entrypoints instead.
>>>>>>>>> The X Display struct has as its first member a pointer to XExtData.  The
>>>>>>>>> pointer will always have bits 0 and 1 set to 0, which we can use to
>>>>>>>>> distinguish a struct that's not an X display.  This lets us pass in a
>>>>>>>>> custom struct that can provide initialization data for other platforms
>>>>>>>>> in the same libEGL.so.
>>>>>>>>>
>>>>>>>>> The convention we're establishing is that the first field of such a struct
>>>>>>>>> must be a pointer, so as to overlap with the layout of the X display
>>>>>>>>> struct.  We can then enummerate the different display types using odd
>>>>>>>>> numbers cast to a pointer (ensuring bit 0 is set).  This patch introduces
>>>>>>>>> two new types of displays: EGL_DISPLAY_TYPE_DRM_MESA which lets us
>>>>>>>>> initialize EGL directly on a DRM file descriptor or device filename and
>>>>>>>>> EGL_DISPLAY_TYPE_XCB_MESA which lets us initialize EGL on X using an
>>>>>>>>> xcb_connection_t instead of a classic Xlib Display.
>>>>>>>>
>>>>>>>> This sounds good to me, there are just some minor nitpicks that would
>>>>>>>> be nice to be resolved.
>>>>>>>>
>>>>>>>> 1. Could you write some docu about EGLDisplayTypeDRMMESA, like that
>>>>>>>> the display should return the FD after init in the struct. Also what
>>>>>>>> happens if (drm->device_name == null && drm->fd < 0) looking at the
>>>>>>>> code it would fail to load is this what we want?
>>>>>>> I think the EGL prefix should be dropped.  We are defining a new
>>>>>>> platform here: a platform whose display can be an xlib display, xcb
>>>>>>> connection or drm.  It should not have the "EGL" prefix.
>>>>>>
>>>>>> That is a good point - so we'll call it MesaDisplayTypeDRM and
>>>>>> MESA_DISPLAY_TYPE_DRM and similar for XCB.
>>>>>>
>>>>>>> Actually, I think this platform should have its own header file.
>>>>>>> eglplatform.h will include the header file and typedef the native
>>>>>>> types defined there to EGL types.
>>>>>> You mean that the native types for the other platforms are defined in
>>>>>> their respective header files and so should these new types?  I can
>>>>>> see that point of view, but from a more pragmatic point of view, I
>>>>>> don't think it's worth the trouble. Also, the types are specific to
>>>>>> the mesa implementation, they are not data types from another display
>>>>>> system or platform, and the Khronos implementors guide suggests that
>>>>>> the vendor/implementor can modify eglplatform.h.
>>>>> I just figured that if an application uses xcb_connection_t for the
>>>>> display, it might as well use xcb types for windows and pixmaps.
>>>> That's the expected behaviour.  When using XCB, windows and pixmaps
>>>> are still just uint32_t handles on the client side and passing them to
>>>> eglCreateWindowSurface() or eglCreatePixmapSurface() will work just
>>>> fine.  The way Xlib on XCB works, they share the same client ID
>>>> allocator, so you could even initialize EGL with an XCB connection and
>>>> pass it windows created using Xlib or the other way around.
>>> What do you think if these platform details are moved to a header file
>>> of its own, and the header is distributed with Mesa EGL?  I'd like to
>>> see eglplatform.h have less platform details.  We may then put some
>>> macros into the new platform header that ease the access of the native
>>> display.
>> Isn't that what eglplatform.h is for?
> There should be a platform first.  Then eglplatform.h may be updated
> to support the platform.
>
> I would consider the combination of xlib/xcb/drm as a new platform.
> Because I expect there to be some accessor macros that egl_dri2 (and
> other drivers) can use to access the native display, instead of
> manually casting the native display to struct generic_display.  Having
> these macros in eglplatform.h seems weird to me.

Right you are, I always imagned these to be in the egl implenetation
in src/egl/main and not exported to the app as it does not need to
know about these.

>>>>> Things could get really complex.  Do you have in mind how/when
>>>>> xcb_connection_t will be used?
>>>>
>>>> Using the xcb_connection_t display makes sense if your toolkit or
>>>> application is using XCB already and you want to avoid the Xlib
>>>> dependency.  It was easy for me to add to egl_dri2, since it already
>>>> uses XCB for DRI2 protocol, which avoids another copy of the DRI2
>>>> protocol code.
>>>>
>>>> What is a little complex though is how to define/use the window/pixmap
>>>> surface constructors under EGL on DRM.  I don't think we can really
>>>> use them in that kind of setting.  First of all, on the "unix
>>>> platform" they take an XID (uint32_t), not a pointer as
>>>> eglGetDisplay() does, so we can't pass a pointer to a struct like what
>>>> I'm suggesting for eglGetDisplay() in this patch.  That would break on
>>>> 64 bit platforms.
>>>>
>>>> One option is to add a new entrypoint to create some kind of native
>>>> window or pixmap under a DRM display, which returns a uint32_t, which
>>>> we can then pass to eglCreateWindowSurface().  Or we can do as the
>>>> EGL_MESA_screen_surface extension does and just define a new
>>>> EGLSurface constructor.  That is certainly a simpler and more elegant
>>>> approach, but it leaves the eglCreateWindowSurface() and
>>>> eglCreatePixmapSurface() entrypoints useless.  But maybe they were
>>>> useless all along and we would have been better off with strongly
>>>> typed, platform specific surface constructors instead of this pseudo
>>>> generic mess we have now (oops, I fell into one of my old rants [1]).
>>>>
>>>> The last option I can think of is to avoid EGLSurfaces alltogether and
>>>> just use FBOs.  My plan for EGL on KMS is to add an extension that
>>>> lets us create an EGLImage from scratch (that is, not lifted from a
>>>> client API object) and adds eglQueryImageMESA() to get the kernel mm
>>>> handle and stride of the buffer so we can use it directly with the DRM
>>>> KMS API.  Then set that EGLImage as a renderbuffer for your FBO,
>>>> render to it and then use libdrm to configure scanout from that
>>>> EGLImage.  It's more complex than the mesa extension, but it give the
>>>> application all the control over double/triple/n buffering,
>>>> preserve/discard backbuffer contents, throttling and page flipping.
>>>> It lets applications use the libdrm KMS API directly, which, while
>>>> complex, is the only real option if you want to base a display server
>>>> on DRM on KMS and expose all the features there.  Of course, this
>>>> option can co-exist with the mesa extension, so we can expose both the
>>>> easy-to-use, non-drm-specific extension and the more complex but
>>>> full-featured libdrm API.
>>> Yeah, I had a look at your branch some time ago and I liked the idea.
>>> Though I'd like to see eglCreatePbufferSurface be used to create a
>>> "system pbuffer", and eglQuerySurface be used to query the buffer
>>> handle and stride.  EGLImages are then created from the system
>>> pbuffers.  This is just some ideas and I haven't verified if they
>>> work.
>> TBH I prefer to just go to a EGLImage directly, there are some things
>> which doesn't work out that well with Surfaces, like which handle to
>> return if it has a depth buffer.

Sounds like we have concencus Kristian, can you post a updated patch?

Cheers Jakob.