[PATCH 3/7] gl-renderer: Make dummy surface current after all outputs are gone

[Pekka Paalanen]

On Fri, 29 Jul 2016 13:26:24 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:

> When all outputs are gone, there are no current read/write
> surfaces associated with a context. This makes the previously
> created dummy surface current until an output gets attached
> to avoid any potential crashes.
> 
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>

Hi Armin,

this patch seems to have several unnecessary things, see below.

> ---
>  libweston/gl-renderer.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index be6b11e..56eb344 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -167,6 +167,7 @@ struct gl_surface_state {
>  
>  struct gl_renderer {
>  	struct weston_renderer base;
> +	struct weston_compositor *compositor;

This member is not really necessary.

>  	int fragment_shader_debug;
>  	int fan_debug;
>  	struct weston_binding *fragment_binding;
> @@ -216,6 +217,9 @@ struct gl_renderer {
>  	struct gl_shader *current_shader;
>  
>  	struct wl_signal destroy_signal;
> +
> +	struct wl_signal output_destroy_signal;

output_destroy_signal seems unused. Does it have any purpose?

> +	struct wl_listener output_destroy_listener;
>  };
>  
>  static PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display = NULL;
> @@ -2642,6 +2646,8 @@ gl_renderer_destroy(struct weston_compositor *ec)
>  	eglTerminate(gr->egl_display);
>  	eglReleaseThread();
>  
> +	wl_list_remove(&gr->output_destroy_listener.link);
> +
>  	wl_array_release(&gr->vertices);
>  	wl_array_release(&gr->vtxcnt);
>  
> @@ -2827,6 +2833,21 @@ platform_to_extension(EGLenum platform)
>  	}
>  }
>  
> +static void
> +output_handle_destroy(struct wl_listener *listener, void *data)
> +{
> +	struct gl_renderer *gr;
> +	struct weston_compositor *ec;
> +
> +	gr = container_of(listener, struct gl_renderer,
> +			  output_destroy_listener);
> +	ec = gr->compositor;

Rather than this, you could do:
struct weston_output *output = data;
ec = output->compositor;

> +
> +	if (wl_list_empty(&ec->output_list))
> +		eglMakeCurrent(gr->egl_display, gr->dummy_surface,
> +			       gr->dummy_surface, gr->egl_context);
> +}
> +
>  static int
>  gl_renderer_create_pbuffer_surface(struct gl_renderer *gr) {
>  	EGLConfig pbuffer_config;
> @@ -2884,6 +2905,7 @@ gl_renderer_create(struct weston_compositor *ec, EGLenum platform,
>  	if (gr == NULL)
>  		return -1;
>  
> +	gr->compositor = ec;

This would be unneeded.

>  	gr->base.read_pixels = gl_renderer_read_pixels;
>  	gr->base.repaint_output = gl_renderer_repaint_output;
>  	gr->base.flush_damage = gl_renderer_flush_damage;
> @@ -2962,6 +2984,7 @@ gl_renderer_create(struct weston_compositor *ec, EGLenum platform,
>  	wl_display_add_shm_format(ec->wl_display, WL_SHM_FORMAT_RGB565);
>  
>  	wl_signal_init(&gr->destroy_signal);
> +	wl_signal_init(&gr->output_destroy_signal);

This is unused.

>  
>  	if (gl_renderer_setup(ec, gr->dummy_surface) < 0) {
>  		if (gr->dummy_surface != EGL_NO_SURFACE)
> @@ -3136,6 +3159,10 @@ gl_renderer_setup(struct weston_compositor *ec, EGLSurface egl_surface)
>  						    fan_debug_repaint_binding,
>  						    ec);
>  
> +	gr->output_destroy_listener.notify = output_handle_destroy;
> +	wl_signal_add(&ec->output_destroyed_signal,
> +		      &gr->output_destroy_listener);
> +
>  	weston_log("GL ES 2 renderer features:\n");
>  	weston_log_continue(STAMP_SPACE "read-back format: %s\n",
>  		ec->read_format == PIXMAN_a8r8g8b8 ? "BGRA" : "RGBA");

Other than that, this patch looks good.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160805/21584d59/attachment.sig>