Re: label performance patch

[Owen Taylor <otaylor redhat com>]

Alex Larsson <alexl redhat com> writes:

> GtkLabel keeps reflowing wrapped labels all the time. Here is a fix:


> @@ -1319,9 +1317,7 @@ gtk_label_clear_layout (GtkLabel *label)
>  }
>  
>  static void
> -gtk_label_ensure_layout (GtkLabel *label,
> -                         gint     *widthp,
> -                         gint     *heightp)
> +gtk_label_ensure_layout (GtkLabel *label)
>  {
>    GtkWidget *widget;
>    PangoRectangle logical_rect;
> @@ -1334,10 +1330,10 @@ gtk_label_ensure_layout (GtkLabel *label
>     * our text:
>     *
>     *     1. text changed.
> -   *     2. justification changed either from to to GTK_JUSTIFY_FILL.
> +   *     2. justification changed either from or to GTK_JUSTIFY_FILL.
>     *     3. font changed.
>     *
> -   * These have been detected elsewhere, and label->words will be zero,
> +   * These have been detected elsewhere, and label->layout will be NULL,
>     * if one of the above has occured.
>     *
>     * Additionally, though, if GTK_JUSTIFY_FILL, we need to re-fill if:
> @@ -1346,8 +1342,8 @@ gtk_label_ensure_layout (GtkLabel *label
>     *     5. gtk_misc_set_padding has changed xpad.
>     *     6.  maybe others?...
>     *
> -   * Too much of a pain to detect all these case, so always re-fill.  I
> -   * don't think it's really that slow.
> +   * (un)Fortunately, PangoLayout does not currently support GTK_JUSTIFY_FILL,
> +   * so this is not an issue.
>     */

Actually, this comment drifted a bit from the the code ... if I
recall correctly GTK_JUSTIFY_FILL was overloaded to mean 
autowrap originally, and we only later separated out wrap.
GTK_JUSTIFY_FILL, if implemented would only affect drawing,
what this is actually referring to is a case that still
happens:

 If word wrap is on, and gtk_widget_set_usize() has been called to
 explicitely set the width of the widget, then the requested height
 will depend on the width, which is a function of:

  - The width set on the width
  - the padding of the widget

I think the way we should handle this is do something in 
gtk_label_size_request() like:

 if (label->wrap && _gtk_widget_get_aux_info (widget, FALSE))
   gtk_label_clear_layout (label);

And move the appropriate part of the comment to that location
with fixes to make it intelligable. Since we cache size
requisitions now, there shouldn't be any real performance
problem with clearing the layout in size_request().

The rest of the patch looks OK ... I'm assuming you didn't
change the logic of the "calculate a nice width" code since
trying to decipher the actual changes from the indentation
changes there is pretty hard.

Regards,
                                        Owen