use of STL extension hash_map.h, osf1/Tru64

[Enchanter]

I'm trying to build inkscape (0.41 and/or recent CVS) on
alphaev56-dec-osf 5.1b (Tru64 UNIX 5.1b) with the
vendor toolchain.

Building inkscape fails on gradient-toolbar.cpp,
because verbs.h includes hash_map.h.

Tru64 has a good C++ compiler and the stock STL, but it
doesn't include any of the STL extensions (like
hash_map/hash_map.h).

Using g++ to build inkscape would be a last resort, because

- all of the prerequisite libraries have been built
with the vendor cxx compiler, and mixing gcc and
non-gcc libraries is known to sometimes cause stability
problems (and would likely be worse for C++ than stock C).

- executables and libraries built with gcc/g++ are
often significantly slower on commercial UNIXes than
executables/libraries produced with the native
compiler. This is definitely true for Tru64. The
vendor cc/cxx produce much more optimized code in most
cases.

With that in mind, is there any way to work around the
abscense of hash_map.h? Comments in verbs.h seem to
indicate that switching to GHashTables is planned,
which would (I think) get rid of the dependency on
hash_map.h.
What kind of a priority is that, and any idea how much
work it would be?

[Buliabyak-users (buliabyak-users)]

Ted, I think it's your code.

Would a simple std::map work?

[ScislaC (scislac)]

Does this still happen to you?

[Ted Gould (ted)]

Thanks for the report, this has been updated in CVS. We are
now using std::map instead of gnu::hash_map. We would love
to know if Inkscape will work on Tru64 now.

[Enchanter (enchanter)]

I'll grab a CVS snapshot and test this week, and report back.

Thanks much!

Tim

[Enchanter (enchanter)]

Although verb.h no longer uses hash_map, there are
unfortunately still other places that use ext/hash_map
and/or require the GNU hash_map, specifically:

  libnrtype/FontFactory.h
  libnrtype/RasterFont.h
  libnrtype/font-instance.h

My C++ is weak -- I learned C++ in the early 90's, and
obviously a lot has changed. Is the process of converting
from a hash_map to a std::map pretty easy, or at least
well-documented somewhere? If so, I would be willing to
give it a try.

Also, is there an upstream version of the NR libraries? I'm
just wondering if the Inkscape versions of libnr and
libnrtype are being kept close to some master copy of those
libraries, or if they've diverged significantly.

[Ted Gould (ted)]

Passing this bug along. Mental, I think you did the
hash_map stuff in libnrtype, right?

[Enchanter (enchanter)]

Based on what had been done to verbs.h to fix the original
problem, I had hoped that it was as simple as changing the
include and modifying instances of

  __gnu_cxx::hash_map

to std::map, but it's not. I tried that approach, and ended
up with quite a lot of errors.

[Enchanter (enchanter)]

I've been brushing up on my C++ to try contribute a bit more
here, and I think I've managed to convert the code in
libnrtype to use std::map instead of __gnu_cxx::hash_map.

I'll upload a patch in a minute, please review my changes
and verify that I haven't done anything stupid. Briefly, my
patch does the following (in src/libnrtype only):

- change all instances of #include <ext/hash_map> to
#include <map>
- change all instances of __gnu_cxx::hash_map to std::map
- in the cases where a __gnu_cxx::hash_map template with
four parameters was being declared, remove the third
template parameter (the hashing function), moving what had
been the fourth parameter (the equality test) into the third
argument.
- in the declaration and definition of the equality test
function, add "const" to the end of the declaration/defintion.

- the two hashing function/classes (font_descr_hash,
font_style_hash) are no longer needed. My patch is a bit
inconsistent here -- I removed the declaration of
font_descr_hash from FontFactory.h and the definition of the
same from FontFactory.cpp, but I just commented
FontInstance.cpp, indicating that the hashing function isn't
needed anymore -- I didn't actually remove it or its
declaration from font-instance.h.

Please review my patch, and let me know if there's anything
that needs changing. If you approve of the patch in
general, I can submit a secondary patch that also removes
the declaration and definition for font_style_hash, rather
than leaving it there as dead code.

[Mental-users (mental-users)]

Sorry for the lack of follow-up -- I tried the patch and I
get a lot of unrefFace failures in font_factory::UnrefFace.

[Bryce Harrington (bryce)]

Is this patch still relevant? It's been sitting in the patch tracker a long time, and it would be nice to reach a resolution with it.

[MenTaLguY (mental-deactivatedaccount)]

I'm afraid it's still an issue, although the patch may have bitrotted at
this point. The remaining uses are still libnrtype/FontFactory.h and
libnrtype/font-instance.h.

It's probably sufficient to replace them with std::map, which is only
O(log(n)) rather than O(n), although as I recall some additional work
may also be required to make the key types suitable for use with
std::map.

-mental

[Bryce Harrington (bryce)]

Okay, this sounds non-critical for the release so we can skip it.
At some point it would be good if someone would refresh the patch to use std::map instead of hash_map.
(Or perhaps this code will go away once we convert to Cairo.)

[Jaspervdg (jaspervdg)]

As far as I could see the problem was that the comparison function needed by hash_map is an equality test, while map needs a less-than comparison. I attached a new patch to replace hash_map by map. It should work fine, but as I'm not very familiar with this part of the code it would be nice if someone who actually knows what all the different fields mean double-checks the code to see if the comparisons make any sense.

[ScislaC (scislac)]

Closing this as Fix Committed as comparisons of the patch to existing code make it appear that the code changes to std::map has happened at some point already. I'm assuming it may have taken place during text tool rewrite or cairofication (setting to Fix Committed and 0.49 rather than Released). If this bug was closed in error, please feel free to reopen it.