Re: [Spice-devel] [PATCH spice-gtk 5/5] egl: fix delayed widget realize

2016-05-25 Thread Marc-André Lureau 
Hi

On Wed, May 25, 2016 at 8:42 AM, Pavel Grunt  wrote:
> On Tue, 2016-05-24 at 21:31 +0200, Marc-André Lureau wrote:
>> When the display is not yet realized, spice_display_widget_gl_scanout()
>> will fail because the egl context is not ready. The display is never
>> marked ready because the egl.image (and egl.scanout) is not set, and
>> some clients, such as virt-viewer will not realize the widget until the
>> display is ready.
>
> would make a sense to "update ready" when egl.image changes?

egl.image can only be set when egl context is ready, ie when the
widget is realized. However, some clients, such as virt-viewer, won't
realize it until the widget is ready. It's chicken egg issue, thus it
can't check for egl.image, we need lower-level checks such as done in
here.

>> Deal with gl scanout updates when the widget is not yet realized, and
>> mark the display as ready when egl is enabled (when last display draw
>> signal is from gl).
> ok
>>
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  src/spice-widget.c | 19 +++
>>  1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/spice-widget.c b/src/spice-widget.c
>> index 1c9ac2d..2b1de68 100644
>> --- a/src/spice-widget.c
>> +++ b/src/spice-widget.c
>> @@ -238,7 +238,7 @@ static void update_ready(SpiceDisplay *display)
>>
>>  if (d->monitor_ready) {
>>  #ifndef G_OS_WIN32
>> -ready = d->egl.enabled ? d->egl.image != NULL : d->mark != 0;
>> +ready = d->egl.enabled || d->mark != 0;
>>  #else
>>  ready = d->mark != 0;
>>  #endif
>> @@ -2296,9 +2296,12 @@ static void update_area(SpiceDisplay *display,
>>
>>  #ifndef G_OS_WIN32
>>  if (d->egl.enabled) {
>> +const SpiceGlScanout *so =
>> +spice_display_get_gl_scanout(SPICE_DISPLAY_CHANNEL(d->display));
>> +g_return_if_fail(so != NULL);
>>  primary = (GdkRectangle) {
>> -.width = d->egl.scanout.width,
>> -.height = d->egl.scanout.height
>> +.width = so->width,
>> +.height = so->height
>>  };
>>  } else
>>  #endif
>> @@ -2621,15 +2624,15 @@ void spice_display_widget_gl_scanout(SpiceDisplay
>> *display)
>>  SPICE_DEBUG("%s: got scanout",  __FUNCTION__);
>>  set_egl_enabled(display, true);
>>
>> -g_return_if_fail(d->egl.context_ready);
>> -
>>  scanout = spice_display_get_gl_scanout(SPICE_DISPLAY_CHANNEL(d-
>> >display));
>>  /* should only be called when the display has a scanout */
>>  g_return_if_fail(scanout != NULL);
> ^^^
> these lines and varible definitions should be moved ...
>>
>> -if (!spice_egl_update_scanout(display, scanout, )) {
>> -g_critical("update scanout failed: %s", err->message);
>> -g_clear_error();
>> +if (d->egl.context_ready) {
> ... here

ok

>> +if (!spice_egl_update_scanout(display, scanout, )) {
>> +g_critical("update scanout failed: %s", err->message);
>> +g_clear_error();
>> +}
>>  }
>>  }
>>



-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk 5/5] egl: fix delayed widget realize

2016-05-25 Thread Pavel Grunt 
On Tue, 2016-05-24 at 21:31 +0200, Marc-André Lureau wrote:
> When the display is not yet realized, spice_display_widget_gl_scanout()
> will fail because the egl context is not ready. The display is never
> marked ready because the egl.image (and egl.scanout) is not set, and
> some clients, such as virt-viewer will not realize the widget until the
> display is ready.

would make a sense to "update ready" when egl.image changes?
> 
> Deal with gl scanout updates when the widget is not yet realized, and
> mark the display as ready when egl is enabled (when last display draw
> signal is from gl).
ok
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  src/spice-widget.c | 19 +++
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 1c9ac2d..2b1de68 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -238,7 +238,7 @@ static void update_ready(SpiceDisplay *display)
>  
>  if (d->monitor_ready) {
>  #ifndef G_OS_WIN32
> -ready = d->egl.enabled ? d->egl.image != NULL : d->mark != 0;
> +ready = d->egl.enabled || d->mark != 0;
>  #else
>  ready = d->mark != 0;
>  #endif
> @@ -2296,9 +2296,12 @@ static void update_area(SpiceDisplay *display,
>  
>  #ifndef G_OS_WIN32
>  if (d->egl.enabled) {
> +const SpiceGlScanout *so =
> +spice_display_get_gl_scanout(SPICE_DISPLAY_CHANNEL(d->display));
> +g_return_if_fail(so != NULL);
>  primary = (GdkRectangle) {
> -.width = d->egl.scanout.width,
> -.height = d->egl.scanout.height
> +.width = so->width,
> +.height = so->height
>  };
>  } else
>  #endif
> @@ -2621,15 +2624,15 @@ void spice_display_widget_gl_scanout(SpiceDisplay
> *display)
>  SPICE_DEBUG("%s: got scanout",  __FUNCTION__);
>  set_egl_enabled(display, true);
>  
> -g_return_if_fail(d->egl.context_ready);
> -
>  scanout = spice_display_get_gl_scanout(SPICE_DISPLAY_CHANNEL(d-
> >display));
>  /* should only be called when the display has a scanout */
>  g_return_if_fail(scanout != NULL);
^^^
these lines and varible definitions should be moved ...
>  
> -if (!spice_egl_update_scanout(display, scanout, )) {
> -g_critical("update scanout failed: %s", err->message);
> -g_clear_error();
> +if (d->egl.context_ready) {
... here
> +if (!spice_egl_update_scanout(display, scanout, )) {
> +g_critical("update scanout failed: %s", err->message);
> +g_clear_error();
> +}
>  }
>  }
>  
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel