Sort color profile lists

[houz]

The lists of color profiles (for example in the document properties) gets sorted by filename. Sorting by displayed name would be nicer. The attached patch does that and also gets rid of some duplicates.

To see the duplicates just add the same directory (that also contains color profiles) to XDG_DATA_DIRS twice.

[jazzynico (jazzynico)]

The patch doesn't compile correctly on Crunchbang Waldorf (Debian stable based distro), and make returns the following errors:
-----

  CXX color-profile.o
In file included from /usr/include/c++/4.7/algorithm:63:0,
                 from /usr/include/glibmm-2.4/glibmm/containerhandle_shared.h:30,
                 from /usr/include/glibmm-2.4/glibmm/arrayhandle.h:23,
                 from /usr/include/glibmm-2.4/glibmm.h:91,
                 from /usr/include/gdkmm-2.4/gdkmm/color.h:9,
                 from color-profile.cpp:10:
/usr/include/c++/4.7/bits/stl_algo.h: In instantiation of ‘_RandomAccessIterator std::__unguarded_partition(_RandomAccessIterator, _RandomAccessIterator, const _Tp&, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<std::pair<Glib::ustring, Glib::ustring>*, std::vector<std::pair<Glib::ustring, Glib::ustring> > >; _Tp = std::pair<Glib::ustring, Glib::ustring>; _Compare = bool (*)(std::pair<Glib::ustring, Glib::ustring>&, std::pair<Glib::ustring, Glib::ustring>&)]’:
/usr/include/c++/4.7/bits/stl_algo.h:2321:78: required from ‘_RandomAccessIterator std::__unguarded_partition_pivot(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<std::pair<Glib::ustring, Glib::ustring>*, std::vector<std::pair<Glib::ustring, Glib::ustring> > >; _Compare = bool (*)(std::pair<Glib::ustring, Glib::ustring>&, std::pair<Glib::ustring, Glib::ustring>&)]’
/usr/include/c++/4.7/bits/stl_algo.h:2362:62: required from ‘void std::__introsort_loop(_RandomAccessIterator, _RandomAccessIterator, _Size, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<std::pair<Glib::ustring, Glib::ustring>*, std::vector<std::pair<Glib::ustring, Glib::ustring> > >; _Size = long int; _Compare = bool (*)(std::pair<Glib::ustring, Glib::ustring>&, std::pair<Glib::ustring, Glib::ustring>&)]’
/usr/include/c++/4.7/bits/stl_algo.h:5514:4: required from ‘void std::sort(_RAIter, _RAIter, _Compare) [with _RAIter = __gnu_cxx::__normal_iterator<std::pair<Glib::ustring, Glib::ustring>*, std::vector<std::pair<Glib::ustring, Glib::ustring> > >; _Compare = bool (*)(std::pair<Glib::ustring, Glib::ustring>&, std::pair<Glib::ustring, Glib::ustring>&)]’
color-profile.cpp:879:69: required from here
/usr/include/c++/4.7/bits/stl_algo.h:2289:4: error: invalid initialization of reference of type ‘std::pair<Glib::ustring, Glib::ustring>&’ from expression of type ‘const std::pair<Glib::ustring, Glib::ustring>’
/usr/include/c++/4.7/bits/stl_algo.h:2292:4: error: invalid initialization of reference of type ‘std::pair<Glib::ustring, Glib::ustring>&’ from expression of type ‘const std::pair<Glib::ustring, Glib::ustring>’
make: *** [color-profile.o] Erreur 1

[houz (houz)]

That doesn't look related to my changes. Are you sure that it compiles without the patch applied?

[su_v (suv-lp)]

Build of Inkscape 0.91+devel r14212+patch fails on OS X 10.7.5 too (compiles ok without patch).

[houz (houz)]

The attached patch fixes compilation for ~suv, does it help on Crunchbang Waldorf, too?

[jazzynico (jazzynico)]

Yes, the new patch compiles on Crunchbang Waldorf too, thanks!

It works correctly, but there is an inconsistency between the Document Properties profile list (sorted case-insensitively) and the Fill and Stroke profile list (case sensitive). It would be nice if both lists could use exactly the same sort type IMHO.

[houz (houz)]

From what I can tell, the list in Fill & Stroke (when setting it to CMS mode) and also the one in document properties showing the added profiles aren't sorted at all but just list all profiles with the most recently added at the top. Do you want me to add sorting there, too?

[jazzynico (jazzynico)]

> Do you want me to add sorting there, too?

If it's not too much work, yes, it would be great, thanks!

[houz (houz)]

Here we go. I opted to sort the list in the places it's used instead of keeping the source sorted. The latter would be easier to use when new code wants to show the list of profiles, however, the code maintaining the list only sees a bunch of SPObject* so it's not exactly straight forward to sort that according to profile names.

[jazzynico (jazzynico)]

Thanks for your new patch!
Tested on Windows XP (32 bit) and Xubuntu 14.04 (64 bit) with Inkscape trunk rev. 14225.

It's better now, but I still see inconsistencies. The Available Color Profiles list (Doc Properties) shows:
Apple-RGB
Compatible-with-Adobe-RGB--1998-
eciRGB-v2
ProPhoto-RGB
sRGB
Wide-Gamut-RGB

And in the Linked Color Profiles (Doc Properties) and in the Fil and Stroke profile lists, the order is different:
Apple-RGB
Compatible-with-Adobe-RGB--1998-
ProPhoto-RGB
Wide-Gamut-RGB
eciRGB-v2
sRGB

Do you thing it would be complicated to use the same order for all the lists (IMHO, the case insensitive one looks better)?

[houz (houz)]

So, here it is with case insensitive comparison. I am not 100% sure if it will give the exactly same results with some exotic Unicode characters but in the general case it seems to be working fine.

[su_v (suv-lp)]

<opinion>Personally, I'm not sure I'd prefer the proposed new sorting method: On OS X, the lists had been sorted nicely before - per directory searched for profiles - IIRC with system profiles at the top, and user profiles at the bottom. I kind of liked to have an idea about whether or not I was going link to one provided by the OS, or e.g. to a custom profile I installed myself as user). My personal preference probably might be to have the full path displayed as tooltip (then sorting the profiles grouped per location or just per display name is not as important anymore).</opinion>

[su_v (suv-lp)]

Comment 12 refers to the dropdown list for 'Document Properties > Color > Available Color Profiles'.

[jazzynico (jazzynico)]

The report has been sleeping in my todo list for too long...

I suggest we commit the ICC selector part of the patch.

For the Document properties, one possibility would be to create two list (one for the system profiles and one for the user profiles), sort them independently and then add them in the list with a separator (system profiles at the top, user profiles at the bottom).

@houz - I know how frustrating it is to provide a patch that takes too long to be accepted. Would you be willing to work on it again and see if the suggested solution can be implemented?

[houz (houz)]

Sure, I can do that. I already have some changes locally to adapt to some refactoring that happened in that code. However it will have to wait until after LGM, I am too busy right now.

[houz (houz)]

Here's a patch that should do what you suggested. I also wanted to add the tooltips su_v wanted, however, GTK doesn't seem to support that.
I use a somewhat ugly nesting of std:: containers instead of creating a new class type, but given it's only used to pass information between very few places I think it's ok like this. If you feel that it's not clean enough I can probably come up with something more sophisticated instead.

[jazzynico (jazzynico)]

Tested successfully on Windows 7, Inkscape trunk rev. 14862, with a limited number of system profiles.
I'll try to test it with a bigger list (and some user ICC profiles) on Xubuntu later.

Thanks for your new patch!

[jazzynico (jazzynico)]

Also tested (even more) successfully on Xubuntu 16.04, Inkscape trunk rev. 14881, with system and user profiles. I can't comment on code quality, but the feature is really nice. Thanks!

[Mc (mc...)]

pushed in r14946, thanks !