Re: [Spice-devel] [PATCH spice-gtk] Do not unrealize disabled egl

2017-06-19 Thread Marc-André Lureau
Hi

- Original Message -
> On Mon, 2017-06-19 at 06:00 -0400, Marc-André Lureau wrote:
> > 
> > - Original Message -
> > > 
> > > 
> > > - Original Message -
> > > > Avoids a critical to be logged when closing remote-viewer:
> > > >  "gl_make_current: assertion 'd->egl.context_ready' failed"
> > > 
> > > ack, thanks
> > 
> > actually, why not check egl.context_ready?
> 
> In which cases you want to test the precondition added in
> 422572c37793bf6141bc58d441bcda37090c74f3 ?

Yes, and as Victor suggested, you can do the check at the caller location.

> 
> >  (you might be switch back to 2d, and still have egl context to
> > clean up)
> 
> Yes. (btw I am not sure the switch's worked - eg in rebooting)

It used to work. It shouldn't be hard to check, with qemu <2.8 release iirc.

> 
> Pavel
> > 
> > > 
> > > > ---
> > > >  src/spice-widget-egl.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
> > > > index 7c21113..5951ae6 100644
> > > > --- a/src/spice-widget-egl.c
> > > > +++ b/src/spice-widget-egl.c
> > > > @@ -374,6 +374,9 @@ void
> > > > spice_egl_unrealize_display(SpiceDisplay *display)
> > > >  
> > > >  DISPLAY_DEBUG(display, "egl unrealize %p", d->egl.surface);
> > > >  
> > > > +if (!d->egl.enabled)
> > > > +return;
> > > > +
> > > >  if (!gl_make_current(display, NULL))
> > > >  return;
> > > >  
> > > > --
> > > > 2.13.0
> > > > 
> > > > ___
> > > > Spice-devel mailing list
> > > > Spice-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > 
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] Do not unrealize disabled egl

2017-06-19 Thread Pavel Grunt
On Mon, 2017-06-19 at 06:00 -0400, Marc-André Lureau wrote:
> 
> - Original Message -
> > 
> > 
> > - Original Message -
> > > Avoids a critical to be logged when closing remote-viewer:
> > >  "gl_make_current: assertion 'd->egl.context_ready' failed"
> > 
> > ack, thanks
> 
> actually, why not check egl.context_ready?

In which cases you want to test the precondition added in
422572c37793bf6141bc58d441bcda37090c74f3 ?

>  (you might be switch back to 2d, and still have egl context to
> clean up)

Yes. (btw I am not sure the switch's worked - eg in rebooting)

Pavel
> 
> > 
> > > ---
> > >  src/spice-widget-egl.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
> > > index 7c21113..5951ae6 100644
> > > --- a/src/spice-widget-egl.c
> > > +++ b/src/spice-widget-egl.c
> > > @@ -374,6 +374,9 @@ void
> > > spice_egl_unrealize_display(SpiceDisplay *display)
> > >  
> > >  DISPLAY_DEBUG(display, "egl unrealize %p", d->egl.surface);
> > >  
> > > +if (!d->egl.enabled)
> > > +return;
> > > +
> > >  if (!gl_make_current(display, NULL))
> > >  return;
> > >  
> > > --
> > > 2.13.0
> > > 
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] Do not unrealize disabled egl

2017-06-19 Thread Victor Toso
Hi,

On Mon, Jun 19, 2017 at 11:50:45AM +0200, Pavel Grunt wrote:
> Avoids a critical to be logged when closing remote-viewer:
>  "gl_make_current: assertion 'd->egl.context_ready' failed"
> ---
>  src/spice-widget-egl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
> index 7c21113..5951ae6 100644
> --- a/src/spice-widget-egl.c
> +++ b/src/spice-widget-egl.c
> @@ -374,6 +374,9 @@ void spice_egl_unrealize_display(SpiceDisplay *display)
>
>  DISPLAY_DEBUG(display, "egl unrealize %p", d->egl.surface);
>
> +if (!d->egl.enabled)
> +return;
> +

Although your patch is fine, spice_egl_unrealize_display() should not be
called, I think. Maybe use egl_enabled(d) before
spice_egl_unrealize_display() on spice-widget as well?

Cheers,


>  if (!gl_make_current(display, NULL))
>  return;
>  
> -- 
> 2.13.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] Do not unrealize disabled egl

2017-06-19 Thread Marc-André Lureau


- Original Message -
> 
> 
> - Original Message -
> > Avoids a critical to be logged when closing remote-viewer:
> >  "gl_make_current: assertion 'd->egl.context_ready' failed"
> 
> ack, thanks

actually, why not check egl.context_ready? (you might be switch back to 2d, and 
still have egl context to clean up)

> 
> > ---
> >  src/spice-widget-egl.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
> > index 7c21113..5951ae6 100644
> > --- a/src/spice-widget-egl.c
> > +++ b/src/spice-widget-egl.c
> > @@ -374,6 +374,9 @@ void spice_egl_unrealize_display(SpiceDisplay *display)
> >  
> >  DISPLAY_DEBUG(display, "egl unrealize %p", d->egl.surface);
> >  
> > +if (!d->egl.enabled)
> > +return;
> > +
> >  if (!gl_make_current(display, NULL))
> >  return;
> >  
> > --
> > 2.13.0
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] Do not unrealize disabled egl

2017-06-19 Thread Marc-André Lureau


- Original Message -
> Avoids a critical to be logged when closing remote-viewer:
>  "gl_make_current: assertion 'd->egl.context_ready' failed"

ack, thanks

> ---
>  src/spice-widget-egl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
> index 7c21113..5951ae6 100644
> --- a/src/spice-widget-egl.c
> +++ b/src/spice-widget-egl.c
> @@ -374,6 +374,9 @@ void spice_egl_unrealize_display(SpiceDisplay *display)
>  
>  DISPLAY_DEBUG(display, "egl unrealize %p", d->egl.surface);
>  
> +if (!d->egl.enabled)
> +return;
> +
>  if (!gl_make_current(display, NULL))
>  return;
>  
> --
> 2.13.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk] Do not unrealize disabled egl

2017-06-19 Thread Pavel Grunt
Avoids a critical to be logged when closing remote-viewer:
 "gl_make_current: assertion 'd->egl.context_ready' failed"
---
 src/spice-widget-egl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
index 7c21113..5951ae6 100644
--- a/src/spice-widget-egl.c
+++ b/src/spice-widget-egl.c
@@ -374,6 +374,9 @@ void spice_egl_unrealize_display(SpiceDisplay *display)
 
 DISPLAY_DEBUG(display, "egl unrealize %p", d->egl.surface);
 
+if (!d->egl.enabled)
+return;
+
 if (!gl_make_current(display, NULL))
 return;
 
-- 
2.13.0

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