[PATCH weston 1/4] gl-renderer: use eglGetPlatformDisplayEXT to get an EGLDisplay
Derek Foreman
derekf at osg.samsung.com
Mon Feb 23 09:44:36 PST 2015
Very cool, comments below.
On 17/02/15 09:48 AM, Jonny Lamb wrote:
> ---
> src/compositor-drm.c | 14 ++++++++-
> src/compositor-fbdev.c | 2 +-
> src/compositor-wayland.c | 21 +++++++++++--
> src/compositor-x11.c | 14 ++++++++-
> src/gl-renderer.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
> src/gl-renderer.h | 6 +++-
> 6 files changed, 126 insertions(+), 10 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 209f2ae..bf3dab8 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -52,6 +52,8 @@
> #include "vaapi-recorder.h"
> #include "presentation_timing-server-protocol.h"
>
> +#include <EGL/eglext.h>
> +
> #ifndef DRM_CAP_TIMESTAMP_MONOTONIC
> #define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
> #endif
> @@ -68,6 +70,10 @@
> #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
> #endif
>
> +#ifndef EGL_PLATFORM_GBM_KHR
> +#define EGL_PLATFORM_GBM_KHR 0x31D7
> +#endif
> +
> static int option_current_mode = 0;
>
> enum output_config {
> @@ -1391,8 +1397,14 @@ drm_compositor_create_gl_renderer(struct drm_compositor *ec)
> {
> EGLint format;
>
> + if (!gl_renderer->supports ||
> + (gl_renderer->supports(&ec->base, "EGL_KHR_platform_gbm") < 0 &&
> + gl_renderer->supports(&ec->base, "EGL_MESA_platform_gbm") < 0)) {
> + return -1;
> + }
> +
> format = ec->format;
> - if (gl_renderer->create(&ec->base, ec->gbm,
> + if (gl_renderer->create(&ec->base, EGL_PLATFORM_GBM_KHR, (void *) ec->gbm,
> gl_renderer->opaque_attribs, &format) < 0) {
> return -1;
> }
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index eedf252..41b43f1 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -925,7 +925,7 @@ fbdev_compositor_create(struct wl_display *display, int *argc, char *argv[],
> goto out_launcher;
> }
>
> - if (gl_renderer->create(&compositor->base, EGL_DEFAULT_DISPLAY,
> + if (gl_renderer->create(&compositor->base, 0, EGL_DEFAULT_DISPLAY,
> gl_renderer->opaque_attribs,
> NULL) < 0) {
Why is fbdev treated differently than the other compositors? I guess no
associated platform extension?
> weston_log("gl_renderer_create failed.\n");
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index 04a2331..850cd4c 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -45,8 +45,14 @@
> #include "fullscreen-shell-client-protocol.h"
> #include "presentation_timing-server-protocol.h"
>
> +#include <EGL/eglext.h>
> +
> #define WINDOW_TITLE "Weston Compositor"
>
> +#ifndef EGL_PLATFORM_WAYLAND_KHR
> +#define EGL_PLATFORM_WAYLAND_KHR 0x31D8
> +#endif
> +
> struct wayland_compositor {
> struct weston_compositor base;
>
> @@ -1961,9 +1967,18 @@ wayland_compositor_create(struct wl_display *display, int use_pixman,
> }
>
> if (!c->use_pixman) {
> - if (gl_renderer->create(&c->base, c->parent.wl_display,
> - gl_renderer->alpha_attribs,
> - NULL) < 0) {
> + if (!gl_renderer->supports ||
> + (gl_renderer->supports(&c->base, "EGL_KHR_platform_wayland") < 0 &&
> + gl_renderer->supports(&c->base, "EGL_EXT_platform_wayland"))) {
> + weston_log("No support for EGL_KHR_platform_wayland; "
> + "falling back to pixman.\n");
> + c->use_pixman = 1;
> +
extra blank line? ^
> + } else if (gl_renderer->create(&c->base,
> + EGL_PLATFORM_WAYLAND_KHR,
> + c->parent.wl_display,
> + gl_renderer->alpha_attribs,
> + NULL) < 0) {
> weston_log("Failed to initialize the GL renderer; "
> "falling back to pixman.\n");
> c->use_pixman = 1;
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index aa1e519..ad7f4c3 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -53,8 +53,14 @@
> #include "../shared/image-loader.h"
> #include "presentation_timing-server-protocol.h"
>
> +#include <EGL/eglext.h>
> +
> #define DEFAULT_AXIS_STEP_DISTANCE wl_fixed_from_int(10)
>
> +#ifndef EGL_PLATFORM_X11_KHR
> +#define EGL_PLATFORM_X11_KHR 0x31D5
> +#endif
> +
> static int option_width;
> static int option_height;
> static int option_scale;
> @@ -1485,7 +1491,13 @@ init_gl_renderer(struct x11_compositor *c)
> if (!gl_renderer)
> return -1;
>
> - ret = gl_renderer->create(&c->base, (EGLNativeDisplayType) c->dpy,
> + if (!gl_renderer->supports ||
> + (gl_renderer->supports(&c->base, "EGL_KHR_platform_x11") < 0 &&
> + gl_renderer->supports(&c->base, "EGL_EXT_platform_x11") < 0)) {
> + return -1;
> + }
> +
> + ret = gl_renderer->create(&c->base, EGL_PLATFORM_X11_KHR, (void *) c->dpy,
> gl_renderer->opaque_attribs, NULL);
>
> return ret;
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index bb46acd..279028b 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -162,6 +162,10 @@ struct gl_renderer {
> struct wl_signal destroy_signal;
> };
>
> +#ifdef EGL_EXT_platform_base
> +static PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display = NULL;
> +#endif
> +
> static inline struct gl_output_state *
> get_output_state(struct weston_output *output)
> {
> @@ -1971,9 +1975,70 @@ static const EGLint gl_renderer_alpha_attribs[] = {
> EGL_NONE
> };
>
> +/* returns negative:
> + * 1. if EGL client extensions aren't supported.
> + * returns positive:
> + * 1. if the EGL version is >= 1.5,
> + * 2. if the supplied EGL client extension is supported,
> + * 3. in the fallback case to using eglGetDisplay after logging a
> + * warning. */
Would prefer to see the above comments in a doxygen format.
> +static int
> +gl_renderer_supports(struct weston_compositor *ec, const char *extension)
> +{
> + static int natively_supported = -1;
> + static const char *extensions = NULL;
> +
> + /* first check if EGL is >= 1.5, where platform_base is guaranteed to
> + * be present */
> +
> + if (natively_supported < 0) {
> + const char *version = eglQueryString(EGL_NO_DISPLAY, EGL_VERSION);
> +
> + /* version will be NULL if the version is < 1.5 */
> + natively_supported = (version != NULL);
> + }
> +
> + if (natively_supported > 0) {
> + weston_log("EGL version is >= 1.5 so assuming platform_base support.\n");
> + return 1;
> + }
> +
> + /* now get the client extensions and the eglGetPlatformDisplayEXT
> + * function pointer */
> +
> + if (!extensions) {
> + extensions = (const char *) eglQueryString(
> + EGL_NO_DISPLAY, EGL_EXTENSIONS);
> +
> + if (!extensions)
> + return -1;
> +
> + log_extensions("EGL client extensions",
> + extensions);
> +
> +#ifdef EGL_EXT_platform_base
> + get_platform_display =
> + (void *) eglGetProcAddress("eglGetPlatformDisplayEXT");
> +#endif
It's a little counter intuitive that a function called
"gl_renderer_supports" actually does initialization. Please document it
in the comments above the function (or do it somewhere else :)
> + }
> +
> + /* check the supplied extension name is present in the (present)
> + * client extensions */
> +
> + if (strstr(extensions, extension))
> + return 1;
> +
> + /* in the end fall back to eglGetDisplay and friends */
> +
> + weston_log("warning: unable to check for %s support; "
> + "could fall back to eglGetDisplay.\n", extension);
I kind of don't like having the logging here. I'd rather see it logged
in the compositor only if we're actually doing the fallback. My EGL
doesn't support the KHR variants and I found it a little confusing to
see warnings logged despite the EXT variant being available.
Another option would be just to pass in the tail of the string (x11,
gbm, wayland) and concatenate it (with EXT, MESA, KHR expecting some not
to exist...) in this function and do all testing here without the
compositor making potentially multiple calls. Whether that's a
reasonable idea or not is up to you. :)
> + return 1;
> +}
> +
> static int
> -gl_renderer_create(struct weston_compositor *ec, EGLNativeDisplayType display,
> - const EGLint *attribs, const EGLint *visual_id)
> +gl_renderer_create(struct weston_compositor *ec, EGLenum platform,
> + void *native_window, const EGLint *attribs,
> + const EGLint *visual_id)
> {
> struct gl_renderer *gr;
> EGLint major, minor;
> @@ -1989,7 +2054,14 @@ gl_renderer_create(struct weston_compositor *ec, EGLNativeDisplayType display,
> gr->base.surface_set_color = gl_renderer_surface_set_color;
> gr->base.destroy = gl_renderer_destroy;
>
> - gr->egl_display = eglGetDisplay(display);
> + if (get_platform_display && platform) {
> + gr->egl_display =
> + get_platform_display(platform, native_window,
> + NULL);
> + } else {
> + gr->egl_display = eglGetDisplay(native_window);
> + }
> +
> if (gr->egl_display == EGL_NO_DISPLAY) {
> weston_log("failed to create display\n");
> goto err_egl;
> @@ -2200,6 +2272,7 @@ WL_EXPORT struct gl_renderer_interface gl_renderer_interface = {
> .opaque_attribs = gl_renderer_opaque_attribs,
> .alpha_attribs = gl_renderer_alpha_attribs,
>
> + .supports = gl_renderer_supports,
> .create = gl_renderer_create,
> .display = gl_renderer_display,
> .output_create = gl_renderer_output_create,
> diff --git a/src/gl-renderer.h b/src/gl-renderer.h
> index 77b1952..96b3cd5 100644
> --- a/src/gl-renderer.h
> +++ b/src/gl-renderer.h
> @@ -50,8 +50,12 @@ struct gl_renderer_interface {
> const EGLint *opaque_attribs;
> const EGLint *alpha_attribs;
>
> + int (*supports)(struct weston_compositor *ec,
> + const char *extension);
> +
> int (*create)(struct weston_compositor *ec,
> - EGLNativeDisplayType display,
> + EGLenum platform,
> + void *native_window,
> const EGLint *attribs,
> const EGLint *visual_id);
>
>
More information about the wayland-devel
mailing list