Re: label performance patch

[Owen Taylor <otaylor redhat com>]

Alex Larsson <alexl redhat com> writes:

> On 14 Nov 2001, Owen Taylor wrote:
> 
> > Alex Larsson <alexl redhat com> writes:
> > 
> > > GtkLabel keeps reflowing wrapped labels all the time. Here is a fix:
> > 
> > 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);
> 
> Ok. Here is a new patch, this time without whitespace changes:

Looks just about right; should be OK to commit, with one needed
fix noted below.

> +  /*
> +   * There are a number of conditions which will necessitate re-filling
> +   * our text, potentially changing the size request.
> +   *
> +   *     1. text changed.
> +   *     2. wrapping has been turned on or off
> +   *     3. font changed.
> +   *
> +   * These have been detected elsewhere, and label->layout will be NULL,
> +   * if one of the above has occured.
> +   *
> +   * Additionally, though, 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 widget
> +   *   - the padding of the widget (xpad, set by gtk_misc_set_padding)
> +   *
> +   * We don't actually detect this, instead we just clear the layout
> +   * whenever we're wrapping and have a usize.
> +   */

Let's compactify this comment; it's a lot of words for little purpose.
I'd delete the first section, then say:

 /*  
 * If word wrapping is on, then the height requisition can depend
 * on:
 *
 *   - Any width set on the widget via gtk_widget_set_usize().
 *   - The padding of the widget (xpad, set by gtk_misc_set_padding)
 *
 * Instead of trying to detect changes to these quantities, if we
 * are wrapping, we just rewrap for each size request. Since
 * size requisitions are cached by the GTK+ core, this is not
 * expensive.
 */

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

As I mentioned this morning, this doesn't actually work ... because
we don't detect the case where the aux info was set, then subsequently
unset. I think we can just do 

 if (label->wrap)
   gtk_label_clear_layout (label);

Regards,
                                        Owen