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

[Tristan Van Berkom <tristanvb openismus com>]

On Sun, 2010-10-31 at 15:21 +0900, Tristan Van Berkom wrote:
> On Wed, 2010-10-27 at 08:59 +0200, Kristian Rietveld wrote:
> > On Tue, Oct 26, 2010 at 6:34 PM, Tristan Van Berkom
> > <tristanvb openismus com> wrote:
> > > 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).
> > 
> > As soon as you have implemented the allocation methods, I can also
> > take over and do the GtkTreeViewColumn migration, together with
> > merging in the refactorings I was talking about earlier.
> > 
> > > 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).
> > 
> > So I take it we will not be doing focus drawing in GtkCellArea, but
> > rather in the parent layout widget, right?  The focus drawing code for
> > GtkTreeView is currently at several places, so I will also look into
> > properly migrating that.
> > 
> > Another thing what get_bounds() will be useful for is to make it
> > possible to start rubber banding when clicking on an area that is not
> > covered by cell contents (and is thus "background").  See bug 350618.
> > 
> > > 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).
> > 
> > As soon as you have the allocate methods in place that you described
> > above, tell me and I can quickly do a tree view migration during the
> > evenings and see how far we can come with the current API.
> > 
> 
> Ok so I'm pretty much finished the request/allocation code... I've got
> as far as having a list of renderers with allocated positions and sizes
> come time for ->render()/->event() etc.
> 
> If I understand correctly about cell_area vs background_area, the
> difference is basically only the focus line width... the reason for
> this is that we want cells to be allowed to fill the whole background
> area with a possible background color before we go ahead and pain a
> dotted focus line over the possibly modified background area.
> 
> so currently the api on the GtkCellArea only takes a single cell_area
> when rendering, I think thats going to be fine in general and make for 
> a clearer API (let me know if I missed something).
> 
> So for this I'd like to add a "focus-line-width" property to the
> base class GtkCellArea, and just make the cell area implementations
> always request space for focus-line-width around cells ... then
> come time to render we just create a background/cell_area based
> on the focus-line-width and pass both areas to
> gtk_cell_renderer_render()... I'll do that part presently.
> 
> So as an overview of what I have in the branch, heres some
> outlines on how the API is expected to be used particularly
> in the case of treeview in the form of some pseudo code:
> 
> ------------------------------------------------------------
> /* First cells are configured and setup with these brand
>  * of apis 
>  */
> GtkCellArea *gtk_cell_area_box_new (void);
> 
> /* Note the 'align' argument here denotes whether the position
>  * of the added cell is to be aligned with adjacent rows
>  * using the same iter.
>  */
> void gtk_cell_area_box_pack_start (GtkCellAreaBox  *box,
> 				   GtkCellRenderer *renderer,
> 				   gboolean         expand,
> 				   gboolean         align);
> 
> gint gtk_cell_area_box_set_spacing (GtkCellAreaBox  *box);
> 				   (gint             spacing);
> 
> /* By default the box is horizontal anyway */
> gtk_orientable_set_orientation (GTK_ORIENTABLE (box));
> 
> 
> /* Then at least one GtkCellAreaIter is aquired, its possible
>  * to use several iters if one wants to group rendering by
>  * row data (for instance by treeview depth, or GtkComboBox
>  * will probably want to use one per submenu).
>  */
> GtkCellAreaIter *
> gtk_cell_area_create_iter (GtkCellArea *area);
> 
> At this point we have an unrequested/unallocated 'iter' so
> we should only be performing requests... for treeview it should
> generally look like this:
> 
> while (looping over some rows) {
> 
>     /* Apply the row data for every consecutive call */
>     void gtk_cell_area_apply_attributes(GtkCellArea        *area,
> 					GtkTreeModel       *tree_model,
> 					GtkTreeIter        *iter);
> 
> 
>    /* Run the get_preferred_width, we ignore returned results 
>     * for each row as that is stored in the iter 
>     */
>    void gtk_cell_area_get_preferred_width (GtkCellArea *area,
> 					GtkCellAreaIter  *iter,
> 					GtkWidget        *widget,
> 					gint             *minimum_size,
> 					gint             *natural_size)
> }
> 
> /* Now we've saved some processing to sum up the alignments of every
>  * row, so its important to go ahead and call: 
>  */
> void gtk_cell_area_iter_sum_preferred_width (GtkCellAreaIter *iter);
> 
> /* Now that we have some alignments of cells stored, we can access our
>  * proposed widths
>  */
> void gtk_cell_area_iter_get_preferred_width (GtkCellAreaIter *iter,
> 					     gint    *minimum_width,
> 					     gint    *natural_width);
> 
> /* ... And then we can get the heights of each row for that width 
>  */
> while (looping over some rows) {
> 
>     /* Apply the row data for every consecutive call */
>     void gtk_cell_area_apply_attributes(GtkCellArea        *area,
> 					GtkTreeModel       *tree_model,
> 					GtkTreeIter        *iter);
> 
>    /* Run the get_preferred_height_width() on a series of rows
>     * We can gather the collective heights here for variable
>     * row heights... or...
>     */
>    void gtk_cell_area_get_preferred_width (GtkCellArea *area,
> 				GtkCellAreaIter  *iter,
> 				GtkWidget        *widget,
> 				gint              width,
> 				gint             *minimum_height,
> 				gint             *natural_height);
> }
> 
> /* ... Or for fixed row heights we can at this point sum up the
>  * heights for that with using:
>  */
> void 
> gtk_cell_area_iter_sum_preferred_height_for_width 
> 				(GtkCellAreaIter *iter,
> 				 gint             for_width);
> 
> /* And then access it at any time using: */
> void
> gtk_cell_area_iter_get_preferred_height_for_width 
> 				(GtkCellAreaIter *iter,
> 				gint             for_width,
> 				gint            *minimum_height,
> 				gint            *natural_height);
> 
> 
> 
> /* Now, before we can operate on the GtkCellArea we need to allocate
>  * the iter in at least the orientation of the GtkCellAreaBox (this
>  * is a requirement of the GtkCellAreaBox itself but might not be
>  * a requirement for others, still I think generally its optimal to
>  * have areas orientable and demand an allocation in their orientation).
>  */
> void gtk_cell_area_iter_allocate_width (GtkCellAreaIter *iter,
> 					gint             width);
> 
> 
> /* Now we can go ahead and render cells, the previous call to
>  * allocate the iter has performed the natural allocation
>  * among any cells that are to be aligned across rows,
>  * when rendering cells, any unaligned cells leading up to
>  * an aligned cell will be done on demand for every row:
>  */
> while (looping over some rows) {
> 
>     /* Apply the row data for every consecutive call */
>     void gtk_cell_area_apply_attributes(GtkCellArea        *area,
> 					GtkTreeModel       *tree_model,
> 					GtkTreeIter        *iter);
> 
> 
>     /* Render the GtkCellArea */
>     void gtk_cell_area_render (GtkCellArea        *area,
> 			       GtkCellAreaIter    *iter,
> 			       GtkWidget          *widget,
> 			       cairo_t            *cr,
> 			       const GdkRectangle *cell_area);
> }
> 
> 
> /* When an allocated width for a height for width treeview changes,
>  * its important to flush out the previous results (or when we
>  * allow the width of the treeview to shrink in consequence of
>  * data updates and we may want to re-request the whole thing),
>  * for this we have:
>  */
> void gtk_cell_area_iter_flush (GtkCellAreaIter *iter);
> void gtk_cell_area_iter_flush_preferred_width (GtkCellAreaIter *iter);
> void gtk_cell_area_iter_flush_preferred_height_for_width
> 					(GtkCellAreaIter *iter,
> 					gint             for_width);
> void gtk_cell_area_iter_flush_preferred_height (GtkCellAreaIter *iter);
> void gtk_cell_area_iter_flush_preferred_width_for_height
>                                         (GtkCellAreaIter *iter,
> 					gint             for_height);
> 
> /* The contextual height-for-width flushing can be useful if the 
>  * implementing layout widget wants to synchronously respond to
>  * gtk_widget_height_for_width() calls, it would perform the request
>  * for a said width, accumulate the results and flush the cache...
>  * however for large treeviews this might not be interesting.
>  */
> 
> /* Another thing that may or may not be advantageous, the 
>  * GtkCellAreaIter provides some notifications so one can
>  * watch the base preferred widths/heights as properties
>  * as well as trap some signals which tell of heights which
>  * change for a given width... this may be interesting for
>  * some layouts who want to request things in the background
>  * and just find out when the overall sum of the required
>  * width has grown.
>  */
> 
> ------------------------------------------------------------
> 
> So it's not so vague anymore, probably a little rough around the
> edges but taking shape.
> 
> There remains only to implement the actual render/event methods,
> add the focus area introspecting... and then we should be ready
> to try this out.
> 

Whew, ok I implemented GtkCellArea->render for GtkCellAreaBox for the
most part, however I'm still missing the GtkCellRendererState flags ;-)

So for this part I was thinking it might make more sense to create
a new GtkCellAreaStateFlags type with just per-row states (and add that
as an argument to GtkCellArea->render() and also GtkCellArea->event())

and then somehow tidy up the code that in GtkTreeViewColumn is
currently:

  _gtk_tree_view_column_count_special_cells (tree_column)

I haven't quite got around to reading/understanding what that does yet.

For focus handling and such I guess it will probably make sense to add:

  GtkCellArea->set/get_focus_cell()

Which is currently missing... then I suppose from ->event() if the row
for which an event is handled itself has focus, it will make sense to
activate the focused cell.

Maybe for keystroke events we can even put the code which handles that
in the base GtkCellArea class by chaining up to it first from 
GtkCellAreaBox, then we can have GtkCellAreaBox only handling events
which require geometrical calculations (i.e. finding out which cell
a mouse event belongs to).

Eager to hear your thoughts on what we should do for
managing/creating the GtkCellRendererState flags appropriate for
rendering the cells :)

Cheers,
          -Tristan

PS: I also got around to implementing:
    gtk_cell_area_class_install_cell_property()
and all of it's counterparts... I havent glorified it with a
notification queue though, I'm not sure if we need to notify
about changing child cell properties, could be added later
if desired but wont be needed for GtkBuilder purposes which is
the main reason I added that machinery.

GtkCellAreaBox "boolean align","boolean expand" and 
"GtkPackType pack-type" child cell properties are now implemented
on the branch as well.