[virt-tools-list] [PATCH virt-viewer v4 1/2] Simplify file transfer dialog UI
Pavel Grunt
pgrunt at redhat.com
Thu Oct 27 09:41:49 UTC 2016
Hi,
On Mon, 2016-10-24 at 16:04 -0500, Jonathon Jongsma wrote:
> When transferring a large number of files, the file transfer dialog
> was
> unusable because the window size would be larger than the client
> desktop.
The another issue is that it is always on the top
> To solve this, remove the list of individual files (and the
> ability to cancel each file transfer independantly) and only display
> a single overall progress bar that shows the status of all ongoing
> transfers.
>
> This also allows us to remove the delayed unref of the task since we
> don't need to show the task information about each individual
> transfer
> task until the window is closed. Removes TaskFinishedData type.
>
> This patch requires new API from spice-gtk to calculate the overall
> progress:
> spice_file_transfer_task_get_total_bytes()
> spice_file_transfer_task_get_transferred_bytes()
Related: rhbz#1351847
> ---
> configure.ac | 2 +-
> src/Makefile.am | 1 +
> .../ui/virt-viewer-file-transfer-dialog.ui | 87 ++++++++++
> src/resources/virt-viewer.gresource.xml | 1 +
> src/virt-viewer-file-transfer-dialog.c | 178 +++++++---
> -----------
> 5 files changed, 149 insertions(+), 120 deletions(-)
> create mode 100644 src/resources/ui/virt-viewer-file-transfer-
> dialog.ui
>
> diff --git a/configure.ac b/configure.ac
> index 89e7641..81cb576 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -24,7 +24,7 @@ LIBXML2_REQUIRED="2.6.0"
> LIBVIRT_REQUIRED="0.10.0"
> LIBVIRT_GLIB_REQUIRED="0.1.8"
> GTK_VNC_REQUIRED="0.4.0"
> -SPICE_GTK_REQUIRED="0.31"
> +SPICE_GTK_REQUIRED="0.33"
> SPICE_PROTOCOL_REQUIRED="0.12.7"
> GOVIRT_REQUIRED="0.3.2"
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 0c48e40..272c4ff 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -13,6 +13,7 @@ noinst_DATA = \
> resources/ui/virt-viewer-vm-connection.ui \
> resources/ui/virt-viewer-preferences.ui \
> resources/ui/remote-viewer-connect.ui \
> + resources/ui/virt-viewer-file-transfer-dialog.ui \
> $(NULL)
>
> EXTRA_DIST = \
> diff --git a/src/resources/ui/virt-viewer-file-transfer-dialog.ui
> b/src/resources/ui/virt-viewer-file-transfer-dialog.ui
> new file mode 100644
> index 0000000..e3514d6
> --- /dev/null
> +++ b/src/resources/ui/virt-viewer-file-transfer-dialog.ui
> @@ -0,0 +1,87 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<interface>
> + <requires lib="gtk+" version="2.24"/>
should be 3.10
> + <!-- interface-naming-policy project-wide -->
> + <template class="VirtViewerFileTransferDialog"
> parent="GtkDialog">
> + <property name="default_width">400</property>
> + <property name="can_focus">False</property>
> + <property name="border_width">5</property>
> + <property name="type_hint">dialog</property>
> + <child internal-child="vbox">
> + <object class="GtkVBox" id="dialog-vbox1">
GtkVBox is deprecated, please switch to GtkGrid
Usually I open the file in Glade and set required version from there -
it informs about deprecated/not available widgets and properties
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="spacing">12</property>
> + <property name="border-width">12</property>
> + <child internal-child="action_area">
> + <object class="GtkHButtonBox" id="dialog-action_area1">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="layout_style">end</property>
> + <child>
> + <placeholder/>
> + </child>
> + <child>
> + <object class="GtkButton" id="button1">
> + <property name="label">gtk-cancel</property>
> + <property name="visible">True</property>
> + <property name="can_focus">True</property>
> + <property name="receives_default">True</property>
> + <property name="use_underline">True</property>
> + <property name="use_stock">True</property>
> + </object>
> + <packing>
> + <property name="expand">False</property>
> + <property name="fill">False</property>
> + <property name="position">1</property>
> + </packing>
> + </child>
> + </object>
> + <packing>
> + <property name="expand">True</property>
> + <property name="fill">True</property>
> + <property name="position">0</property>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkVBox" id="vbox1">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="spacing">12</property>
> + <child>
> + <object class="GtkLabel" id="transfer_summary">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="label"
> translatable="yes">label</property>
> + </object>
> + <packing>
> + <property name="expand">True</property>
> + <property name="fill">True</property>
> + <property name="position">0</property>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkProgressBar" id="progressbar">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + </object>
> + <packing>
> + <property name="expand">True</property>
> + <property name="fill">True</property>
> + <property name="position">1</property>
> + </packing>
> + </child>
> + </object>
> + <packing>
> + <property name="expand">True</property>
> + <property name="fill">True</property>
> + <property name="position">1</property>
> + </packing>
> + </child>
> + </object>
> + </child>
> + <action-widgets>
> + <action-widget response="-6">button1</action-widget>
> + </action-widgets>
> + </template>
> +</interface>
> diff --git a/src/resources/virt-viewer.gresource.xml
> b/src/resources/virt-viewer.gresource.xml
> index b8ced29..f9b4a9f 100644
> --- a/src/resources/virt-viewer.gresource.xml
> +++ b/src/resources/virt-viewer.gresource.xml
> @@ -8,6 +8,7 @@
> <file>ui/virt-viewer-preferences.ui</file>
> <file>ui/virt-viewer-vm-connection.ui</file>
> <file>ui/virt-viewer.ui</file>
> + <file>ui/virt-viewer-file-transfer-dialog.ui</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>
> diff --git a/src/virt-viewer-file-transfer-dialog.c b/src/virt-
> viewer-file-transfer-dialog.c
> index c8b50f0..3339ba4 100644
> --- a/src/virt-viewer-file-transfer-dialog.c
> +++ b/src/virt-viewer-file-transfer-dialog.c
> @@ -23,26 +23,30 @@
> #include "virt-viewer-file-transfer-dialog.h"
> #include <glib/gi18n.h>
>
> -G_DEFINE_TYPE(VirtViewerFileTransferDialog,
> virt_viewer_file_transfer_dialog, GTK_TYPE_DIALOG)
> -
> -#define FILE_TRANSFER_DIALOG_PRIVATE(o) \
> - (G_TYPE_INSTANCE_GET_PRIVATE((o),
> VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG,
> VirtViewerFileTransferDialogPrivate))
> -
> struct _VirtViewerFileTransferDialogPrivate
> {
> - /* GHashTable<SpiceFileTransferTask, widgets> */
> - GHashTable *file_transfers;
> + GSList *file_transfers;
> guint timer_show_src;
> guint timer_hide_src;
> + GtkWidget *transfer_summary;
> + GtkWidget *progressbar;
> };
>
> +G_DEFINE_TYPE_WITH_PRIVATE(VirtViewerFileTransferDialog,
> virt_viewer_file_transfer_dialog, GTK_TYPE_DIALOG)
> +
> +#define FILE_TRANSFER_DIALOG_PRIVATE(o) \
> + (G_TYPE_INSTANCE_GET_PRIVATE((o),
> VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG,
> VirtViewerFileTransferDialogPrivate))
> +
>
> static void
> virt_viewer_file_transfer_dialog_dispose(GObject *object)
> {
> VirtViewerFileTransferDialog *self =
> VIRT_VIEWER_FILE_TRANSFER_DIALOG(object);
>
> - g_clear_pointer(&self->priv->file_transfers,
> g_hash_table_unref);
> + if (self->priv->file_transfers) {
> + g_slist_free_full(self->priv->file_transfers,
> g_object_unref);
> + self->priv->file_transfers = NULL;
> + }
>
> G_OBJECT_CLASS(virt_viewer_file_transfer_dialog_parent_class)-
> >dispose(object);
> }
> @@ -51,8 +55,16 @@ static void
> virt_viewer_file_transfer_dialog_class_init(VirtViewerFileTransferD
> ialogClass *klass)
> {
> GObjectClass *object_class = G_OBJECT_CLASS(klass);
> + GtkWidgetClass *widget_class = GTK_WIDGET_CLASS(klass);
>
> - g_type_class_add_private(klass,
> sizeof(VirtViewerFileTransferDialogPrivate));
> + gtk_widget_class_set_template_from_resource(widget_class,
> + "/org/virt-
> manager/virt-viewer/ui/virt-viewer-file-transfer-dialog.ui");
> + gtk_widget_class_bind_template_child_private(widget_class,
> + VirtViewerFileTran
> sferDialog,
> + transfer_summary);
> + gtk_widget_class_bind_template_child_private(widget_class,
> + VirtViewerFileTran
> sferDialog,
> + progressbar);
>
> object_class->dispose =
> virt_viewer_file_transfer_dialog_dispose;
> }
> @@ -63,16 +75,13 @@ dialog_response(GtkDialog *dialog,
> gpointer user_data G_GNUC_UNUSED)
> {
> VirtViewerFileTransferDialog *self =
> VIRT_VIEWER_FILE_TRANSFER_DIALOG(dialog);
> - GHashTableIter iter;
> - gpointer key, value;
> + GSList *slist = self->priv->file_transfers;
>
> switch (response_id) {
> case GTK_RESPONSE_CANCEL:
> /* cancel all current tasks */
> - g_hash_table_iter_init(&iter, self->priv-
> >file_transfers);
> -
> - while (g_hash_table_iter_next(&iter, &key, &value)) {
> - spice_file_transfer_task_cancel(SPICE_FILE_TRANSFER
> _TASK(key));
> + for (slist = self->priv->file_transfers; slist != NULL;
> slist = g_slist_next(slist)) {
> + spice_file_transfer_task_cancel(SPICE_FILE_TRANSFER
> _TASK(slist->data));
> }
> break;
> case GTK_RESPONSE_DELETE_EVENT:
> @@ -83,53 +92,6 @@ dialog_response(GtkDialog *dialog,
> }
> }
>
> -static void task_cancel_clicked(GtkButton *button G_GNUC_UNUSED,
> - gpointer user_data)
> -{
> - SpiceFileTransferTask *task = user_data;
> - spice_file_transfer_task_cancel(task);
> -}
> -
> -typedef struct {
> - GtkWidget *vbox;
> - GtkWidget *hbox;
> - GtkWidget *progress;
> - GtkWidget *label;
> - GtkWidget *cancel;
> -} TaskWidgets;
> -
> -static TaskWidgets *task_widgets_new(SpiceFileTransferTask *task)
> -{
> - TaskWidgets *w = g_new0(TaskWidgets, 1);
> - gchar *filename;
> -
> - w->vbox = gtk_box_new(GTK_ORIENTATION_VERTICAL, 6);
> - w->hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 12);
> - w->progress = gtk_progress_bar_new();
> - filename = spice_file_transfer_task_get_filename(task);
> - w->label = gtk_label_new(filename);
> - g_free(filename);
> - w->cancel = gtk_button_new_from_icon_name("gtk-cancel",
> GTK_ICON_SIZE_SMALL_TOOLBAR);
> - gtk_widget_set_hexpand(w->progress, TRUE);
> - gtk_widget_set_valign(w->progress, GTK_ALIGN_CENTER);
> - gtk_widget_set_hexpand(w->label, TRUE);
> - gtk_widget_set_valign(w->label, GTK_ALIGN_END);
> - gtk_widget_set_halign(w->label, GTK_ALIGN_START);
> - gtk_widget_set_hexpand(w->cancel, FALSE);
> - gtk_widget_set_valign(w->cancel, GTK_ALIGN_CENTER);
> -
> - g_signal_connect(w->cancel, "clicked",
> - G_CALLBACK(task_cancel_clicked), task);
> -
> - gtk_box_pack_start(GTK_BOX(w->hbox), w->progress, TRUE, TRUE,
> 0);
> - gtk_box_pack_start(GTK_BOX(w->hbox), w->cancel, FALSE, TRUE,
> 0);
> - gtk_box_pack_start(GTK_BOX(w->vbox), w->label, TRUE, TRUE, 0);
> - gtk_box_pack_start(GTK_BOX(w->vbox), w->hbox, TRUE, TRUE, 0);
> -
> - gtk_widget_show_all(w->vbox);
> - return w;
> -}
> -
> static gboolean delete_event(GtkWidget *widget,
> GdkEvent *event G_GNUC_UNUSED,
> gpointer user_data G_GNUC_UNUSED)
> @@ -143,18 +105,10 @@ static gboolean delete_event(GtkWidget
> *widget,
> static void
> virt_viewer_file_transfer_dialog_init(VirtViewerFileTransferDialog
> *self)
> {
> - GtkBox *content =
> GTK_BOX(gtk_dialog_get_content_area(GTK_DIALOG(self)));
> + gtk_widget_init_template(GTK_WIDGET(self));
>
> self->priv = FILE_TRANSFER_DIALOG_PRIVATE(self);
>
> - gtk_widget_set_size_request(GTK_WIDGET(content), 400, -1);
> - gtk_container_set_border_width(GTK_CONTAINER(content), 12);
> - self->priv->file_transfers =
> g_hash_table_new_full(g_direct_hash, g_direct_equal,
> - g_object_unr
> ef,
> - (GDestroyNot
> ify)g_free);
> - gtk_dialog_add_button(GTK_DIALOG(self), _("Cancel"),
> GTK_RESPONSE_CANCEL);
> - gtk_dialog_set_default_response(GTK_DIALOG(self),
> - GTK_RESPONSE_CANCEL);
> g_signal_connect(self, "response", G_CALLBACK(dialog_response),
> NULL);
> g_signal_connect(self, "delete-event",
> G_CALLBACK(delete_event), NULL);
> }
> @@ -169,17 +123,38 @@ virt_viewer_file_transfer_dialog_new(GtkWindow
> *parent)
> NULL);
> }
>
> -static void task_progress_notify(GObject *object,
> +static void update_global_progress(VirtViewerFileTransferDialog
> *self)
> +{
> + GSList *slist;
> + guint64 total = 0, transferred = 0;
> + gchar *message = NULL;
> + guint n_files = 0;
> + gdouble fraction = 1.0;
> +
> + for (slist = self->priv->file_transfers; slist != NULL; slist =
> g_slist_next(slist)) {
> + SpiceFileTransferTask *task = slist->data;
> + total += spice_file_transfer_task_get_total_bytes(task);
> + transferred +=
> spice_file_transfer_task_get_transferred_bytes(task);
> + n_files++;
> + }
> +
> + if (n_files > 0)
> + fraction = (gdouble)transferred / total;
> + message = g_strdup_printf(ngettext("Transferring %d file...",
> + "Transferring %d files...",
> n_files),
> + n_files);
> + gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(self->priv-
> >progressbar), fraction);
> + gtk_label_set_text(GTK_LABEL(self->priv->transfer_summary),
> message);
> + g_free(message);
> +}
> +
> +static void task_progress_notify(GObject *object G_GNUC_UNUSED,
> GParamSpec *pspec G_GNUC_UNUSED,
> gpointer user_data)
> {
> VirtViewerFileTransferDialog *self =
> VIRT_VIEWER_FILE_TRANSFER_DIALOG(user_data);
> - SpiceFileTransferTask *task = SPICE_FILE_TRANSFER_TASK(object);
> - TaskWidgets *w = g_hash_table_lookup(self->priv-
> >file_transfers, task);
> - g_return_if_fail(w);
>
> - double pct = spice_file_transfer_task_get_progress(task);
> - gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(w->progress),
> pct);
> + update_global_progress(self);
> }
>
> static gboolean hide_transfer_dialog(gpointer data)
> @@ -193,51 +168,21 @@ static gboolean hide_transfer_dialog(gpointer
> data)
> return G_SOURCE_REMOVE;
> }
>
> -typedef struct {
> - VirtViewerFileTransferDialog *self;
> - TaskWidgets *widgets;
> - SpiceFileTransferTask *task;
> -} TaskFinishedData;
> -
> -static gboolean task_finished_remove(gpointer user_data)
> -{
> - TaskFinishedData *d = user_data;
> -
> - gtk_widget_destroy(d->widgets->vbox);
> -
> - g_free(d->widgets);
> - g_object_unref(d->task);
> - g_free(d);
> -
> - return G_SOURCE_REMOVE;
> -}
> -
> static void task_finished(SpiceFileTransferTask *task,
> GError *error,
> gpointer user_data)
> {
> - TaskFinishedData *d;
> VirtViewerFileTransferDialog *self =
> VIRT_VIEWER_FILE_TRANSFER_DIALOG(user_data);
> - TaskWidgets *w = g_hash_table_lookup(self->priv-
> >file_transfers, task);
>
> if (error && !g_error_matches(error, G_IO_ERROR,
> G_IO_ERROR_CANCELLED))
> g_warning("File transfer task %p failed: %s", task, error-
> >message);
>
> - g_return_if_fail(w);
> - gtk_widget_set_sensitive(w->cancel, FALSE);
> -
> -
> - d = g_new0(TaskFinishedData, 1);
> - d->self = self;
> - d->widgets = w;
> - d->task = task;
> -
> - g_timeout_add(500, task_finished_remove, d);
> -
> - g_hash_table_steal(self->priv->file_transfers, task);
> + self->priv->file_transfers = g_slist_remove(self->priv-
> >file_transfers, task);
> + g_object_unref(task);
> + update_global_progress(self);
>
> /* if this is the last transfer, close the dialog */
> - if (!g_hash_table_size(d->self->priv->file_transfers)) {
> + if (!g_slist_length(self->priv->file_transfers)) {
> /* cancel any pending 'show' operations if all tasks
> complete before
> * the dialog can be shown */
> if (self->priv->timer_show_src) {
> @@ -245,7 +190,7 @@ static void task_finished(SpiceFileTransferTask
> *task,
> self->priv->timer_show_src = 0;
> }
> self->priv->timer_hide_src = g_timeout_add(500,
> hide_transfer_dialog,
> - d->self);
> + self);
> }
> }
>
> @@ -254,6 +199,7 @@ static gboolean
> show_transfer_dialog_delayed(gpointer user_data)
> VirtViewerFileTransferDialog *self = user_data;
>
> self->priv->timer_show_src = 0;
> + update_global_progress(self);
> gtk_widget_show(GTK_WIDGET(self));
>
> return G_SOURCE_REMOVE;
> @@ -282,13 +228,7 @@ static void
> show_transfer_dialog(VirtViewerFileTransferDialog *self)
> void
> virt_viewer_file_transfer_dialog_add_task(VirtViewerFileTransferDial
> og *self,
> SpiceFileTransferTas
> k *task)
> {
> - GtkBox *content =
> GTK_BOX(gtk_dialog_get_content_area(GTK_DIALOG(self)));
> - TaskWidgets *w = task_widgets_new(task);
> -
> - gtk_box_pack_start(content,
> - w->vbox,
> - TRUE, TRUE, 12);
> - g_hash_table_insert(self->priv->file_transfers,
> g_object_ref(task), w);
> + self->priv->file_transfers = g_slist_prepend(self->priv-
> >file_transfers, g_object_ref(task));
> g_signal_connect(task, "notify::progress",
> G_CALLBACK(task_progress_notify), self);
> g_signal_connect(task, "finished", G_CALLBACK(task_finished),
> self);
>
overall it looks good to me.
Pavel
More information about the virt-tools-list
mailing list