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

[Tristan Van Berkom <tristanvb openismus com>]

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.

Cheers,
         -Tristan