After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 587091 - merge filesystemmodel branch
merge filesystemmodel branch
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkFileChooser
2.13.x
Other Linux
: Normal normal
: ---
Assigned To: Federico Mena Quintero
Federico Mena Quintero
Depends on:
Blocks:
 
 
Reported: 2009-06-26 20:40 UTC by Benjamin Otte (Company)
Modified: 2009-10-20 17:04 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Benjamin Otte (Company) 2009-06-26 20:40:02 UTC
I have created a filesystemmodel branch in the git repository, where I cleaned up code and improved performance for the file chooser.

See http://mail.gnome.org/archives/gtk-devel-list/2009-June/msg00092.html for a longer discussion about the why and what.

I'm done with that work now and would like to get it reviewed and merged.
The diffstat:
 gtk/gtkfilechooserbutton.c   |    2 -
 gtk/gtkfilechooserdefault.c  | 3026 ++++++++++--------------------------------
 gtk/gtkfilechooserdialog.c   |    9 +-
 gtk/gtkfilechooserprivate.h  |   68 +-
 gtk/gtkfilechoosersettings.c |   12 +-
 gtk/gtkfilesystemmodel.c     | 2866 +++++++++++++++++++---------------------
 gtk/gtkfilesystemmodel.h     |   76 +-
 gtk/gtkliststore.c           |    2 +-
 gtk/gtktreestore.c           |    2 +-
 gtk/gtktreeview.c            |   21 +-
 10 files changed, 2122 insertions(+), 3962 deletions(-)

You can ignore the small ones, those are cleanups that I'm gonna just merge.
All the changes are exclusive to gtkfilechooserdefault.c gtkfilesystemmodel.c and gtkfilesystemmodel.h.
There have been no public API changes at all. The functionality of the file chooser itself has not changed either.
Comment 1 Bastien Nocera 2009-06-26 23:22:59 UTC
Please post incremental improvements as separate patches, as requested on the mailing-list.
Comment 2 Benjamin Otte (Company) 2009-06-27 07:18:51 UTC
Could you elaborate on how you imagine this happening?
The whole current patchset in incremental form can be seen at http://git.gnome.org/cgit/gtk+/log/?h=filesystemmodel right now.

However, the code regresses quite a bit during those patches as I throw away the model and recode it from scratch. It's probably not desired that master regresses, or is it?

If it's ok to regress, it's the same patches as in the link. If it's not ok to regress, it's the same patches as in the link, just as (pretty much) one big chunk.
Comment 3 Christian Dywan 2009-06-28 01:14:01 UTC
I presume the intention behind incremental patches is about implementing your changes step by step instead of rewriting the code form scratch. As it is, the file chooser is famous for its regressions.
Comment 4 Benjamin Otte (Company) 2009-07-01 08:57:54 UTC
I had a sudden insight on how to handle a patch series that can be applied one by one, so I did that and pushed a new filesystemmodel branch. The branch does not regress between single commits, so it can be applied step-by-step.

If anyone wants, I can attach the patches here, but as it is 19 separate patches, I'm not sure it wouldn't just clutter the attachment view.
Comment 5 Steve Frécinaux 2009-07-10 08:21:07 UTC
Too bad you didn't explain the time improvements for each step of your branch. It would have been instructive...
Comment 6 Benjamin Otte (Company) 2009-07-10 18:51:40 UTC
(In reply to comment #5)
> Too bad you didn't explain the time improvements for each step of your branch.
> It would have been instructive...
> 
That's because I didn't measure them. I did quite some reorganization as mentioned in comment 4, so you will not be able to see an effect from single patches.
However, I just wrote up stuff that helps when making tree views faster in my blog at http://blogs.gnome.org/otte/2009/07/10/treeview-tips/ - that and the commit messages from the patches should give quite a good picture of stuff that helps.
Comment 7 Federico Mena Quintero 2009-08-19 19:27:15 UTC
I'm reviewing this; will post comments here.
Comment 8 Javier Jardón (IRC: jjardon) 2009-10-20 17:03:54 UTC
landed in glib 2.24 development version
Comment 9 Javier Jardón (IRC: jjardon) 2009-10-20 17:04:46 UTC
Sorry, I meant gtk+ 2.20 development version