Re: GtkTreeView Refactoring Considerations [was Re: Private types inside GTK+]

[Tristan Van Berkom <tristanvb openismus com>]

On Mon, 2010-10-25 at 17:26 +0200, Kristian Rietveld wrote:
> On Sat, Oct 23, 2010 at 9:44 AM, Tristan Van Berkom
> <tristanvb openismus com> wrote:
> > I'm a few days into this and I've written up a GtkCellAreaClass and
> > started out implementing an orientable GtkCellAreaBoxClass.
> >
> > An initial problem here has to do with pushing data to the GtkCellArea
> > instead of pulling it from the model, do we really want to be able to
> > use GtkCellArea to render cells for data that is not coming from a
> > GtkTreeModel ? (is it worth the trouble ?)
> 
> I have always been relatively undecided on this and just meant to
> bring up that you can distinguish between the two methods and one will
> have to be chosen.  Adding to this that I haven't come up with a
> concrete case yet wherein using GtkCellArea or GtkCellAreaBox without
> a tree model could be useful.
> 
> Another thing that this depends on is whether cell renderers are
> shared amongst GtkCellAreas in the same GtkCellAreaBox and whether
> GtkCellArea implements GtkCellLayout.  From your e-mail I understood
> that GtkCellArea indeed implements GtkCellLayout, does this mean that
> each GtkCellArea will need its own GtkCellRenderers to render?
> 
> Would it be possible and make sense to share GtkCellRenderers amongst
> GtkCellAreas that are contained in the same GtkCellAreaBox?  This
> stays kind of close to the model that we currently have and you could
> say that GtkCellAreaBox would pull the data and push this into a
> GtkCellArea.  In this case GtkCellArea would not implement
> GtkCellLayout, but GtkCellAreaBox would.

Hmm seems I didn't communicate this clearly enough, GtkCellArea
is a base abstract class, and GtkCellAreaBox is the first concrete
subclass of GtkCellArea... this allows treeviewcolumn (and other
layouts) to communicate to the GtkCellArea over some generalized
apis (i.e. a GtkCellArea does not contain any GtkCellAreaBox...
together they contain GtkCellRenderers).

Currently I have it setup so that the GtkCellArea class has
->add()/->remove()/->forall() virtual methods that are expected
to be implemented by concrete subclasses.

Since I've went with the "pull data from a GtkTreeModel/GtkTreeIter"
api style... already I've implemented:
  - gtk_cell_area_connect_attribute()
  - gtk_cell_area_disconnect_attribute()
  - gtk_cell_area_apply_attributes()

>From GtkCellArea base class which requires no cooperation from 
the subclass GtkCellAreaBox (the box only needs to manage it's
list of renderers and some "expand"/"GtkPackType" attributes,
then it needs to do it's geometry apis/->event()/->render() etc).

I'll publish my branch to the git later today, it doesn't do
much yet but should give a clearer picture of the APIs I'm playing
with.

Also yesterday I gave alot of thought to how height-for-width
geometry is going to play out in treeviews with regards to cell
alignments and generally overall size of a GtkCellArea when it's
size is requested for an arbitrary number of rows.

I'm thinking of trying something like:

/* Subclass creates a class specific 'GtkCellAreaIter' which
 * can be used to accumulate cell alignments and the overall
 * sizes when requested for multiple rows
 */
GtkCellAreaIter *gtk_cell_area_create_iter (GtkCellArea *area);

Then the request apis get an additional 'iter' context argument,
it could be for instance that one might use a different 'iter'
for different depths in the treeview (or to align cells in
a combo-box menu together in a per submenu context).

gtk_cell_area_get_preferred_width (GtkCellArea *area,
                                   GtkCellAreaIter *iter,
                                   GtkWidget *widget,
                                   gint *min_width,
                                   gint *nat_width);

The above api would store the resulting overall requested
width (and cell alignments) in the iter which was created
by the specific GtkCellArea subclass... however it would
return a *min_width and *nat_width which is only contextual
to the GtkCellArea for the specific row-data that is currently
applied. 

Then later the caller can call:

gtk_cell_area_iter_get_preferred_width (GtkCellAreaIter *iter,
                                        gint *min_width,
                                        gint *nat_width);

Which would return the overall minimum and natural width of 
a said GtkCellArea after having iterated over a number of rows.

With GtkCellAreaIter we:

  - Cleanup the way that GtkTreeViewColumn bumps its requested
    width to be the maximum of all previous calls and store that
    data separately.

  - Store any alignments of cells in the way that the subclass
    (GtkCellAreaBox) was configured to do.

  - As mentioned above, also allow different alignment contexts
    to span groups of rows (most plausibly grouped by treemodel
    "depth").

Have to run catch a train... I'll push my code later and probably
start writing the GtkCellAreaIter stuff later today... (or on
the train).

Cheers,
     -Tristan


> 
> > Currently this presents a few problems, which are not
> > insurmountable but still weird to handle since the whole thing
> > is tied heavily into GtkTreeModel already:
> >
> >      a.) We cant do gtk_cell_layout_set_cell_data_func() from
> >          inside the GtkCellArea, that stuff would have to be done
> >          from a GtkCellLayout implementation aside from pushing
> >          data to the GtkCellArea.
> 
> Right, I guess this is because data needs to be pushed to the
> GtkCellArea whereas in order to be able to call the CellDataFunc you
> need to pull the data (in GtkCellArea) first?  This should be the
> responsibility of the pusher, and in case we decide to mark
> GtkCellAreaBox to be the pusher, this is not a problem, right?
> 
> >      b.) gtk_cell_renderer_activate() takes a "path" argument;
> >          I think it would be better if this were a gconstpointer
> >          user_data argument in any case that CellArea was rendered
> >          for something that is not a GtkTreeModel row... If I continue
> >          with the "pushing" data approach, this will likely turn into
> >          a "detail" string argument to gtk_cell_area_event().
> >
> > On the other hand, if we pull data from GtkTreeModel/GtkTreeIter we
> > get to easily reuse/share code that actually pulls data from
> > GtkTreeIter and pushes it to the underlying cells (as that would
> > be handled by GtkCellArea), and also we have less api churn to deal
> > with, since treepaths still make sense in GtkCellRenderer:activate
> > signals.
> 
> Yes, that makes sense.
> 
> >   - GtkCellLayout->pack_start()/pack_end() should be deprecated
> >     in favour of ->add()/->set_child_property(), since those apis
> >     can only be implemented from a GtkCellAreaBox (but dont make
> >     sense for say; a GtkCellAreaTable or GtkCellAreaGrid).
> 
> Makes sense.
> 
> > All-in-all I'm not sure that the "push data to the CellArea"
> > thing is going to be worth while, I'm considering changing
> > this to a "pull data from the model" approach which seems to
> > fit the current GTK+ codebase a lot easier.
> 
> Yes, perhaps pull is indeed the way to go, or have GtkCellAreaBox pull
> and push this into GtkCellAreas as I described above.  Not sure if you
> agree with that or whether that aligns with what you had in mind.  An
> advantage of this could be that GtkCellAreaBox would be a
> straightforward refactor of the current GtkTreeViewColumn as all the
> layouting algorithms/calculations can be re-used.  For a start, we
> would keep the same code/semantics for the cell_area, background_area
> calculations (taking the h/v separator into account, focus line width,
> etc.).This should make it easy to port GtkTreeView* to this without
> breaking the current way of layouting, so that this can be feasible
> material for 3.0.
> 
> Later we can start changing cell_area etc. calculations, either in the
> same GtkCellAreaBox class or in a new one.  You could perhaps make
> GtkTreeViewColumn use a different GtkCellAreaBox subclass for each
> column.
> 
> 
> regards,
> 
> -kris.