Hi, On Tue, Aug 16, 2016 at 10:30:57AM -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. 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. We might miss the cancel per each file transfer as we don't know which files are being copied, which files had error and which files were totally copied to the guest. Let's take two situations: 1) One operation with several files - Drag and drop several files with different sizes. You'll see the counter decrease but from UI you can't say which files are being transferred. Let's say that, by mistake, you selected a big file and would like to cancel it, you can't. 2) Several operations - Drag and drop one big file; then another big file; then another. Now you will see the counter at '3'. Based on network performance, you might regret that and wanted to cancel two transfers to use the bandwidth only on one per time. (Old days with dial-up feelings!) The previous version, you were hiding the files and that was great. You could work around situations by let the user cancel the file transfers of individual files by themselves. Not anymore. I think the use-case is different from Nautilus as mentioned previously by Fabiano [0] - It is more likely download/upload then plain copy on mounted devices. [0] https://www.redhat.com/archives/virt-tools-list/2016-August/msg00048.html With that being said, I agree that the major goal is moving towards better guest integration but I still think that we will need SpiceFileTransferTask and client-side (transfer) information so this UI (re)work on virt-viewer is still important for the long run. Cheers, toso > > 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() > --- > Changes in v3: > - patch should now apply to git master > > 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 | 152 ++++++++------------- > 4 files changed, 146 insertions(+), 95 deletions(-) > create mode 100644 src/resources/ui/virt-viewer-file-transfer-dialog.ui > > 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"/> > + <!-- 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"> > + <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..cef5448 100644 > --- a/src/virt-viewer-file-transfer-dialog.c > +++ b/src/virt-viewer-file-transfer-dialog.c > @@ -23,26 +23,31 @@ > #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 +56,16 @@ static void > virt_viewer_file_transfer_dialog_class_init(VirtViewerFileTransferDialogClass *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, > + VirtViewerFileTransferDialog, > + transfer_summary); > + gtk_widget_class_bind_template_child_private(widget_class, > + VirtViewerFileTransferDialog, > + progressbar); > > object_class->dispose = virt_viewer_file_transfer_dialog_dispose; > } > @@ -63,16 +76,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 +93,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 +106,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_unref, > - (GDestroyNotify)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 +124,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) > @@ -195,7 +171,6 @@ static gboolean hide_transfer_dialog(gpointer data) > > typedef struct { > VirtViewerFileTransferDialog *self; > - TaskWidgets *widgets; > SpiceFileTransferTask *task; > } TaskFinishedData; > > @@ -203,9 +178,6 @@ 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); > > @@ -218,26 +190,21 @@ static void task_finished(SpiceFileTransferTask *task, > { > 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); > + 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(d->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) { > @@ -254,6 +221,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 +250,7 @@ static void show_transfer_dialog(VirtViewerFileTransferDialog *self) > void virt_viewer_file_transfer_dialog_add_task(VirtViewerFileTransferDialog *self, > SpiceFileTransferTask *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); > > -- > 2.7.4 > > _______________________________________________ > virt-tools-list mailing list > virt-tools-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/virt-tools-list _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
