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

[Tristan Van Berkom <tristanvb openismus com>]

On Tue, 2010-10-26 at 18:32 +0900, Tristan Van Berkom wrote:
> On Tue, 2010-10-26 at 16:54 +0900, Tristan Van Berkom wrote:
> > On Tue, 2010-10-26 at 09:23 +0200, Kristian Rietveld wrote:
> > > On Tue, Oct 26, 2010 at 3:28 AM, Tristan Van Berkom
> > > <tristanvb openismus com> wrote:
> > > > On Mon, 2010-10-25 at 17:26 +0200, Kristian Rietveld wrote:
> > > > 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).
> > > 
> > > Aha! Sorry for misunderstanding.  But then what I said about
> > > GtkCellAreaBox applies to GtkCellArea in general and thus the pull
> > > model is indeed the easiest to go with now.  I understand how you got
> > > into trouble with the CellDataFuncs and also this is solved by going
> > > pull.
> > > 
> > > > 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()
> > > 
> > > What exactly do these functions accomplish?  Are connect/disconnect
> > > similar to the current add/remove attribute functions?  Typically we
> > > work with one set of attributes per GtkCellLayout, or is this
> > > something you want to change?
> > 
> > They are exactly that, they manage attributes for the GtkCellArea,
> > which is currently a GtkCellLayout, I chose different names because
> > they would clash with the GtkCellLayout api names.
> > 
> > I'm not sure if in the long run these apis should/will be exported
> > at all, maybe its best to just say the GtkCellArea is a GtkCellLayout
> > and use the appropriate GtkCellLayout apis (however GtkCellLayout
> > doesnt have the ->remove_attribute()... which could be added).
> > 
> > > 
> > > > 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).
> > > 
> > > Yep, so we can move all the gory TreeViewColumn details to
> > > GtkCellAreaBox.  I can also do that for you when the GtkCellArea API
> > > takes shape as I am pretty familiar with the GtkTreeViewColumn code
> > > and can perhaps also merge in some of the refactorings (that replace
> > > gtk_tree_view_cell_process_action() with something much saner) I have
> > > on my disk somewhere while at it.
> > > 
> > > 
> > > > 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.
> > > 
> > > Okay, so you need to make sure the specific row data is set first.
> > > (If needed, we could have a likewise iter creation function that also
> > > takes a range of GtkTreePath (start and end) as argument).
> > > 
> > > Thinking in TreeView terms/compatibility here: the
> > > get_preferred_width() function will be very useful in getting an
> > > initial width for TreeViewColumns, or get the preferred width when
> > > doing "auto size".  I think right now "auto size" will make sure that
> > > "all" contents can be visible (so the width of each column is big
> > > enough for each possible row), but perhaps it makes sense to change
> > > this to the preferred width or have a mode toggle for this somehow.
> > > 
> > > Outside of that, the width of a TreeViewColumn is mostly fixed.  Also
> > > because a width can be mandated by the user through manual resizing
> > > (and this requirement is trickier than it might appear at first).
> > > 
> > > How exactly will height handling work?  Height is actually "unbouded"
> > > in tree view, so I guess get_preferred_height() on the GtkCellArea
> > > will not be of use for GtkTreeView, but can be of use for other
> > > layouts of course.  Or will get_preferred_height() return the
> > > preferred minimum height for the scrollable window?
> > 
> > get_preferred_height() is generally only useful for other layouts,
> > possibly GtkIconView can be orientable and decide whether it will
> > query it's content in HEIGHT_FOR_WIDTH or in WIDTH_FOR_HEIGHT mode.
> > 
> > For uniformity its generally nice when content is at least capable
> > of handling either of the requests.
> > 
> > > 
> > > Another idea is that we can possibly make the row validation scheme in
> > > GtkTreeView more efficient (or at least have a slightly cleaner
> > > implementation) by having GtkCellArea measure multiple rows at once
> > > using your API above instead of calling validate_row() separately for
> > > each row.  Though there is a case where we might need efficient single
> > > row measurements from GtkCellArea.  This is in
> > > validate_visible_area(), which as the name might suggest could be
> > > fully handled by GtkCellArea and a range of visible rows instead, but
> > > there is some very intricate scrolling code inside here.
> > > 
> > > I know the validate_row()/validate_visible_area() code by heart, so
> > > also in this area I can help out by doing this migration.
> > 
> > I'm grateful for your interest in helping out in these sticky areas :)
> > 
> > I still havent published the branch but will try to do so in the next
> > couple of hours so you can at least take a look at some actual code.
> > 
> > > 
> > > Finally, we also need support for scrolling support.  Basically
> > > GtkCellArea needs the possibility that the first row is only partially
> > > rendered, so GtkCellArea starts rendering a range of rows at a row +
> > > pixel offset.
> > > 
> > > 
> > > > 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.
> > > 
> > > I am not really sure how we are going to change this.  Is this just
> > > about the size requisition phase or in general?  If in general, would
> > > the tree view start shrinking columns if needed?
> > 
> > Yes this is all just for the requisition phase, It could not shrink
> > columns as that would require caching the aligned width of every cell
> > for every treemodel row that has had it's size requested (I dont think
> > such caching is really what we want to do... but if we want to cache
> > every size for every row, then we would be able to more quickly shrink
> > the request of a column based on the data change of a single row).
> > 
> > To make sure we're talking about the same thing, and for readers dont
> > know the treeview code that well, the above refers to a situation where
> > a treeview is populated and has a size allocated, but a row data
> > changes. At that point in time it's interesting to GROW_ONLY and avoid
> > recalculating the entire width of the column for every row all over
> > again.
> > 
> > > >  - As mentioned above, also allow different alignment contexts
> > > >    to span groups of rows (most plausibly grouped by treemodel
> > > >    "depth").
> > > 
> > > Depth is likely the best attribute to do grouping on.  I guess the
> > > only alternative would be ranges defined in terms of GtkTreePath, but
> > > that is much more hairy.
> > > 
> > 
> > Right, it's actually possible to add some layer that goes over portions
> > of the model by using a GtkCellAreaIter and doing repeated requests, I
> > haven't explored that yet... GtkCellAreaIter as I see it will be a
> > context handle for a group of requests which can be observed/updated
> > by the implementing GtkCellArea[Box], this way some higher level code
> > or the layout itself will ask the GtkCellArea to create a
> > GtkCellAreaIter for a series of requests it plans to make, and then
> > use the GtkCellAreaIter api itself to observe the collective results
> > along the way.
> > 
> > Another interesting thing is that the overall widths and heights
> > gathered by the GtkCellAreaIter are observable via properties
> > and signals (so for instance in a fixed-height treeview, one
> > can perform a request on a single row which changed size and 
> > only re-allocate the view if a notification of the overall
> > width changes, or if the overall height-for-allocated-width
> > changes).
> > 
> > I'll push the branch soon which has the new/half-implemented classes,
> > thanks a lot for looking into this with me :)
> > 
> 
> I just pushed the branch 'treeview-refactor', it currently contains
> (unfinished) classes:
>    - GtkCellArea
>    - GtkCellAreaBox
>    - GtkCellAreaIter
>    - GtkCellAreaBoxIter
> 
> I'm going to implement the size requests for GtkCellAreaBox later
> tonight so it should be more clear what GtkCellAreaBoxIter is doing
> by looking at the GtkCellAreaBox request code.
> 

Most of the request code is now written in the branch, there is still
missing the most important request: height-for-width on a horizontally
oriented GtkCellAreaBox.

However from what's there it should be clear what's happening with
GtkCellAreaBox <--> GtkCellAreaBoxIter interactions.

I've added a "spacing" and an "align-cells" property to the
GtkCellAreaBox class which will define whether or not the box should
store/use alignment information when updating the iter and allocating
cell sizes.

After finishing up with the request code I plan to add:
  - gtk_cell_area_allocate_width ()
  - gtk_cell_area_allocate_height ()

Both with class level vfuncs ofcourse to be implemented by
subclasses. Having an allocated size will of course reduce
computing when doing things like ->event()/->render().

Depending on the GtkSizeRequestMode in use by the parenting
layout widget (hfw of wfh), generally only allocate_width()
or allocate_height() will be called. However there will be
cases where we get to further reduce computing of sizes at
->event()/->render() time by having both width and height 
allocated on a GtkCellArea (i.e. GtkTreeView's fixed-height-mode
will allow us to just always know the complete allocated
size and position of each renderer before rendering or handling
events).

After implementing the allocation methods, I'll move on
to write/implement the actual ->render() and ->event() methods
(which is more or less just copying what GtkTreeViewColumn
does already).

And finally what is still missing is a ->get_bounds() method
to return a GdkRectangle of the actual space used by cell
renderers inside the space allocated to a GtkCellArea (i.e.
to allow parent layout widgets to smartly do their focus
painting).

And then will come the grimy situation of pushing this all
into GtkTreeView/GtkTreeViewColumn, looking in depth at
validate_row again etc. (I'm sure I'll have some more
questions and be happy to get any help in that area).

Cheers,
          -Tristan