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

[Frediano Ziglio]

> 
> 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
> 

Well, according to documentation now the default is true so you
could remove first 2 calls which set buffering to true just after
widget creation. Last call is very simple and adding 
G_GNUC_BEGIN_IGNORE_DEPRECATIONS/G_GNUC_END_IGNORE_DEPRECATIONS seems
enough.
Didn't test it.

> > 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.
> 

Then from what I understand from the documentation we should not
call this function. Unless it only applied to these "translucent
drawing".

> Cheers,
> 

Frediano