Hi Armin, several minor nitpicks on wording here, but the code looks good.
On Thu, 23 Jun 2016 11:59:35 +0200
Armin Krezović <krezovic.ar...@gmail.com> wrote:
> Currently, the gl-renderer setup is being done on per-output
> basis. This isn't desirable when trying to make weston run
> with zero outputs.
On "first output" rather than "per-output" basis.
>
> When there are no outputs present, there is no surface available
> to attach an EGLContext to with eglMakeCurrent, which makes
> any EGL command fail.
GL command. Only certain EGL calls use the current context, while all
GL calls use it. Specifically, image_target_texture_2d is the GL
function glEGLImageTargetTexture2DOES which IIRC was failing without
the renderer setup.
>
> The problem is solved by using EGL_KHR_surfaceless_context to
> bind an EGLContext to EGL_NO_SURFACE, or if that is
> unavailable, creating a dummy PbufferSurface and binding an
> EGLContext to it, so EGL gets set up properly.
>
> v2:
>
> - Move PbufferSurface creation into its own function
> - Introduce a new EGLConfig with EGL_PBUFFER_BIT set
> and use it to create a PbufferSurface
> - Make PbufferSurface attributes definition static
> - Check for return of gl_renderer_setup and terminate
> in case it fails
> - Remove redundant gl_renderer_setup call from
> gl_renderer_output_create
> - Only destroy the dummy surface if it is valid
>
> Signed-off-by: Armin Krezović <krezovic.ar...@gmail.com>
> ---
> src/gl-renderer.c | 71
> ++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 65 insertions(+), 6 deletions(-)
>
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index 23c0cd7..28c0b50 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -175,6 +175,8 @@ struct gl_renderer {
> EGLContext egl_context;
> EGLConfig egl_config;
>
> + EGLSurface dummy_surface;
> +
> struct wl_array vertices;
> struct wl_array vtxcnt;
>
> @@ -201,6 +203,8 @@ struct gl_renderer {
>
> int has_configless_context;
>
> + int has_surfaceless_context;
> +
> int has_dmabuf_import;
> struct wl_list dmabuf_images;
>
> @@ -2597,12 +2601,6 @@ gl_renderer_output_create(struct weston_output *output,
> return -1;
> }
>
> - if (gr->egl_context == NULL)
> - if (gl_renderer_setup(ec, go->egl_surface) < 0) {
> - free(go);
> - return -1;
> - }
> -
> for (i = 0; i < BUFFER_DAMAGE_COUNT; i++)
> pixman_region32_init(&go->buffer_damage[i]);
>
> @@ -2654,6 +2652,9 @@ gl_renderer_destroy(struct weston_compositor *ec)
> wl_list_for_each_safe(image, next, &gr->dmabuf_images, link)
> dmabuf_image_destroy(image);
>
> + if (gr->dummy_surface != EGL_NO_SURFACE)
> + eglDestroySurface(gr->egl_display, gr->dummy_surface);
> +
> eglTerminate(gr->egl_display);
> eglReleaseThread();
>
> @@ -2765,6 +2766,9 @@ gl_renderer_setup_egl_extensions(struct
> weston_compositor *ec)
> gr->has_configless_context = 1;
> #endif
>
> + if (check_extension(extensions, "EGL_KHR_surfaceless_context"))
> + gr->has_surfaceless_context = 1;
> +
> #ifdef EGL_EXT_image_dma_buf_import
> if (check_extension(extensions, "EGL_EXT_image_dma_buf_import"))
> gr->has_dmabuf_import = 1;
> @@ -2795,6 +2799,7 @@ static const EGLint gl_renderer_alpha_attribs[] = {
> EGL_NONE
> };
>
> +
> /** Checks whether a platform EGL client extension is supported
> *
> * \param ec The weston compositor
> @@ -2873,6 +2878,43 @@ platform_to_extension(EGLenum platform)
> }
>
> static int
> +gl_renderer_create_pbuffer_surface(struct gl_renderer *gr) {
Style-wise, this function could just take a EGLDisplay as an argument,
and return the EGLSurface. That way the setting of dummy_surface would
be in the caller, which would make it easier to read.
> + EGLConfig pbuffer_config;
> +
> + static const EGLint pbuffer_config_attribs[] = {
> + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT,
> + EGL_RED_SIZE, 1,
> + EGL_GREEN_SIZE, 1,
> + EGL_BLUE_SIZE, 1,
> + EGL_ALPHA_SIZE, 0,
> + EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
> + EGL_NONE
> + };
> +
> + static const EGLint pbuffer_attribs[] = {
> + EGL_WIDTH, 10,
> + EGL_HEIGHT, 10,
> + EGL_NONE
> + };
> +
> + if (egl_choose_config(gr, pbuffer_config_attribs, NULL, 0,
> &pbuffer_config) < 0) {
> + weston_log("failed to choose EGL config for PbufferSurface");
> + return -1;
> + }
> +
> + gr->dummy_surface = eglCreatePbufferSurface(gr->egl_display,
> + pbuffer_config,
> + pbuffer_attribs);
> +
> + if (gr->dummy_surface == EGL_NO_SURFACE) {
> + weston_log("failed to create PbufferSurface\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> gl_renderer_create(struct weston_compositor *ec, EGLenum platform,
> void *native_window, const EGLint *attribs,
> const EGLint *visual_id, int n_ids)
> @@ -2956,10 +2998,27 @@ gl_renderer_create(struct weston_compositor *ec,
> EGLenum platform,
> if (gr->has_dmabuf_import)
> gr->base.import_dmabuf = gl_renderer_import_dmabuf;
>
> + if (gr->has_surfaceless_context) {
> + weston_log("EGL_KHR_surfaceless_context available\n");
> + gr->dummy_surface = EGL_NO_SURFACE;
> + } else {
> + weston_log("EGL_KHR_surfaceless_context unavailable. "
> + "Trying PbufferSurface\n");
> +
> + if (gl_renderer_create_pbuffer_surface(gr) < 0)
> + goto fail_with_error;
> + }
> +
> wl_display_add_shm_format(ec->wl_display, WL_SHM_FORMAT_RGB565);
>
> wl_signal_init(&gr->destroy_signal);
>
> + if (gl_renderer_setup(ec, gr->dummy_surface) < 0) {
> + if (gr->dummy_surface != EGL_NO_SURFACE)
> + eglDestroySurface(gr->egl_display, gr->dummy_surface);
To follow the error clean-up style, you'd have a new goto label
'fail_with_dummy' or something, put the eglDestroySurface() call after
that, and then let it fall through to fail_with_error etc.
But, not a big deal.
> + goto fail_with_error;
> + }
> +
> return 0;
>
> fail_with_error:
I filed a Mesa bug about the "libEGL warning: FIXME: egl/x11 doesn't
support front buffer rendering." because I think it's wrong:
https://bugs.freedesktop.org/show_bug.cgi?id=96694
I don't think it should hurt anything though, so pushed:
ea0316a..28d240f master -> master
The rest of the series is for another day.
Thanks,
pq
pgpWbCseUjed0.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
