On 27.06.2016 15:08, Pekka Paalanen wrote:
> 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
> Hi, thanks for merging most of the patches. Do you want me to send another patch which fixes minor issues you've outlined? Cheers, Armin
signature.asc
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
