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

[Tristan Van Berkom <tristanvb openismus com>]

First sorry for the delayed reply... lets just say that
rome was not built in a day ;-)

On Wed, 2010-09-29 at 21:25 +0200, Kristian Rietveld wrote:
> On Sep 23, 2010, at 10:56 AM, Tristan Van Berkom wrote:
> > So to help stay on track without straying too too much, these
> > are (my perceived) reasons for the said refactoring work:
> > 
> >  - Code sharing: A good refactoring of cell layouting logic 
> >    into some classes that can be (more) easily reused by the 
> >    major candidates GtkTreeView, GtkIconView and GtkComboBox[Menu]. 
> > 
> >  - Adaptation for height-for-width support (currently only
> >    the GtkComboBox uses hfw'ness of cell renderers, big patch
> >    pending on treeviews could be better written based on the
> >    refactored code).
> > 
> >  - Alignment of GtkComboBox menu renderers currently cant be
> >    done as described in bug 629133.
> > 
> >  - While we're on the topic of refactoring the cell layouting
> >    code, I'd personally like to open up new avenues for rendering
> >    cell layouts in more dynamic ways (as I'll describe in more 
> >    detail below); Rationale for this is if we're going to give this
> >    a go at all, we better be ambitious and awesome about it.
> 
> Layouting cells in more dynamic ways is one reason; for me another big reason is to also make column/row layouting (thus the layouting of cell layouts ;) more flexible.  Think of:
> 
>  - Making it easier to create widgets such as icon view, Windows-style columnary list view, etc.  Basically, you only want to write the algorithm laying out the rows and re-use the algorithms that layout cells.
> 
>  - For list and tree views, make row and column spanning possible.
> 
>  - Proper implementations of separator rows, group headings or other custom content.  (More on that below).
> 
>  - Possibly embedding non-cell content in list/tree views, such as custom widgets.  Most notably, for example a button (see mobile platforms).
> 
> 
> > GtkCellArea
> > -----------
> >   A GtkCellArea would be an abstract class which implements 
> > GtkCellLayout and acts as a collection of cells which are rendered 
> > in a completely undefined way, GtkCellArea classes would be responsible 
> > for handling all geometric tasks in the laying out of cells:
> > 
> >  - Allocation of cell positions inside the area 
> > 
> >  - Returning overall bounderies that the cells take up inside an
> >    area as well as returning the bountries of a single cell inside
> >    the area.
> > 
> >    This is important for widgets like TreeView and IconView
> >    who want to draw focus around a cell, a column or an entire row.
> > 
> >  - Returning the cell at a specified coordinate inside the area for
> >    a given treemodel row.
> > 
> >    This is needed for the sake of handling mouse-clicks so that the 
> >    owner of the CellArea can go on to start editing a given cell, or
> >    activate an icon in a given cell.
> > 
> >  - Responding to height-for-width/width-for-height apis for a 
> >    collection of cells for a given treemodel row.
> > 
> >  - Actually render the GtkCellArea for a given row into a given 
> >    space/GdkRectangle.
> > 
> > Conceptually, the GtkCellArea is for a cell renderer what a 
> > GtkContainer is for a GtkWidget. The main bonus of keeping the 
> > GtkCellArea as an abstract class and responding to some generalized 
> > apis is it opens up avenues for more complex implementations such as 
> > a tabular CellArea, or a cell-wrapping CellArea which could be more 
> > easily implemented in the future.
> 
> At first, I thought this would be an entity for rendering one "row", so one set of cells (basically, what cell view does currently).  
> But I think you mean to have GtkCellArea manage multiple rows?  
> I leave whether or not GtkCellArea should manage one or more rows in the middle below.

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.

But it's a good question still.

> 
> An "object" that handles cell layout for a single row is what I kind of ended up with in a previous refactoring.  
> This was not really an object, as it was a set of internal functions.  
> What I did was refactoring the cell layouting code in gtktreeviewcolumn.c into a manageable set of functions that
>  implemented an iteratable interface over a list of cells.  Turning something like this into a stand-alone class 
> (and enable subclassing) sounds like a good idea to me.
> 
> One of my original ideas was to introduce the concept of a "row renderer".  This would be an abstract class with several
>  subclasses.  One row renderer subclass for rendering a row consisting out of cell renderers, re-using the original 
> cell layouting code.  Other "row renderers" for embedding widgets, separators, group headings, etc.  Layouting row 
> renderers would then give you your full-featured tree view, icon view, etc.  
> (Though combining an icon view and a group heading row renderer would mean combining tabular and "list" layout -- 
> another puzzle to solve but an interesting feature).
> 
> To go back to the proposed GtkCellArea:
> 
>  - I am not yet sure if this should be tied to GtkTreeModel.  The main consideration is if you want to use 
> GtkCellArea without a GtkTreeModel.  Though it seems cumbersome to provide some other means to populate the
>  properties of the cell renderers in this case ...
> 
>   Another consideration is whether GtkCellArea will manage one or multiple rows.  If only one, then you 
> might want to have another object set the data on the cell renderers (the data is "pushed" by another object).  
> If multiple, then directly accessing the model seems a better fit (GtkCellArea "pulls" the data by itself).

Yes I had not considered that but I suppose it's a better fit to have 
GtkTreeViewColumn push the object properties to the GtkCellArea before
rendering (I might have done it the other way around).

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

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

> 
> > The CellAlignment helps in general to optimize the overall size requests
> > of treeview rows, however where performance is not an issue; the use of
> > CellAlignment by the GtkCellAreaBox can be optional, in this way 
> > allowing cell renderers to "flow naturally" across a GtkTreeView column
> 
> Yes, such natural flowing of cell renderers in a column should definitely be supported.
> 
> > GtkCellAreaManager
> > ------------------
> >   This would essentially be a cleaned up API and refactored version of
> > the current GtkRBTree. The rbtree currently caches information about how
> > a GtkTreeView is currently rendering a given model, it caches things
> > such as which rows are expanded; the indentation level used by the
> > treeview and most notably the variable height of rows.
> > 
> > Some things that would change in the GtkCellAreaManager would be:
> > 
> >  - Rip out GtkTreeView specific information such as expanded rows
> >    and indentation.
> > 
> >  - Add a client data pointer to be defined by the user which describes
> >    what kind of data is cached in a row (all the treeview specific
> >    row cached stuff comes back here)
> 
> I assume that such a pointer will be placed in GtkRBNode.  You mention the expanded rows and indentation as tree view specific stuff above.  Note that this data is not stored in the GtkRBNode fields.  Expanded rows are deduced from the RBTree structure, simply because collapsed rows are not present in the RBTree.  Indentation is calculated based on row depth (which is in turn taken from the RBTree structure again).  What is stored in the GtkRBNode, is the state of the expand/collapse animation (the expander animation).
> 
> What has been requested as feature in the past is to save the selection and expansion state of rows that are being collapsed.  For example, if you collapse a row and then expand it again, the selection will appear as it was before you collapsed the row.  The same for expansion state of child rows.  If we were to provide such a feature, RBTree would need to see changes.
> 
> >  - Provide a configurable iterative process for updating the cache,
> >    this would be performed synchronously for all the visible rows and
> >    then fall off into the background only updating a configurable
> >    ~1000 rows at at time and then marking them "validated" for access.
> 
> This would basically be moving out as much of GtkTreeView's validation code as possible into this class.
> 
> > How does this all map to current GtkCellLayout implementations ?
> > 
> > Most of the above text referrs to GtkTreeView, being the most
> > complex configuration making use of these classes.
> 
> Whereas the RBTree works very well for lists and trees, are we sure that this will work nicely and 
> performant for tabular layouts as well?  Or would a different data structure be a better choice?
> 
> If it is useful to re-use RBTree in several widgets, then we should likely do it in a similar way 
> as you proposed with GtkCellAreaManager.  However, more study might be required as RBTree is quite a 
> central element of the tree view.  Because it is so central, we should likely also take performance into account.
> 
> Have you given any thought to selection handling?  
> GtkTreeSelection is now only used by GtkTreeView, you would also want to generalize that.  
> Currently, selection state is stored in GtkRBTree, so GtkTreeSelection and GtkRBTree are again 
> intertwined with each other. Perhaps GtkCellAreaManager is also a good choice for placing the 
> selection handling code, possibly made extensible/adaptable through a delegate object?
> 
> > Conclusion: There is a plethora of requirements relating to
> > GtkTreeView in general, some of which I'm sure to have missed,
> > some of which will surely present themselves as road-blocks
> > while implementing the said refactoring.
> 
> It's a huge task, which I think is (unfortunately) impossible to get done before 3.0.  
> If we do something like this, I think  we also have to decide on some kind of compatibility 
> with current GtkTreeView. Getting there throughout the 3.x releases would be cool though.
> 
> I will try to look for my notes on tree view refactoring which I wrote a long time ago and 
> see if there were more ideas that could fit in nicely with what has been proposed so far.  
> While pencil and paper is great for brainstorming, paper has a tendency to get lost :)

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.

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

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

Cheers,
    -Tristan

> 
> 
> regards,
> 
> -kris.