GSequence iterators point at the position between two elements so an
iterator pointing at the N tree model node is actually between the N-1
and N sequence elements. This means that asking for the previous
sequence iterator first and then checking if it is the begin iterator
would yeld true for an iterator pointing at the 2nd tree model node
and make us return FALSE mistakenly.
https://bugzilla.gnome.org/show_bug.cgi?id=679910
This bug is resolved by fixing two things in
gtk_tree_model_filter_row_deleted():
(1) It is possible for an elt to have elt->visible_siter == NULL, when
it is deleted. Only call g_sequence_remove() if this pointer is
non-NULL.
(2) For the case len (level->seq) > 1, free the elt->children level
if non-NULL. Failing to do this means the level will stick around.
If this child level was not referenced, it will still have a zero
ref count on its parent which cannot be removed!
For both bugs unit tests have been added in the preceding commit.
gtk_tree_model_filter_free_level() should always release ref on parent
unless the parent has been removed in the child model. The unit tests
added in the previous commit test this.
If the level to be reordered did not contain an elt with offset == 0
(for example if that node was hidden), the first reference was not
correctly transferred.
A unit test for this has been added in a previous commit.
This is due to the special handling for filter functions that depend
on child level state. If the parent level of a level is the root level,
the level cannot be removed from the cache. Any change in this level
can cause its parent in the root level to become visible again.
The most important change is that the function now properly releases
any external ref count and propagates these changes to the child model.
If a node is removed due to a filtering action, we now properly release
all external reference counts for its hierarchy of child nodes.
Apart from that, the function has been restructured to remove code
duplication.
Finally, there is still some doubt whether there are more calls to
gtk_tree_model_filter_free_level() which need unref_external set to TRUE.
Should be helpful when debugging this thing. Perhaps this needs to
be properly integrated in G_DEBUG/GTK_DEBUG, but currently I do not see
the need to.
There was still a discrepancy between the comments and the code about when
exactly a level was to be removed. The rule is now set on that for a level
to be removed its zero_ref_count as well as the zero_ref_count of its
parent has to be zero.
Significantly improves performance when e.g. removing (filtering) a lot
of rows from the filter model. Fixes bug 616871.
This commit includes changes by Kristian Rietveld to make the patch apply
on top of the treemodel-fix branch and pass all newly written unit tests.
- Before we kept a reference on all nodes in non-root levels. This has
been changed, now we keep a reference on the first node of each level.
If, due to changes in the model, another node becomes the first node in
the level, the reference is transferred to this new first node.
- All non-root levels keep a reference on their parent.
- By making use of the external ref count, the filter model now emits less
unnecessary signals.
- GtkTreeModelFilter does support filter functions which decide visibility
of a given node based on the number of or visibility of children.
To accomplish this, a child level of a node is cached when its
parent has an external ref count > 0, because changes to the node might
affect this parent.
- An optimization for not building the root level in case the inserted
node is not visible in gtk_tree_model_filter_row_inserted() has been
removed. In this case, we still need to build the root level and
possibly a child level to monitor for signals which might make
this row visible.
In gtk_tree_model_filter_check_ancestors(), also handle the case when
a node is already in the cache, but invisible, in the root level.
With the upcoming changes to GtkTreeModelFilter's ref counting this
case can occur.
We need to distinguish between the ref count objects have on us versus
the ref count we have on our child model. To keep track of the former,
we introduce the "external ref count" in this commit. The zero_ref_count
needs to be determined from the external ref count, because objects that
have a ref count on us have say in which levels must be cached and which
can be released.
Before the caching in GtkTreeModelFilter was essentially broken and
levels were never released. This was caused because the zero_ref_count
was connected to the ref count the filter model had on its child model.
Now that this depends on the external ref count, this is working fine and
as to be expected.
The row-deleted signal should be emitted after the internal data
structures have been updated. In gtk_tree_model_filter_remove_elt_from_level
and gtk_tree_model_filter_virtual_root_deleted the signal was still being
emitted before the updates were carried out.
The bulk of the fix is to walk the chain of ancestors, starting at the
root level, and check if the visibility of any of the ancestors has
changed. If yes, the necessary signals are emitted so that this change
is propagated properly. This walk is done after a node has been
inserted, changed or deleted, see function
gtk_tree_model_filter_check_ancestors().
Bug reported, and initial debugging and analysis, by Xavier Claessens.
Suggested by Xavier Claessens / bug 621076.
(Additional obseration: this should speed up the filter model's
handling of row-inserted as a binary search is now used instead
of a linear scan).
gtk_tree_model_filter_row_deleted was emitting ::row-deleted while
the internal state of the model was still in disarray, causing
segfaults e.g. when mapping the file chooser with accessibility
turned on. This is just a bandaid fix, and doesn't try address
any of the deeper problems of the filter model code.
I did take the time to reduce rampant variable shadowing in that
function, though.
