Re: gtk_tree_model_sort_increment_stamp doesn't increment stamp

[Kristian Rietveld <kris gtk org>]

On Tue, 2002-02-12 at 01:44, Jonathan Blandford wrote:
> Darin Adler <darin bentspoon com> writes:
> 
> > On 2/11/02 4:11 PM, "Jonathan Blandford" <jrb redhat com> wrote:
> > 
> > > Eek.  That is almost certainly supposed to be:
> > > 
> > > do { tree_model_sort->stamp++; } while (tree_model_sort->stamp == 0);
> > > 
> > > We don't want stamp wrapping around to 0, but we do want to increase it
> > > most of the time. (-;
> > 
> > I had assumed that, and I changed it to that, and found all sorts of asserts
> > firing. I think gtk_tree_model_sort_increment_stamp is called at in many
> > places where it's not correct to invalidate all outstanding iterators.
> 
> That's bad!  This bug may hurt.
> 
> > I have a more-correct version of the patch to fix the insert_value function,
> > but things aren't sorting correctly for me, so I assume there's more to fix.
> > That's why I didn't post the patch before now. I'm running out of stamina
> > for GtkTreeModelSort hacking for today.
> 
> Understandable.
> 
> > Also, there is enough untested here that I think I (or someone else) better
> > debug this in a GtkTreeModelSort test, rather than using the Nautilus tree
> > view as the test case.
> 
> I'm getting a few warnings here with treestoretest.  Kristian, do you
> have time to give it a quick look?  I think it's just stamp incremented
> at the wrong time, but I could be wrong.

Hmm, I fooled around with treestoretest and found out that the
assertions sometimes occur when you remove a node. This was caused by
ref counting problems. Appended patch fixes these issues for me.

Jonathan, may I commit this?


regards,

	Kris


Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/gtk+/ChangeLog,v
retrieving revision 1.2998
diff -u -r1.2998 ChangeLog
--- ChangeLog	2002/02/12 01:39:20	1.2998
+++ ChangeLog	2002/02/12 13:28:33
@@ -1,3 +1,13 @@
+Tue Feb 12 14:27:41 2002  Kristian Rietveld  <kris gtk org>
+
+	* gtk/gtktreemodelsort.c (gtk_tree_model_sort_row_deleted): emit
+	row_deleted before we start unreffing the nodes ourselves
+	(gtk_tree_model_sort_increment_stamp): always increment the stamp,
+	and not only if the stamp equals 0 (pointed out by Darin Adler)
+
+	* gtk/gtktreemodel.c (gtk_tree_row_ref_deleted_callback): we want
+	to continue the while loop and not the nested for loop.
+
 Tue Feb 12 02:19:49 2002  Tim Janik  <timj gtk org>
 
 	* gtk/gtktable.c (gtk_table_size_allocate_pass1): always spread out
Index: gtk/gtktreemodelsort.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktreemodelsort.c,v
retrieving revision 1.55
diff -u -r1.55 gtktreemodelsort.c
--- gtk/gtktreemodelsort.c	2002/02/12 01:00:22	1.55
+++ gtk/gtktreemodelsort.c	2002/02/12 13:28:35
@@ -693,6 +693,10 @@
   elt = SORT_ELT (iter.user_data2);
   offset = elt->offset;
 
+  gtk_tree_model_row_deleted (GTK_TREE_MODEL (data), path);
+
+  gtk_tree_model_get_iter (GTK_TREE_MODEL (data), &iter, path);
+
   while (elt->ref_count > 0)
     gtk_tree_model_sort_real_unref_node (GTK_TREE_MODEL (data), &iter, FALSE);
 
@@ -701,13 +705,11 @@
       /* This will prune the level, so I can just emit the signal and not worry
        * about cleaning this level up. */
       gtk_tree_model_sort_increment_stamp (tree_model_sort);
-      gtk_tree_model_row_deleted (GTK_TREE_MODEL (data), path);
       gtk_tree_path_free (path);
       return;
     }
 
   gtk_tree_model_sort_increment_stamp (tree_model_sort);
-  gtk_tree_model_row_deleted (GTK_TREE_MODEL (data), path);
 
   /* Remove the row */
   for (i = 0; i < level->array->len; i++)
@@ -2141,7 +2143,11 @@
 static void
 gtk_tree_model_sort_increment_stamp (GtkTreeModelSort *tree_model_sort)
 {
-  while (tree_model_sort->stamp == 0) tree_model_sort->stamp++;
+  do
+    {
+      tree_model_sort->stamp++;
+    }
+  while (tree_model_sort->stamp == 0);
 
   gtk_tree_model_sort_clear_cache (tree_model_sort);
 }
Index: gtk/gtktreemodel.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktreemodel.c,v
retrieving revision 1.73
diff -u -r1.73 gtktreemodel.c
--- gtk/gtktreemodel.c	2002/01/31 23:47:19	1.73
+++ gtk/gtktreemodel.c	2002/02/12 13:28:36
@@ -1341,10 +1341,7 @@
 	  for (i = 0; i < path->depth - 1; i++)
 	    {
 	      if (path->indices[i] != reference->path->indices[i])
-		{
-		  tmp_list = g_slist_next (tmp_list);
-		  continue;
-		}
+		goto next;
 	    }
 
 	  /* We know it affects us. */
@@ -1359,6 +1356,8 @@
 	      reference->path->indices[path->depth-1]-=1;
 	    }
 	}
+
+next:
       tmp_list = g_slist_next (tmp_list);
     }
 }