masneyb / gftp

gFTP is a free multithreaded file transfer client for *NIX based machines. 56 language translations available.
http://www.gftp.org
MIT License
118 stars 21 forks source link

GdkPixmap cairo converson support #84

Open sedwards opened 4 years ago

sedwards commented 4 years ago

Most of the major remaining issues I see blocking the full GTK3 port are based around GdkPixmap which will have to be replace with Cairo surface support functionality.

I believe these changes can be made and maintain compatibility with the gtk2 build but it is going to require quite a bit of work and I haven't the time to get my head around it at the moment.

wdlkmpx commented 4 years ago

I made many changes in the last months and removed GdkPixmap stuff, the big challenge is the GtkTreeView and I'm one commit away from removing the remaining gftp_graphic / GdkPixmap functions, but it might take me a month to finish the commit while I slowly learn. Relevant commits:

add gftp_get_pixbuf() and pixbuf_hash_table https://github.com/masneyb/gftp/commit/47f0f01d68f2e02366445b913055166b8aca3360

GtkTreeView Part1: replace GtkCList ["listbox"] https://github.com/masneyb/gftp/commit/13d0fc64a6cafeec6e54ea9fd21befa59197e7f3

GtkTreeView Part3: replace GtkCTree in Bookmarks dialog (bookmarks.c) https://github.com/masneyb/gftp/commit/192cbd2ed7bc7ea5a16ad6c4b7b373abb3e27be5

sedwards commented 4 years ago

I'll review...maybe I can help nibble at some of the edges without stepping on your work.

sedwards commented 4 years ago

Thats's awesome progress by the way. Check out the current state of the GTK3 build as of today.

Screen Shot 2020-07-19 at 12 51 16 AM
wdlkmpx commented 4 years ago

I think the main issue with porting GtkCList/CTree to GtkTreeTView is ... GtkTreeIter, it's not a pointer but a structure that is filled by functions.

gFTP uses hash tables and linked lists that store information about the tree nodes, in this case it can't be a pointer but a structure with 4 fields, how about gftp_iter?

Things are going and it takes quite a bit of time to understand it all. I see potential but some refactoring would make it work better, but only a gtk guru of the highest order can do that.

I tend to use g_object_new to create widgets, you can specify many properties, and with g_object_set() you don't need to learn a thousand functions ... that's just the beginning, it's possible to create classes and so on, but it makes the code harder to understand. Object oriented programming is a serious business.

wdlkmpx commented 4 years ago

The "transfer window" GtkCTree combines the difficulty of all the previous examples, that's why the Bookmarks GtkTreeView must be properly implemented.

I have come to the conclusion that the gFTP functions should operate directly on the GtkTreeModel (GtkTreeStore, GtkListStore)... the linked lists should be converted to GtkTreeModel by using a pointer to entries in a hidden column (already done).

And then converted back to the usual gftp linked lists / hash tables, by iterating through the GtkTreeModel nodes. This simplifies the logic.

That way the Bookmarks GtkTreeView can be reorderable.

In this example that I'll probably push later, I allocate a GtkTreeIter dynamically and make bookmark->cnode point to it, this simplifies the logic, but I have to free bookmark->cnode manually.

Of course this is not the correct way to try to make the treeview reorderable because bookmark->cnode becomes invalid when the GtkTreeView nodes are deleted or rearranged

diff --git a/src/gtk/bookmarks.c b/src/gtk/bookmarks.c
index 2a3b186..4eecdf3 100644
--- a/src/gtk/bookmarks.c
+++ b/src/gtk/bookmarks.c
@@ -359,7 +360,7 @@ _free_menu_entry (gftp_bookmarks_var * entry)

 static void
-bm_apply_changes (GtkWidget * widget, gpointer backup_data)
+bm_apply_changes ()
 {
   gftp_bookmarks_var * tempentry, * delentry;

@@ -398,19 +399,11 @@ bm_apply_changes (GtkWidget * widget, gpointer backup_data)
       g_hash_table_destroy (gftp_bookmarks_htable);
     }

-  if (backup_data)
-    {
-      gftp_bookmarks = copy_bookmarks (new_bookmarks);
-      gftp_bookmarks_htable = build_bookmarks_hash_table (gftp_bookmarks);
-    }
-  else
-    {
-      gftp_bookmarks = new_bookmarks;
-      gftp_bookmarks_htable = new_bookmarks_htable;
+  gftp_bookmarks = new_bookmarks;
+  gftp_bookmarks_htable = new_bookmarks_htable;

-      new_bookmarks = NULL;
-      new_bookmarks_htable = NULL;
-    }
+  new_bookmarks = NULL;
+  new_bookmarks_htable = NULL;

   build_bookmarks_menu ();
   gftp_write_bookmarks_file ();
@@ -422,15 +415,42 @@ on_gtk_dialog_response_BookmarkDlg (GtkDialog * dialog,
                                     gint response,
                                     gpointer user_data)
 { /* ok | cancel | close dialog */
+
+  gftp_bookmarks_var * tempentry;
+  tempentry = new_bookmarks->children;
+
   if (response == GTK_RESPONSE_OK)
     {
-        bm_apply_changes (GTK_WIDGET(dialog), NULL);
+        bm_apply_changes ();
+        tempentry = gftp_bookmarks->children;
     }

-  /* free allocated memory */
   if (edit_bm_entry_dlg != NULL)
     return;

+  /* free allocated memory */
+
+  gftp_bookmarks_var * child_entry;
+  while (tempentry)
+    {
+       if (tempentry->cnode)
+         { /* free cnode (GtkTreeIter) */
+            fprintf (stderr, "%s\n", tempentry->path);
+            g_free (tempentry->cnode);
+         }
+       if (tempentry->isfolder && tempentry->children)
+         {
+           child_entry = tempentry->children;
+           while (child_entry) {
+              fprintf (stderr, "%s\n", child_entry->path);
+              if (child_entry->cnode)
+                  g_free (child_entry->cnode);
+              child_entry = child_entry->next;
+            }
+          }
+       tempentry = tempentry->next;
+    }
+
   if (new_bookmarks_htable != NULL)
     {
       g_hash_table_destroy (new_bookmarks_htable);
@@ -504,6 +524,9 @@ do_make_new (gpointer data, gftp_dialog_data * ddata)
                       BTREEVIEW_COL_BOOKMARK, newentry,
                       -1);
   g_hash_table_insert (new_bookmarks_htable, newentry->path, newentry);
+
+  newentry->cnode = g_malloc0 (sizeof (iter));
+  memcpy (newentry->cnode, &iter, sizeof (iter));
 }

@@ -533,6 +556,9 @@ do_delete_entry (gftp_bookmarks_var * entry, gftp_dialog_data * ddata)
   gftp_bookmarks_var * tempentry, * delentry;

   g_hash_table_remove (new_bookmarks_htable, entry->path);
+  /* free cnode (GtkTreeIter) */
+  if (entry->cnode)
+    g_free (entry->cnode);

   btree_remove_selected_node ();

@@ -542,7 +568,7 @@ do_delete_entry (gftp_bookmarks_var * entry, gftp_dialog_data * ddata)
     {
       tempentry = entry->prev->children;
       while (tempentry->next != entry)
-   tempentry = tempentry->next;
+        tempentry = tempentry->next;
       tempentry->next = entry->next;
     }

@@ -1307,64 +1333,10 @@ btree_create()
 }

-static gboolean
-btree_find_iter_by_str_bpath (GtkTreeModel *model,
-                              GtkTreePath *path,
-                              GtkTreeIter *iter,
-                              gpointer data)
-{
-  // Search GtkTreeView for duplicate or parent nodes.
-  //
-  // btree_current_path can be:
-  // - BSD Sites
-  // - BSD Sites/FreeBSD
-  //      dir     child
-  //
-  // Need to find duplicate entries and ignore them.
-  // If there's a 'dir', there certainly is a parent node
-
-  char *dir, *p, tempchar;
-  dir = strrchr (btree_current_path, '/');
-
-  gftp_bookmarks_var *entry;
-  GtkTreeIter *found_parent = (GtkTreeIter *) data;
-
-  gtk_tree_model_get (model, iter, BTREEVIEW_COL_BOOKMARK, &entry, -1);
-  if (entry && entry->path) {
-    if (strcmp(entry->path, btree_current_path) == 0)
-    {
-      // duplicate, ignore
-      btree_current_path = NULL;
-      return TRUE;
-    }
-    if (dir) /* determine parent node */
-    {
-      // remove and restore 'child' path
-      // BSD Sites/FreeBSD -> BSD Sites (strcmp) -> BSD Sites/FreeBSD
-      p = dir;
-      tempchar = *p;
-      *p = 0;
-      if (strcmp(entry->path, btree_current_path) == 0)
-      {
-        *p = tempchar;
-        found_parent->stamp = iter->stamp;
-        found_parent->user_data = iter->user_data;
-        found_parent->user_data2 = iter->user_data2;
-        found_parent->user_data3 = iter->user_data3;
-        return TRUE;
-      }
-      *p = tempchar;
-    }
-  }
-  return FALSE;
-}
-
-
 void
 btree_add_node (gftp_bookmarks_var * entry, char *path, int isleaf)
 {
   GtkTreeStore *store = GTK_TREE_STORE (gtk_tree_view_get_model (btree));
-  GtkTreeModel *model = GTK_TREE_MODEL (store);
   GtkTreeIter  iter;
   GtkTreeIter  *parent;
   GdkPixbuf *pixbuf;
@@ -1383,37 +1355,20 @@ btree_add_node (gftp_bookmarks_var * entry, char *path, int isleaf)
   text = pos;

   if (entry->prev == NULL)
-    {
-      // main node - empty entry->path
-      gtk_tree_store_insert (store, &iter, NULL, -1);
-      main_btree_node.stamp = iter.stamp;
-      main_btree_node.user_data = iter.user_data;
-      main_btree_node.user_data2 = iter.user_data2;
-      main_btree_node.user_data3 = iter.user_data3;
-    }
+    parent = NULL;
   else
-    {
-      // (no parent)      parent   text
-      // BSD Sites      BSD Sites/FreeBSD
-      GtkTreeIter found_parent;
-      memset (&found_parent, 0, sizeof(GtkTreeIter));
-      btree_current_path = entry->path;
-      gtk_tree_model_foreach (model, btree_find_iter_by_str_bpath, &found_parent);
-      if (btree_current_path == NULL) {
-         // duplicate entry, ignore
-         return;
-      }
-      if (found_parent.stamp == 0) {
-        parent = &main_btree_node;
-      } else {
-        parent = &found_parent;
-      }
-      gtk_tree_store_insert (store, &iter, parent, -1);
-    }
+    parent = entry->prev->cnode;
   ///fprintf(stderr, "%s\n-- %s\n\n", text, entry->path);

-  // hack, cnode must not be NULL, see build_bookmarks_tree()
-  entry->cnode = GINT_TO_POINTER (1);
+  /* &iter is modified to contain the new node information */
+  gtk_tree_store_insert (store, &iter, parent, -1);
+  if (!parent) {
+    memcpy (&main_btree_node, &iter, sizeof (iter));
+  }
+
+  /* allocate a GtkTreeIter.. must be freed */
+  entry->cnode = g_malloc0 (sizeof (iter));
+  memcpy (entry->cnode, &iter, sizeof (iter));

   if (isleaf) {
     pixbuf = bookmark_pixbuf;