Better column resizing for GtkTreeView

2013-01-13 Thread John Lindgren

Hi all,

There are several problems with the current support for resizable 
columns in GtkTreeView, which I have written some patches to fix:


* There is no way to set distinct minimum and natural widths for a 
GtkTreeViewColumn.  I propose that the "fixed-width" property be 
interpreted as the natural width of the column rather than the minimum 
width (we already have the "min-width" property for that).


* Column resizing is implemented using a hidden "resized-width" property 
that is not visible to applications.  Hence, user changes to column size 
are lost every time the application is closed.  I propose to get rid of 
the "resized-width" property and instead modify the "fixed-width" and 
"expand" properties to get the desired column size.  Applications can 
then preserve user-set column sizes by saving and restoring these two 
properties.


* The code that currently handles the "resized-width" property is 
convoluted and causes unexpected results when the size of the entire 
treeview is changed (see [1]).  After my patches, the code is simpler 
and behaves as expected (see [2]).


Please look over the attached patches and comment.

   John Lindgren

[1] screencast before (ogv): https://www.box.com/s/gz23ksylzjp046dn8ah5
[2] screencast after (ogv): https://www.box.com/s/c6zhov1igiqss7x97k5r
>From 9c8ee6c3a1977a9341455e6e5c18aff9d99e1e46 Mon Sep 17 00:00:00 2001
From: John Lindgren 
Date: Tue, 18 Dec 2012 00:23:27 -0500
Subject: [PATCH 1/6] Remove extraneous size request

---
 gtk/gtktreeview.c | 25 +
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/gtk/gtktreeview.c b/gtk/gtktreeview.c
index b1528da..1e64582 100644
--- a/gtk/gtktreeview.c
+++ b/gtk/gtktreeview.c
@@ -715,7 +715,7 @@ static gboolean validate_row (GtkTreeView *tree_view,
 static void validate_visible_area(GtkTreeView *tree_view);
 static gboolean validate_rows_handler(GtkTreeView *tree_view);
 static gboolean do_validate_rows (GtkTreeView *tree_view,
-	  gboolean size_request);
+	  gboolean queue_resize);
 static gboolean validate_rows(GtkTreeView *tree_view);
 static gboolean presize_handler_callback (gpointer data);
 static void install_presize_handler  (GtkTreeView *tree_view);
@@ -6701,35 +6701,12 @@ do_validate_rows (GtkTreeView *tree_view, gboolean queue_resize)
  done:
   if (validated_area)
 {
-  GtkRequisition requisition;
-
-  /* We temporarily guess a size, under the assumption that it will be the
-   * same when we get our next size_allocate.  If we don't do this, we'll be
-   * in an inconsistent state when we call top_row_to_dy. */
-
-  /* FIXME: This is called from size_request, for some reason it is not infinitely
-   * recursing, we cannot call gtk_widget_get_preferred_size() here because that's
-   * not allowed (from inside ->get_preferred_width/height() implementations, one
-   * should call the vfuncs directly). However what is desired here is the full
-   * size including any margins and limited by any alignment (i.e. after 
-   * GtkWidget:adjust_size_request() is called).
-   *
-   * Currently bypassing this but the real solution is to not update the scroll adjustments
-   * untill we've recieved an allocation (never update scroll adjustments from size-requests).
-   */
-  gtk_tree_view_size_request (GTK_WIDGET (tree_view), &requisition, FALSE);
-
   /* If rows above the current position have changed height, this has
* affected the current view and thus needs a redraw.
*/
   if (y != -1 && y < gtk_adjustment_get_value (tree_view->priv->vadjustment))
 gtk_widget_queue_draw (GTK_WIDGET (tree_view));
 
-  gtk_adjustment_set_upper (tree_view->priv->hadjustment,
-MAX (gtk_adjustment_get_upper (tree_view->priv->hadjustment), requisition.width));
-  gtk_adjustment_set_upper (tree_view->priv->vadjustment,
-MAX (gtk_adjustment_get_upper (tree_view->priv->vadjustment), requisition.height));
-
   if (queue_resize)
 gtk_widget_queue_resize_no_redraw (GTK_WIDGET (tree_view));
 }
-- 
1.8.0.2

>From ad8dc277807d359536026ad27cee8b83512fb1b3 Mon Sep 17 00:00:00 2001
From: John Lindgren 
Date: Tue, 18 Dec 2012 00:47:07 -0500
Subject: [PATCH 2/6] Kill gtk_tree_view_size_request

---
 gtk/gtktreeview.c | 94 +++
 1 file changed, 25 insertions(+), 69 deletions(-)

diff --git a/gtk/gtktreeview.c b/gtk/gtktreeview.c
index 1e64582..f4776bf 100644
--- a/gtk/gtktreeview.c
+++ b/gtk/gtktreeview.c
@@ -385,8 +385,6 @@ struct _GtkTreeViewPrivate
   gint x_drag;
 
   /* Non-interactive Header Resizing, expand flag support */
-  gint pre

Re: Bug #679291 (please review)

2012-08-04 Thread John Lindgren
On 08/04/2012 05:54 PM, Matthias Clasen wrote:
> On Sat, Aug 4, 2012 at 4:58 PM, Emmanuele Bassi  wrote:
> 
>> the patch in attachment 217892 looks okay - but what I'd like to see:
>>
>> a) bisecting to see what commit broke this;
>> b) a test case for the TreeView test suite, to ensure we don't regress again;
>> c) a patch done using git format-patch or git bz, so that we can credit the
>> author.
> 
> The behaviour change was introduced during the heavy refactoring of
> treeview a11y done by Benjamin last winter. We've talked about
> 'fixing' this (ie suppressing the signal in the destroy path), but
> there's a more general question here: do we want to add tons of
> special-cases to prevent this kind of noise in destroy paths ? There's
> plenty of other places where this could happen.

What do you suggest as an alternative?  I don't think it's reasonable to
require every GTK+ client to disconnect every signal handler from every
object before it is destroyed, in fear of the handler being called from
the destroy path, when the internal state of the object is more or less
undefined.

How about disconnecting all signal handlers on the GTK+ side immediately
after emitting the "destroy" signal?  It seems reasonable to me that no
signal should ever be emitted after "destroy", but I could be
overlooking something.

-- John
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: Bug #679291 (please review)

2012-08-04 Thread John Lindgren
On 08/04/2012 04:58 PM, Emmanuele Bassi wrote:
> hi John;
(snip)
> I left a comment in bug 671939, but I can also repeat it here.
> 
> the patch in attachment 217892 looks okay - but what I'd like to see:
> 
> a) bisecting to see what commit broke this;
> b) a test case for the TreeView test suite, to ensure we don't regress again;
> c) a patch done using git format-patch or git bz, so that we can credit the
> author.
> 
> John, would you be so kind to look at this? that would be stellar.

I can write a test case and do a bisect, I think.  Are there any
instructions around as to how the test case should be encapsulated
(stand-alone program, C function)?

-- John
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list


Bug #679291 (please review)

2012-08-04 Thread John Lindgren
Hi,

About a month ago I sent a patch to the bug tracker to fix a problem
where GtkTreeView would emit signals, thereby executing user code,
during its destroy cycle.  This is a potentially serious problem as the
state of the GtkTreeView is more or less undefined as it is being destroyed.

One André Klapper (never heard of him before and don't know his email)
closed the bug as a duplicate but did nothing further to resolve it.
Can someone please review the patch [1] and commit it?

Thanks in advance,

-- John Lindgren

[1] http://bugzilla-attachments.gnome.org/attachment.cgi?id=217892
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list


g_signal_handlers_block_matched() prototype

2012-02-20 Thread John Lindgren
Hi,

The  argument passed to g_signal_handlers_block_matched() is
currently a gpointer, which causes a warning when GCC is in strict C99
mode, since C99 does not allow converting a function pointer to an
object pointer.  Wouldn't it make more sense for  to have
GCallback type?

-- John Lindgren
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list