Re: Minutes of the GTK+ Team Meeting - 2010-12-14

[Tristan Van Berkom <tristanvb openismus com>]

On Sat, 2010-12-18 at 12:24 -0500, Matthias Clasen wrote:
> On Sat, Dec 18, 2010 at 1:09 AM, Tristan Van Berkom
> <tristanvb openismus com> wrote:
> > On Fri, 2010-12-17 at 19:33 -0500, Matthias Clasen wrote:
> >> Tristan,
> >>
> >> another thing (that I believe Cosimo already pointed out) is that the
> >> new cell area code seems to not quite get interaction between cell
> >> data functions and size allocation right.
> >>
> >> You can see the resulting breakage in testappchooser. The treeview in
> >> the appchooser uses a separate cell renderer for the heading text, and
> >> uses a cell data function to make it visible only for rows that are
> >> supposed to be headings (this is a very common pattern, expect many
> >> things to break in similar ways). After the cellarea merge, the
> >> heading cell renderer seems to always get its size reserved, even in
> >> rows where it is invisible.
> >
> > Ah interesting.
> >
> > Currently this is happening because by default all cells are packed
> > with the "align" property set to true.
> >
> > If we were to make the second cell appear "first" in the case the
> > first cell is invisible, it would be a lie to say that that cell
> > is "aligned" with adjacent cells.
> >
> > Unaligning the cell following the invisible cell will make the
> > second cell cover the first cell's area when invisible, but not
> > following cells, maybe we need some different semantic for that
> > kind of thing (thinking...)
> 
> It seems we need some kind of grouping for these alignment considerations.
> 
> In the 2.x world, all cells in a column could use the space of the
> column, depending on the visibility of the other cells in the same
> column, but cells could not grow across column boundaries.

Hmmm, seems I misunderstood the exact problem, but the solution I've
been thinking of seems to apply here anyway.

There's 2 approaches I've been thinking of to fix this.

  a.) Do nothing to GtkCellArea, set the "align" cell property for
      cells in the app chooser to FALSE (like the attached patch
      does, as an example).

  b.) I was thinking we could implement a separate "fixed-size"
      cell property for GtkCellAreaBox that would still cache the
      largest size of the cell for a given series of requests.

      Setting a "fixed-size" on a cell would achieve a similar
      effect of setting "aligned" on a following cell, except that
      following cells would not be aligned when that "fixed-size" cell
      is invisible (unless the following cell is also set to "align").

Option "a" comes with the disadvantage that rendering is technically
slower (not visibly but cell sizes are recalculated for every render).

Option "b" requires say... an evening of hacking that I'm willing to
do to get things working better, if we go with option "b" I would change
the default behaviour of gtk_cell_layout_pack_start/end for box areas
to be "align" FALSE "fixed size" TRUE, that would be a more exact match
for the behaviour of GTK+ 2.x I suppose.

Also, in the pie-in-the-sky future, I think Kris has some ideas of doing
something specifically for "group header" rows, maybe something we can
add so that all of this cell tinkering becomes unneeded for achieving
the effect of "header rows".

Cheers,
   -Tristan

Offtopic PS:
  On a somewhat related note, currently header rows need to have
actual rows of data in a said model in order to work, I think this
is a bit wrong or awkward. Since Glade implemented GtkTreeModel to
represent it's widget list for the project view widget, we've lost
the header rows for "Actions" "Widgets" and "Objects".

I've been wondering if maybe some kind of GtkTreeModelFilter feature
could be done here, i.e. create groups in a filter and virtually expose
the header rows from there, the filter could sort out and "group" the
underlying data into groups and virtually add rows... not sure if that
approach is exactly the right one though (however I'm sure that moving
away from using GtkListStore/GtkTreeStore towards implementing
GtkTreeModel is the right thing to do for almost every treeview 
out there, excepting combo-boxes and simple, short selection lists).

diff --git a/gtk/gtkappchooserwidget.c b/gtk/gtkappchooserwidget.c
index 5de7c3e..7d9f46d 100644
--- a/gtk/gtkappchooserwidget.c
+++ b/gtk/gtkappchooserwidget.c
@@ -33,6 +33,8 @@
 #include "gtkappchooserwidget.h"
 #include "gtkappchooserprivate.h"
 #include "gtkliststore.h"
+#include "gtkcelllayout.h"
+#include "gtkcellareabox.h"
 #include "gtkcellrenderertext.h"
 #include "gtkcellrendererpixbuf.h"
 #include "gtktreeview.h"
@@ -434,7 +436,7 @@ gtk_app_chooser_sort_func (GtkTreeModel *model,
 }
 
 static void
-padding_cell_renderer_func (GtkTreeViewColumn *column,
+padding_cell_renderer_func (GtkCellLayout     *layout,
                             GtkCellRenderer   *cell,
                             GtkTreeModel      *model,
                             GtkTreeIter       *iter,
@@ -776,6 +778,7 @@ gtk_app_chooser_widget_add_items (GtkAppChooserWidget *self)
   GtkCellRenderer *renderer;
   GtkTreeViewColumn *column;
   GtkTreeModel *sort;
+  GtkCellArea *area;
 
   /* create list store */
   self->priv->program_list_store = gtk_list_store_new (NUM_COLUMNS,
@@ -806,11 +809,11 @@ gtk_app_chooser_widget_add_items (GtkAppChooserWidget *self)
                                        gtk_app_chooser_search_equal_func,
                                        NULL, NULL);
 
-  column = gtk_tree_view_column_new ();
+  area = gtk_cell_area_box_new ();
 
   /* initial padding */
   renderer = gtk_cell_renderer_text_new ();
-  gtk_tree_view_column_pack_start (column, renderer, FALSE);
+  gtk_cell_area_box_pack_start (GTK_CELL_AREA_BOX (area), renderer, FALSE, FALSE);
   g_object_set (renderer,
                 "xpad", self->priv->show_all ? 0 : 6,
                 NULL);
@@ -818,11 +821,11 @@ gtk_app_chooser_widget_add_items (GtkAppChooserWidget *self)
 
   /* heading text renderer */
   renderer = gtk_cell_renderer_text_new ();
-  gtk_tree_view_column_pack_start (column, renderer, FALSE);
-  gtk_tree_view_column_set_attributes (column, renderer,
-                                       "markup", COLUMN_HEADING_TEXT,
-                                       "visible", COLUMN_HEADING,
-                                       NULL);
+  gtk_cell_area_box_pack_start (GTK_CELL_AREA_BOX (area), renderer, FALSE, FALSE);
+  gtk_cell_layout_set_attributes (GTK_CELL_LAYOUT (area), renderer,
+				  "markup", COLUMN_HEADING_TEXT,
+				  "visible", COLUMN_HEADING,
+				  NULL);
   g_object_set (renderer,
                 "ypad", 6,
                 "xpad", 0,
@@ -832,32 +835,33 @@ gtk_app_chooser_widget_add_items (GtkAppChooserWidget *self)
 
   /* padding renderer for non-heading cells */
   renderer = gtk_cell_renderer_text_new ();
-  gtk_tree_view_column_pack_start (column, renderer, FALSE);
-  gtk_tree_view_column_set_cell_data_func (column, renderer,
-                                           padding_cell_renderer_func,
-                                           NULL, NULL);
+  gtk_cell_area_box_pack_start (GTK_CELL_AREA_BOX (area), renderer, FALSE, FALSE);
+  gtk_cell_layout_set_cell_data_func (GTK_CELL_LAYOUT (area), renderer,
+				      padding_cell_renderer_func,
+				      NULL, NULL);
 
   /* app icon renderer */
   renderer = gtk_cell_renderer_pixbuf_new ();
-  gtk_tree_view_column_pack_start (column, renderer, FALSE);
-  gtk_tree_view_column_set_attributes (column, renderer,
-                                       "gicon", COLUMN_GICON,
-                                       NULL);
+  gtk_cell_area_box_pack_start (GTK_CELL_AREA_BOX (area), renderer, FALSE, FALSE);
+  gtk_cell_layout_set_attributes (GTK_CELL_LAYOUT (area), renderer,
+				  "gicon", COLUMN_GICON,
+				  NULL);
   g_object_set (renderer,
                 "stock-size", GTK_ICON_SIZE_DIALOG,
                 NULL);
 
   /* app name renderer */
   renderer = gtk_cell_renderer_text_new ();
-  gtk_tree_view_column_pack_start (column, renderer, TRUE);
-  gtk_tree_view_column_set_attributes (column, renderer,
-                                       "markup", COLUMN_DESC,
-                                       NULL);
+  gtk_cell_area_box_pack_start (GTK_CELL_AREA_BOX (area), renderer, TRUE, FALSE);
+  gtk_cell_layout_set_attributes (GTK_CELL_LAYOUT (area), renderer,
+				  "markup", COLUMN_DESC,
+				  NULL);
   g_object_set (renderer,
                 "ellipsize", PANGO_ELLIPSIZE_END,
                 "ellipsize-set", TRUE,
                 NULL);
   
+  column = gtk_tree_view_column_new_with_area (area);
   gtk_tree_view_column_set_sort_column_id (column, COLUMN_NAME);
   gtk_tree_view_append_column (GTK_TREE_VIEW (self->priv->program_list), column);