GNOME Bugzilla – Bug 795045
More mingw build fixes
Last modified: 2018-04-09 13:49:35 UTC
Created attachment 370624 [details] [review] build: move usp10 before gdi32 Without this pango on mingw64 tries to lookup up the Script* functions in gdi32 and fails. It already fails at the build stage because the introspection dump crashes with a missing entry point error. Moving usp10 before gdi32 makes things work. This might be related to the warning in the uniscribe docs: "Important Starting with Windows 8: To maintain the ability to run on Windows 7, a module that uses Uniscribe must specify Usp10.lib before gdi32.lib in its library list."
Created attachment 370625 [details] [review] pangowin32: fix script cache hash key for 64bit builds It joins the HFONT and script key to a gint64 and uses this as a hash key, but on 64bit Windows hfont doesn't fit in 32bit. This also breaks the build with meson where -Werror=pointer-to-int-cast is enabled by default. Instead of using the gint64 hash functions add our own key type and implement matching hash and equality functions.
Created attachment 370660 [details] [review] meson: skip pangoxft headers for the Windows gtk-doc build The API isn't available on Windows and gtk-doc fails with a linker error otherwise.
Review of attachment 370624 [details] [review]: Fine for me but I would probably add a comment in the meson.build file as well...
Review of attachment 370660 [details] [review]: Fine for me
Review of attachment 370625 [details] [review]: Apart from the coding style which need some fixing, please have a look at the comment inline. ::: pango/pangowin32-shape.c @@ +379,3 @@ +script_cache_key_hash_func (script_cache_key *key) { + gint64 temp = key->script; + return g_direct_hash (key->hfont) ^ g_int64_hash (&temp); As far as I understand we are doing all of this to use 64 bit for hfont but then here you are using g_direct_hash which returns a guint, also you do int64_hash of the script while the script is a gint32, should they be reversed?
(In reply to Ignacio Casal Quinteiro (nacho) from comment #5) > Review of attachment 370625 [details] [review] [review]: > > Apart from the coding style which need some fixing, please have a look at > the comment inline. > > ::: pango/pangowin32-shape.c > @@ +379,3 @@ > +script_cache_key_hash_func (script_cache_key *key) { > + gint64 temp = key->script; > + return g_direct_hash (key->hfont) ^ g_int64_hash (&temp); > > As far as I understand we are doing all of this to use 64 bit for hfont but > then here you are using g_direct_hash which returns a guint, also you do > int64_hash of the script while the script is a gint32, should they be > reversed? (disclaimer: First time using the hash API). GHashFunc uses guint for the hash as a return value as do all builtin hash functions. The hashes don't have to be unique, so this shouldn't matter. I'm using g_int64_hash because there exists no g_int32_hash in glib and using the existing one seemed preferable.
You have g_int_hash which is basically the same as int32_hash
(In reply to Ignacio Casal Quinteiro (nacho) from comment #7) > You have g_int_hash which is basically the same as int32_hash In practice, yes. Should I change it?
I think it would be more clear yes. Also, do we need to use int64_hash for the font instead of direct hash?
> I think it would be more clear yes ok > Also, do we need to use int64_hash for the font instead of direct hash? I don't think so, HFONT is a pointer type.
OK, just double checking with you since that is what I understood from the commit message. Maybe change the commit message to clarify a bit better.
Comment on attachment 370624 [details] [review] build: move usp10 before gdi32 pushed and added a comment in meson.build pointing to this bug
Created attachment 370692 [details] [review] pangowin32: fix script cache hash key for 64bit builds * Mentioned that HFONT is a pointer type in the commit message * Use g_int_hash instead of g_int64_hash * Coding style fixes
Review of attachment 370692 [details] [review]: Fix the style issue and go ahead with it. ::: pango/pangowin32-shape.c @@ +383,3 @@ + +static gboolean +script_cache_key_equal_func (script_cache_key *a, script_cache_key *b) one param per line
Done, thanks! MSYS2 now uses meson to build pango: https://github.com/Alexpux/MINGW-packages/pull/3570