[Spice-devel] [spice-gtk 3/4] gtk-deprecated: silence warn on gtk_widget_set_double_buffered()

[Victor Toso]

Hi,

On Tue, Jul 24, 2018 at 08:56:53AM -0400, Frediano Ziglio wrote:
> > 
> > From: Victor Toso <me at victortoso.com>
> > 
> > To quote documentation:
> >  | gtk_widget_set_double_buffered has been deprecated since version
> >  | 3.14 and should not be used in newly-written code.
> >  | This function does not work under non-X11 backends or with
> >  | non-native windows. It should not be used in newly written code.
> > 
> > So be sure to only call this function on X11 backend and silence
> > warnings with G_GNUC_BEGIN/END_IGNORE_DEPRECATIONS
> > 
> > Warnings fixed:
> >  | spice-widget.c: In function ‘spice_display_init’:
> >  | spice-widget.c:643:5: warning: ‘gtk_widget_set_double_buffered’ is
> >  | deprecated
> >  |
> >  |     gtk_widget_set_double_buffered(area, true);
> >  |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  |
> >  | spice-widget.c:661:5: warning: ‘gtk_widget_set_double_buffered’ is
> >  | deprecated
> >  |
> >  |     gtk_widget_set_double_buffered(area, true);
> >  |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  |
> >  | spice-widget.c: In function ‘set_egl_enabled’:
> >  | spice-widget.c:1290:9: warning: ‘gtk_widget_set_double_buffered’ is
> >  | deprecated
> >  |     gtk_widget_set_double_buffered(GTK_WIDGET(area), !enabled);
> >  |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/spice-widget.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > index 6ad0865..dc8ee40 100644
> > --- a/src/spice-widget.c
> > +++ b/src/spice-widget.c
> > @@ -640,7 +640,13 @@ static void spice_display_init(SpiceDisplay *display)
> >                       "signal::realize", drawing_area_realize, display,
> >                       NULL);
> >      gtk_stack_add_named(d->stack, area, "draw-area");
> > -    gtk_widget_set_double_buffered(area, true);
> > +#ifdef GDK_WINDOWING_X11
> > +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
> > +        G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> > +        gtk_widget_set_double_buffered(area, true);
> > +        G_GNUC_END_IGNORE_DEPRECATIONS
> > +    }
> > +#endif
> >      gtk_stack_set_visible_child(d->stack, area);
> >  
> >  #if HAVE_EGL
> > @@ -658,7 +664,13 @@ G_GNUC_END_IGNORE_DEPRECATIONS
> >  #endif
> >      area = gtk_drawing_area_new();
> >      gtk_stack_add_named(d->stack, area, "gst-area");
> > -    gtk_widget_set_double_buffered(area, true);
> > +#ifdef GDK_WINDOWING_X11
> > +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
> > +        G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> > +        gtk_widget_set_double_buffered(area, true);
> > +        G_GNUC_END_IGNORE_DEPRECATIONS
> > +    }
> > +#endif
> >  
> >      gtk_widget_show_all(widget);
> >  
> > @@ -1281,7 +1293,9 @@ static void set_egl_enabled(SpiceDisplay *display, bool
> > enabled)
> >           * only way I found to prevent glitches when the window is
> >           * resized. */
> >          GtkWidget *area = gtk_stack_get_child_by_name(d->stack,
> >          "draw-area");
> > +        G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> >          gtk_widget_set_double_buffered(GTK_WIDGET(area), !enabled);
> > +        G_GNUC_END_IGNORE_DEPRECATIONS
> >      } else
> >  #endif
> >      {
> 
> First time I saw this patch I though "quite ugly". Second time
> did not improve much.
> Maybe this function should be wrapped in a compatibility
> function, something kind of suggested at
> https://lists.freedesktop.org/archives/spice-devel/2018-July/044590.html?
> In SPICE server we already have a glib-compat.h header
> (slightly different usage).

Ugly indeed, I didn't care much at first sight but you are right.

I don't mind wrapping this in an utility function/macro but don't
exactly agree to use it in glib-compat as that's usually referred
to glib functions that were removed or too-new but we want to use
in our code (e.g basically a copy-paste from glib).

A local macro like SET_DOUBLE_BUFFER_ON_X11(widget, enabled)
should work well enough

> There are also some scary comment on this function, see
> https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-set-double-buffered, specifically
> "In 3.10 GTK and GDK have been restructured for translucent
> drawing. Since then expose events for double-buffered widgets
> are culled into a single event to the toplevel GDK window. If
> you now unset double buffering, you will cause a separate
> rendering pass for every widget. This will likely cause
> rendering problems - in particular related to stacking - and
> usually increases rendering times significantly."

Do you think I should add it in commit log?

> Which ones are the double-buffered widgets?

Just the display? :) The Drawing area is double buffered.

Cheers,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180724/1f2f8439/attachment.sig>