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

[Tristan Van Berkom <tristanvb openismus com>]

On Tue, 2010-10-12 at 14:51 +0200, Kristian Rietveld wrote:
> On Thu, Oct 7, 2010 at 6:41 AM, Tristan Van Berkom
> <tristanvb openismus com> wrote:
> > I was thinking that a GtkCellArea would only render a single row
> > (actually, a row in a treeview can be composed of several GtkCellAreas,
> > each treeview column would use exactly one cell area to abstract a lot
> > of the column code).
> >
> > Now that I look at this again I'm not sure if the GtkCellArea should
> > store any information about previous and following rows
> >  - should the alignment of cells be stored by the GtkCellArea itself ?
> >  - or should it be externalized to some parenting code that pushes
> >    the alignment data to the area ?
> >
> > I think that the former seems more appropriate since only the CellArea
> > knows how cells can be aligned for a particular implementation, and
> > the initial width/height of cells in the area is also determined by
> > the CellArea.
> 
> I agree that the former seems more appropriate.  This also paves a way
> for doing more flexible layouting in tree view rows.  However, I guess
> that we also want to support how cells are laid out right now in
> columns and this requires some separate entity to maintain the
> alignment and widths for a set of cell areas.
> 
> >>  - GtkCellArea (or a subclass) might also have to implement the custom "event-handling" and focus drawing
> >> code that is currently in GtkTreeViewColumn.
> >
> > I think an abstraction api for event handling is simple enough and a
> > good fit for a single-row handling GtkCellArea.
> >
> > gtk_cell_area_event (GtkCellArea  *cell_area,
> >                     GdkRectangle *area,
> >                     GdkEvent     *event);
> >
> > This would be implemented by CellArea implementations
> > and basically forward the event to the appropriate cell
> > renderer according to the area's layout.
> >
> > I suppose the method for invoking this by treeview for
> > instance would be:
> >   - Get the appropriate GtkTreeViewColumn for the event->x
> >   - The GtkTreeViewColumn would then get the appropriate GtkTreeIter
> >     for event->y
> >   - The GtkTreeViewColumn would then "push" the data for that
> >     column into its delegate GtkCellArea.
> >   - Then the GtkTreeViewColumn calls gtk_cell_area_event().
> >
> > Sounds approximately right ?
> 
> It sounds interesting to me and will likely work.  Do note that
> currently things are done differently, tree view makes the function
> call to "push" the data (though this function is a tree view column
> function, so must be possible to refactor this).  Furthermore,
> currently not GdkEvents are pushed to the cell renderer, but rather
> the "activate" or "start-editing" method on the cell renderer is
> called.  I think this lines up well with the plan to have the GdkEvent
> only reach the cell area.
> 
> > I think that focus painting in this case should be left to the treeview
> > and the GtkCellArea continues to return "bounds" of the overall used
> > space for it's dedicated GdkRectangle (because the treeview sometimes
> > like's to paint focus which spans the entire treeview row... which
> > may span several GtkCellAreas for each column; each of which may
> > have a different "bounds" in height).
> 
> There's also a case in which focus is drawn around a single cell
> renderer (that is part of a column together with other cell
> renderers).  So this means that either the cell area can draw this
> focus, or the exact coordinates of this cell can be acquired from the
> cell area.
> 
> > Ok good points, you'll notice I left out GtkCellAreaManager comments
> > here...
> >
> > I think you're right that the whole big picture is much and probably not
> > reasonable to get done by 3.0.
> >
> > The best move I think is to break this down into iterations, since my
> > immediate concerns are about actual rendering and alignment of cells
> > (i.e. achieve height-for-width treeviews and alignment of cells in
> > combo boxes)... hopefully I can come up with something good and
> > extendable for the GtkCellArea.
> 
> Yes, we should definitely break this down into iterations.  Starting
> with pushing the cell renderer lay out code from GtkTreeViewColumn
> into something like a GtkCellArea sounds like a good start to me.
> 
> >
> > The concept of row separators and group headings will need some
> > thought, I'm not sure it makes sense to chose a different GtkCellArea
> > class to render a different row, these cases may be better handled
> > as general functionality of the base class (which could be overridden
> > by subclasses to achieve a different overall look-n-feel of the
> > treeview).
> 
> Definitely requires more thought and can, as far as I can see, be done
> separately from GtkCellArea.
> 
> > I also wonder with some of the offscreen rendering magic recently added
> > we can even get GtkButtons to be virtually embedded into a GtkCellArea
> > subclass (ofcourse without allocating a separate instance of the
> > GtkButton for every row of data being the challenge)... by simply
> > rendering the button onto every row...
> 
> Interesting idea, and since you want to make it possible to push
> GdkEvents into GtkCellArea, the button will also behave appropriately.
>  (The event handling was the main issue why you couldn't do something
> like a GtkButton in a GtkCellRenderer).
> 

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 ?)

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.

      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.

A few other thoughts/notes I have so far is:

   - GtkCellArea is *not* a GtkCellRenderer, it might have been nice
     if it was, and then GtkCellArea could have been added to another
     GtkCellArea and treated like a cell (like GtkWidget/GtkContainer
     does more properly)... however that's lots of API churn and
     probably not worth while if we can already define CellArea 
     subclasses that position CellRenderers in more dynamic ways.

   - GtkCellArea should implement "child properties" like GtkContainer
     does, this will be particularly useful for defining treeviews
     from GtkBuilder xml (but not useful for property notifications
     and the like)... note there is still no way to tell a treeview
     column that a renderer wants to expand or not via GtkBuilder xml.

   - 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).

   - Currently I have the base class GtkCellArea implementing
     GtkCellLayout, not sure if that should be kept or not, maybe
     its better to have only a clean class to be used from
     GtkCellLayout implementations as a delegate, or maybe its
     convenient to also communicate with the CellArea over the
     GtkCellLayout interface pathways... anyway I can remove that 
     later in the game.

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.

Thoughts ?

Cheers,
         -Tristan