On Wed, May 19, 2010 at 11:39 PM, Chia-I Wu <olva...@gmail.com> wrote: > On Wed, May 19, 2010 at 2:48 PM, Jakob Bornecrantz <wallbra...@gmail.com> > wrote: >> 2010/5/19 Chia-I Wu <olva...@gmail.com>: >>> 2010/5/18 Kristian Høgsberg <k...@bitplanet.net>: >>>> 2010/5/18 Chia-I Wu <olva...@gmail.com>: >>>>> 2010/5/18 Kristian Høgsberg <k...@bitplanet.net>: >>>>>> On Tue, May 18, 2010 at 12:41 PM, Chia-I Wu <olva...@gmail.com> wrote: >>>>>>> 2010/5/18 Jakob Bornecrantz <wallbra...@gmail.com>: >>>>>>>> 2010/5/17 Kristian Høgsberg <k...@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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
