Re: A tale of waiting

[Kristian Rietveld <kris gtk org>]

On Thu, Jun 25, 2009 at 4:54 PM, Benjamin Otte<otte gnome org> wrote:
> On Thu, Jun 25, 2009 at 12:12 PM, Kristian Rietveld<kris gtk org> wrote:
>> As a side comment, I am not really sure how fair it is to compare a
>> full blown GUI to a command line utility with its output redirected to
>> /dev/null.
>>
> I thought about this quite a bit and decided it was a fair comparison,
> as long as I didn't aim to match the numbers exactly. In fact,
> launching a binary that has to do a bunch of setup and format the
> output of the whole list of files and then comparing it to a file
> chooser displaying that directory in an already visible dialog, that
> only displays the top 20 results, it seems the file chooser could even
> be faster.

Ah, ok.  If you work from the starting point of having an already
visible dialog, then I agree with your argument that this is pretty
fair.

> Yeah, I added a quick accessor _gtk_file_system_model_get_value() that
> shortcuts gtk_tree_model_get() and got rid of all performance issues.
>
> I just noted this problem as it's a general issue every time you want
> to sort with the default tree models and have 10000 or more items. And
> I can imagine quite a few places where that would be a relevant
> problem. (Including my favorite, the epiphany history and location
> bar)

Yes.  I think it would be quite nice if we could come up with proper
API to fix such issues.  Currently, I can only think of something that
would involve moving the responsibility of doing the comparison to
inside the model that is being sorted; as opposed to the current
situation where the comparison function is outside and requiring the
costly accesses.  But again, I have no idea yet if this can be
properly done in a non-hackish way.

>> Fixed height mode bypasses this by only measuring the first row, and
>> then set the same size for each row.  Rows cannot have different
>> heights in this mode.
>>
> This would be a worthwhile thing to pursue, as the file chooser rows
> have a fixed height, if it weren't for the column widths that also
> need to be fixed. And that's not what we want there. Otherwise I'd
> have used it already.

Oops, that is exactly what I forgot in my previous mail: with fixed
height mode we cannot really guess proper widths for the columns.  We
could change the code to allow for GROW_ONLY columns, but that will
make your columns resize as you scroll through the tree view.  What
might be possible is to take a sample from the model and guess a
reasonable column size from that.  It won't be as perfect as now, but
it will probably eliminate much of the resizing during scrolling.

> A thing I noticed is that the tree view validates cells even if the
> cell renderers have a fixed size set. I tried to use that with the
> icon renderer (icons all have the same size after all), but it didn't
> help.

That is because the tree view does not communicate with the cell
renderers directly.  Instead, in validate_row() it requests the size
needed by each column and each column will look at individual cell
renderers.  As such, we have to set the cell data for the entire
column.

> I looked at the code, but validate_row() calls
> gtk_tree_view_column_cell_set_cell_data() followed by
> gtk_tree_view_column_cell_get_size(). But to speed this up, you'd have
> to merge these two functions into one. And as that's public API, I
> wasn't sure how good an idea that'd be.

You can't merge those, since they are also used separately.  You could
introduce a variant.  What is important is that currently (in
gtk_cell_renderer_get_size()) the get_size() method on a cell is being
called also if it has fixed size.  We can omit this call if the
x_offset and y_offset arguments to gtk_cell_renderer_get_size() are
NULL.  But in case these are non-NULL (and this happens when the cell
is being rendered and when we need to figure out where the cell is
located so we know where to draw the focus rectangle), we have to call
get_size() and then the cell data has to be set.

> Yeah, that was an idea I had for speeding up the tree view, too. I'd
> be very happy to see such a patch land.
> In particular because it would dramaticlly improve scrollbar behavior
> when not all files have been validated yet. This currently causes
> constant updates and makes the scrollbar jump.
>
> You can see that happening in
> http://people.freedesktop.org/~company/stuff/filechooser.gif - it's a
> recording of me opening my test directory and you can see me being
> irritated at the scrollbar behavior.

Also here, we need a solution for the automatic column widths.  The
sampling method outlined above might be the outcome here.  I don't
remember if I have actually experimented with the column widths yet,
that will be one of the first things to check when I get back to it ;)

> I'm all for it! :)
> About layouting tests, I know that the Mozilla guys use reftests very
> successfully, see
> http://weblogs.mozillazine.org/roc/archives/2008/12/reftests.html and
> Google. What that basically does is taking two widgets by taking a
> screenshot of each and checking the images match pixel by pixel. If
> written properly, those tests are independant of Gtk theme, settings,
> locale, fonts and all that stuff.

Oh, that is an interesting approach, thanks for the link!  Of course I
did think of the very obvious "compare a widget rendering to a static
image" approach, which is incredibly prone to theming and other
settings as you mention.  What mozilla is doing looks like a much
better idea :)


regards,

-kris.