Re: GailTreeView Changes Proposal

[Brian Cameron <Brian Cameron sun com>]

Marc:

> I'm working to change the way in which we store column and row information 
> in GailCell objects which represent cells in treeviews as per my discussion 
> with Brian this morning.  My proposed changes involve the following:
> 
> 1. GailCell's data members and code will remain the same.
> 
> 2. A base class called GailTreeViewCell will be implemented.  This class 
> will contain the GtkTreePath and GtkTreeViewColumn information.  This 
> information provides access to the cell's column and row information such 
> that changes in the ordering of rows or columns in the tree view don't 
> invalidate this information.

I think this will work well.  With one exception.  I just discovered that
it is better to use a GtkTreeRowReference rather than a GtkTreePath.
The reference can rebuilt an appropriate GtkTreePath value even if the
row is moved around due to row adds, deletes, or reorders.  This saves
us from having to listen to the row_insert, row_delete, and row_reordered
signals and recalculating the GtkTreePath ourselves.  See the 
get_row_description, set_row_description, get_row_header, and
get_column_header functions in GailTreeView to see examples of using
the reference.  I just added the code using the references a few minutes
ago.

Also, I just realized that we will have to keep the column and row 
values as gints in addition to storing the GtkTreeViewColumn and
GtkTreeViewColumn information.  Since we're leaving the row/column gint
values in GailCell, this should not be a problem.

The reason GtkTreeView still needs the gint row and column values is
because we want every GailCell to have two properties:  

  defunct
  stale
  
defunct means that the cell has been removed via a row delete or hidden
via a row collapse or because a column has been made invisible.

stale means that the cell is still visible, but that the index to the
cell has changed.  A row-reorder or an insert/delete on a row befor
 the cell would cause this scenario.
 
In other words the GailTreeViewTextCell will need to listen to the
GtkTreeModel's  "inserted", "deleted" and "reordered" signal and check
to see if the row number has been changed.  In order to check this we
will need to cache the row number as a gint as well as keeping track
of the GtkTreeRowReference.

Also, the GailTreeViewTextCell object will need to listen to the
GtkTreeModel's "columns-changed" signal and the "visible" property
for all the GtkTreeView's columns.  Each time one of these signals
happens, we have to loop through the GtkTreeView's GtkTreeViewColumn
pointer array and find out whether it's column number has been
changed.  In order to check this we need to cache the column
number as a gint as well as keeping track of the GtkTreeViewColumn 
pointer value.  You can see how GailTreeView is handling it's
"col_data" value (refer to the _gail_tree_view_columns_changed()
function) to see how it handles the same situation.

Let me know if this is not clear.

> 3. GailTreeViewTextCell will derive from GailTreeViewCell rather than 
> GailCell.  Future implementations of GailTreeViewBooleanCell and 
> GailTreeViewTextPixbufCell will also derive from GailTreeViewCell.
> 
> Currently, when the registry is passsed a GtkCellRenderer, it returns the 
> factory which creates GailCell objects.  My proposed changes would require 
> the registry to be changed to return the factory for GailTreeViewCell 
> objects when passed a GtkCellRenderer.

This is correct.  Now that I'm thinking about it, this idea works well
as long as GtkTreeView is the only thing using GtkCellRenderer.  Looking
at the GTK code, this is currently the case, but I'm not sure it's
guaranteed to stay this way.

Perhaps a safer method would be to leave the hierarchy the same, but
add a void * pointer to the GailCell object and just have the 
gail_tree_view_table_ref_at function malloc a structure with the needed
data (the GtkTreeViewColumn pointer and the GtkTreeRowReference).  The
gail_cell_init() function could take a (void *) userdata argument.
This might be a more generic solution.

If we do this, then the GailTreeViewTextCell (and any peers) would need
a finalize function to free this data.  You can refer to the
gail_tree_view_finalize() function in GailTreeView for an example of
setting up such a finalize function.
 
However, there is still the issue of setting up the callback functions
that are needed by the various GailCell's that are used by GailTreeView.
Since these cells need to support the "defunct" and "stale" signals that
I described above, they need to define callback functions that are 
listening to the signals.

However, the problem is with this code in the gail_tree_view_table_ref_at
function:

    cell = atk_object_factory_create_accessible (factory,
                                                 G_OBJECT (renderer));
    g_return_val_if_fail (GAIL_IS_CELL (cell), NULL);
    gail_cell_init (GAIL_CELL (cell), GTK_WIDGET (view), row, column);

The problem is that the GailTreeViewTextCell is created when 
atk_object_factory_create_accessible is called.  And that is when it's
instance initialize functions are called.

But the GailCell->widget value isn't set until the gail_cell_init function
is called a few lines later.  I'm not sure how to fix this problem.  Perhaps
the "gail_cell_init" function should be defined in the class so that
the GailTreeViewTextCell can override it...and therefore it can be passed
the handle to the GtkTreeView, set it's signal handlers, then pass the
values up to the GailCell (parent's) init function.  This seems the most
straightforward solution to me.

> There are a couple possibilities as to how the subclasses of 
> GailTreeViewCell should be created.  One possibility is that the 
> GailTreeViewCell factory could create the correct type of subclass based on 
> the renderer passed to it.  The other possibility is that the factory for 
> GailTreeViewCell could just do basic initialization, and leave the 
> cell-specific initialization to the ref_at function, or whatever function 
> is returning the ATK_TABLE_CELL object to the caller.  My vote is for the 
> factory to do the right thing, but this may have negative implications that 
> I haven't thought of.  I think this makes sense though, since a 
> GtkCellRenderer is only ever used in a GtkTreeView, and creation of a 
> GtkTreeViewCell by itself without deriving a subclass for the specific cell 
> type doesn't make sense.

I think the best solution is to make the "init" function a function that
the subclasses can override.  That solves all the init problems.

The factory currently generates a GtkTreeViewTextCell.  I think this
is the correct behavior.  I don't see that whether the parent of
GtkTreeViewTextCell is a GailCell or a GailTreeViewCell really matters.

However, I'm starting to think that adding a GtkTreeViewCell into the
hierarchy may be a bad idea.  It assumes that only GtkTreeView uses
renderers which I think might not be something we can count upon. 
Therefore, storing a "userdata" in the GailCell as I described above
might be a reasonable compromise.

Anyone else have thoughts on this?

Brian