[Cogl] Feedback of wip/wayland-for-demo

[Neil Roberts]

Hi Jasper,

Thanks for the patches on the wip/wayland-for-demo branch. I've pushed
the first patch to the cogl-1.18 and master branches.

https://git.gnome.org/browse/cogl/commit/?id=af9057d35f331e2c9509958f

I have a couple of minor comments about the second patch:

> diff --git a/cogl/winsys/cogl-winsys-egl-kms.c b/cogl/winsys/cogl-winsys-egl-kms.c
> index 4af9848..41954ef 100644
> --- a/cogl/winsys/cogl-winsys-egl-kms.c
> +++ b/cogl/winsys/cogl-winsys-egl-kms.c
> @@ -285,14 +285,21 @@ _cogl_winsys_renderer_connect (CoglRenderer *renderer,
>     * we're doing here... */
>    g_setenv ("EGL_PLATFORM", "drm", 1);
>  
> -  kms_renderer->fd = open (device_name, O_RDWR);
> -  if (kms_renderer->fd < 0)
> +  if (renderer->kms_fd > 0)

Shouldn't this be '>= 0'? I could imagine something potentially using
fd 0 if it has closed its stdin.

It would be nice if you could make it avoid closing the user's fd if it
hits the close_fd label. I was going to say that it should also avoid
closing the fd when the renderer is destroyed, but it looks that is
missing anyway even if Cogl opens the device so we can leave that to a
separate patch.

Have you been able to test the last patch? When it returns from
flip_all_crtcs it looks like flip->pending would still be zero which
would make it immediately proceed to delete the framebuffer that it just
set to scan out from. Won't that break?

I think we would also need to make it queue the swap_notify_idle because
Cogl is meant to have the invariant that whenever you call
cogl_onscreen_swap_buffers you will always get a swap notification.

Regards,
- Neil