[virt-tools-list] [virt-viewer] Use GResource for loading ui files

Jonathon Jongsma jjongsma at redhat.com
Tue Mar 1 21:54:11 UTC 2016


On Fri, 2016-02-26 at 23:38 +0100, Fabiano Fidêncio wrote:
> Let's take advantage of GResource for loading ui files in a better and
> cleaner way than virt_viewer_util_load_ui() was doing.
> It also brings the benefit, at least for developers, of being able to
> test ui changes without having to "make install" virt-viewer.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  src/Makefile.am               | 25 ++++++++++++++++++----
>  src/virt-viewer-about.xml     |  1 -
>  src/virt-viewer-app.c         | 10 +++++++++
>  src/virt-viewer-util.c        | 50 ++++++------------------------------------
> -
>  src/virt-viewer-window.c      | 20 +++++++++++++----
>  src/virt-viewer.gresource.xml | 19 ++++++++++++++++
>  6 files changed, 73 insertions(+), 52 deletions(-)
>  create mode 100644 src/virt-viewer.gresource.xml
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index f42a7bf..ce31f08 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -18,8 +18,10 @@ builderxml_DATA =				\
>  
>  EXTRA_DIST =					\
>  	$(builderxml_DATA)			\
> +	$(resource_files)			\
>  	virt-viewer-enums.c.etemplate		\
>  	virt-viewer-enums.h.etemplate		\
> +	virt-viewer.gresource.xml		\
>  	$(NULL)
>  
>  ENUMS_FILES =					\
> @@ -27,15 +29,32 @@ ENUMS_FILES =					\
>  	$(NULL)
>  
>  BUILT_SOURCES =					\
> +	virt-viewer-resources.h			\
> +	virt-viewer-resources.c			\
>  	virt-viewer-enums.h			\
>  	virt-viewer-enums.c			\
>  	$(NULL)
>  
> -$(BUILT_SOURCES): %: %.etemplate $(ENUMS_FILES)
> -	$(AM_V_GEN)$(GLIB_MKENUMS) --template $^ | \
> +resource_files = $(shell glib-compile-resources --sourcedir=$(srcdir) -
> -generate-dependencies $(srcdir)/virt-viewer.gresource.xml)
> +virt-viewer-resources.c: $(srcdir)/virt-viewer.gresource.xml
> $(resource_files)
> +	glib-compile-resources --target=$@ --sourcedir=$(srcdir) --generate
> -source --c-name virt_viewer $(srcdir)/virt-viewer.gresource.xml
> +virt-viewer-resources.h: $(srcdir)/virt-viewer.gresource.xml
> $(resource_files)
> +	glib-compile-resources --target=$@ --sourcedir=$(srcdir) --generate
> -header --c-name virt_viewer $(srcdir)/virt-viewer.gresource.xml
> +
> +virt-viewer-enums.h: %: virt-viewer-enums.h.etemplate $(ENUMS_FILES)
> +	       $(AM_V_GEN)$(GLIB_MKENUMS) --template $^ | \
> +            sed -e 's/VIRT_TYPE_VIEWER/VIRT_VIEWER_TYPE/' \
> +                -e 's,#include "$(srcdir)/,#include ",' > $@
> +
> +virt-viewer-enums.c: %: virt-viewer-enums.c.etemplate $(ENUMS_FILES)
> +	       $(AM_V_GEN)$(GLIB_MKENUMS) --template $^ | \
>              sed -e 's/VIRT_TYPE_VIEWER/VIRT_VIEWER_TYPE/' \
>                  -e 's,#include "$(srcdir)/,#include ",' > $@


This duplication seems a shame. Is there a reason we can't continue using a
pattern rule?

>  
> +CLEANFILES = \
> +	$(BUILT_SOURCES)				\
> +	$(NULL)
> +
>  libvirt_viewer_la_SOURCES =					\
>  	$(BUILT_SOURCES)				\
>  	virt-viewer-util.h				\
> @@ -185,8 +204,6 @@ if OS_WIN32
>  remote_viewer_LDFLAGS += -Wl,--subsystem,windows
>  endif
>  
> -AM_CPPFLAGS = -DPACKAGE_DATADIR=\""$(pkgdatadir)"\"
> -
>  VIRT_VIEWER_RES = virt-viewer.rc virt-viewer.manifest
>  ICONDIR = $(top_builddir)/icons
>  MANIFESTDIR = $(srcdir)
> diff --git a/src/virt-viewer-about.xml b/src/virt-viewer-about.xml
> index 16db3c2..287c68d 100644
> --- a/src/virt-viewer-about.xml
> +++ b/src/virt-viewer-about.xml
> @@ -36,7 +36,6 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 
>  02111-1307  USA
>  Marc-André Lureau
>  </property>
>      <property name="translator_credits" translatable="yes">The Fedora
> Translation Team</property>
> -    <property name="logo_icon_name">virt-viewer</property>
>      <signal name="delete-event" handler="virt_viewer_app_about_delete"
> swapped="no"/>
>      <signal name="response" handler="virt_viewer_app_about_close"
> swapped="no"/>
>      <child internal-child="vbox">
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 3fa80a5..68fe533 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -52,6 +52,7 @@
>  #endif
>  
>  #include "virt-viewer-app.h"
> +#include "virt-viewer-resources.h"
>  #include "virt-viewer-auth.h"
>  #include "virt-viewer-window.h"
>  #include "virt-viewer-session.h"
> @@ -115,6 +116,7 @@ struct _VirtViewerAppPrivate {
>      gchar *clipboard;
>      GtkWidget *preferences;
>      GtkFileChooser *preferences_shared_folder;
> +    GResource *resource;
>      gboolean direct;
>      gboolean verbose;
>      gboolean enable_accel;
> @@ -1717,6 +1719,11 @@ virt_viewer_app_dispose (GObject *object)
>          g_hash_table_unref(tmp);
>      }
>  
> +    if (priv->resource != NULL) {
> +        g_resources_unregister(priv->resource);
> +        priv->resource = NULL;
> +    }
> +

I'm quite sure that this is not actually necessary, since you did not pass -
-manual-register to the glib-compile-resources script. The generated code makes
use of 'constructor' functions to register these static resources automatically.

>      g_clear_object(&priv->session);
>      g_free(priv->title);
>      priv->title = NULL;
> @@ -1863,6 +1870,9 @@ virt_viewer_app_on_application_startup(GApplication
> *app)
>  
>      G_APPLICATION_CLASS(virt_viewer_app_parent_class)->startup(app);
>  
> +    self->priv->resource = virt_viewer_get_resource();
> +    g_resources_register(self->priv->resource);
> +

same as above

>      virt_viewer_app_set_debug(opt_debug);
>      virt_viewer_app_set_fullscreen(self, opt_fullscreen);
>  
> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> index 76b61a1..ce81e06 100644
> --- a/src/virt-viewer-util.c
> +++ b/src/virt-viewer-util.c
> @@ -41,6 +41,8 @@
>  
>  #include "virt-viewer-util.h"
>  
> +#define VIRT_VIEWER_RESOURCE_PREFIX "/org/virt-manager/virt-viewer"
> +
>  GQuark
>  virt_viewer_error_quark(void)
>  {
> @@ -49,53 +51,15 @@ virt_viewer_error_quark(void)
>  
>  GtkBuilder *virt_viewer_util_load_ui(const char *name)
>  {
> -    struct stat sb;
>      GtkBuilder *builder;
> -    GError *error = NULL;
> -
> -    builder = gtk_builder_new();
> -    if (stat(name, &sb) >= 0) {
> -        gtk_builder_add_from_file(builder, name, &error);
> -    } else {
> -        gchar *path = g_build_filename(PACKAGE_DATADIR, "ui", name, NULL);
> -        gboolean success = (gtk_builder_add_from_file(builder, path, &error)
> != 0);
> -        if (error) {
> -            if (!(error->domain == G_FILE_ERROR && error->code ==
> G_FILE_ERROR_NOENT))
> -                g_warning("Failed to add ui file '%s': %s", path, error
> ->message);
> -            g_clear_error(&error);
> -        }
> -        g_free(path);
> -
> -        if (!success) {
> -            const gchar * const * dirs = g_get_system_data_dirs();
> -            g_return_val_if_fail(dirs != NULL, NULL);
> -
> -            while (dirs[0] != NULL) {
> -                path = g_build_filename(dirs[0], PACKAGE, "ui", name, NULL);
> -                if (gtk_builder_add_from_file(builder, path, NULL) != 0) {
> -                    g_free(path);
> -                    break;
> -                }
> -                g_free(path);
> -                dirs++;
> -            }
> -            if (dirs[0] == NULL)
> -                goto failed;
> -        }
> -    }
> +    gchar *resource = g_strdup_printf("%s/%s",
> +                                      VIRT_VIEWER_RESOURCE_PREFIX,
> +                                      name);
>  
> -    if (error) {
> -        g_error("Cannot load UI description %s: %s", name,
> -                error->message);
> -        g_clear_error(&error);
> -        goto failed;
> -    }
> +    builder = gtk_builder_new_from_resource(resource);
>  
> +    g_free(resource);
>      return builder;
> - failed:
> -    g_error("failed to find UI description file");
> -    g_object_unref(builder);
> -    return NULL;
>  }
>  
>  int
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index 8ce34ca..2c46c06 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -1030,11 +1030,24 @@ G_MODULE_EXPORT void
>  virt_viewer_window_menu_help_about(GtkWidget *menu G_GNUC_UNUSED,
>                                     VirtViewerWindow *self)
>  {
> -    GtkBuilder *about = virt_viewer_util_load_ui("virt-viewer-about.xml");
> +    GtkBuilder *about;
> +    GtkWidget *dialog;
> +    GdkPixbuf *icon;
> +
> +    about = virt_viewer_util_load_ui("virt-viewer-about.xml");
> +
> +    dialog = GTK_WIDGET(gtk_builder_get_object(about, "about"));
>  
> -    GtkWidget *dialog = GTK_WIDGET(gtk_builder_get_object(about, "about"));
>      gtk_about_dialog_set_version(GTK_ABOUT_DIALOG(dialog), VERSION BUILDID);
>  
> +    icon = gdk_pixbuf_new_from_resource("/org/virt-manager/virt
> -viewer/icons/48x48/virt-viewer.png", NULL);

VIRT_VIEWER_RESOURCE_PREFIX could be used here.

> +    if (icon != NULL) {
> +        gtk_about_dialog_set_logo(GTK_ABOUT_DIALOG(dialog), icon);
> +        g_object_unref(icon);
> +    } else {
> +        gtk_about_dialog_set_logo_icon_name(GTK_ABOUT_DIALOG(dialog), "virt
> -viewer");
> +    }
> +
>      gtk_window_set_transient_for(GTK_WINDOW(dialog),
>                                   GTK_WINDOW(self->priv->window));
>  
> @@ -1066,8 +1079,7 @@ virt_viewer_window_toolbar_setup(VirtViewerWindow *self)
>      g_signal_connect(button, "clicked",
> G_CALLBACK(virt_viewer_window_menu_file_quit), self);
>  
>      /* USB Device selection */
> -    button = gtk_image_new_from_icon_name("virt-viewer-usb",
> -                                          GTK_ICON_SIZE_INVALID);
> +    button = gtk_image_new_from_resource("/org/virt-manager/virt
> -viewer/icons/24x24/virt-viewer-usb.png");

and here

>      button = GTK_WIDGET(gtk_tool_button_new(button, NULL));
>      gtk_tool_button_set_label(GTK_TOOL_BUTTON(button), _("USB device
> selection"));
>      gtk_tool_item_set_tooltip_text(GTK_TOOL_ITEM(button), _("USB device
> selection"));
> diff --git a/src/virt-viewer.gresource.xml b/src/virt-viewer.gresource.xml
> new file mode 100644
> index 0000000..596889a
> --- /dev/null
> +++ b/src/virt-viewer.gresource.xml
> @@ -0,0 +1,19 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<gresources>
> +  <gresource prefix="/org/virt-manager/virt-viewer">
> +    <file>remote-viewer-connect.xml</file>
> +    <file>virt-viewer-about.xml</file>
> +    <file>virt-viewer-auth.xml</file>
> +    <file>virt-viewer-guest-details.xml</file>
> +    <file>virt-viewer-preferences.xml</file>
> +    <file>virt-viewer-vm-connection.xml</file>
> +    <file>virt-viewer.xml</file>
> +    <file alias="icons/16x16/virt-viewer.png">../icons/16x16/virt
> -viewer.png</file>
> +    <file alias="icons/22x22/virt-viewer.png">../icons/22x22/virt
> -viewer.png</file>
> +    <file alias="icons/24x24/virt-viewer.png">../icons/24x24/virt
> -viewer.png</file>
> +    <file alias="icons/24x24/virt-viewer-usb.png">../icons/24x24/virt-viewer
> -usb.png</file>
> +    <file alias="icons/32x32/virt-viewer.png">../icons/32x32/virt
> -viewer.png</file>
> +    <file alias="icons/48x48/virt-viewer.png">../icons/48x48/virt
> -viewer.png</file>
> +    <file alias="icons/256x256/virt-viewer.png">../icons/256x256/virt
> -viewer.png</file>
> +  </gresource>
> +</gresources>


Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>




More information about the virt-tools-list mailing list