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 733882 - Kill Pango modules, engines, and config files
Kill Pango modules, engines, and config files
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: win32
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
pango-maint
Depends on:
Blocks:
 
 
Reported: 2014-07-28 17:46 UTC by Behdad Esfahbod
Modified: 2015-04-10 17:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pangowin32-fontmap.c: Don't use pango.aliases config file (2.31 KB, patch)
2014-12-30 08:06 UTC, Fan, Chun-wei
none Details | Review

Description Behdad Esfahbod 2014-07-28 17:46:28 UTC
+++ This bug was initially created as a clone of Bug #733137 +++

Make Pango a simple library!

Initial work from Khaled is in kill-dynamic-modules branch.  I'll push out more soon.
Comment 1 Behdad Esfahbod 2014-07-28 23:55:52 UTC
Pushed some more in kill-modules branch.
Comment 2 Behdad Esfahbod 2014-07-29 00:20:09 UTC
Since GNOME git server doesn't allow non-fast-forward push, I moved the branch to git:

  https://github.com/behdad/pango/commits/kill-modules
Comment 3 Matthias Clasen 2014-07-29 10:30:27 UTC
you can delete the branch and push it again
Comment 4 Behdad Esfahbod 2014-07-29 14:54:30 UTC
Yes, I did that a couple of times, but it will send tens of email to commit-list every time.  I expect to merge this really soon, so doesn't matter.

A small report: for the first time ever, I can actually compile and test pangowin32 and pangocoretext myself.  Found a really embarrassing bug in each the moment I got them to run.  Sigh.  I'll be removing lots more abstraction and hardcode harfbuzz shaper across the board soon, but that can happen after the merge.
Comment 5 Khaled Hosny 2014-09-27 14:41:07 UTC
Behdad, did you have any luck with this.
Comment 6 Behdad Esfahbod 2014-09-29 13:15:04 UTC
I've been travelling and just getting back home today.  Will get this rolling again.  Matthias suggested that we branch stable and merge the kill branch and go from there.  I'll do that soon.
Comment 7 Fan, Chun-wei 2014-12-30 08:06:37 UTC
Created attachment 293465 [details] [review]
pangowin32-fontmap.c: Don't use pango.aliases config file

Hi,

This is a first-step patch to PangoWin32 that we stop using the pango.aliases config file.  It would, though, be ideal, if we can make use of the Windows APIs (Uniscribe, or even better, DirectWrite, the successor of Uniscribe), or have HarfBuzz do the font selection for the text.

p.s. I do understand that there was a request to move PangoWin32 to use DirectWrite, but from what I read from it, the intention is to use Harfbuzz (which, AFAIK uses Uniscribe itself when available).  Perhaps Behdad is well-ahead of me at this point though in regards to PangoWin32, so I wouldn't be too sad if this patch was rejected because of this. :)

With blessings, thank you, and Happy New Year!
Comment 8 Behdad Esfahbod 2015-04-05 02:51:38 UTC
Ok, I think the branch is ready:

  https://github.com/behdad/pango/commits/kill-modules

Please test.  In particular, I haven't tested Thai line breaking (and Arabic / Indic cursoring).

This results in 2.6k net removed lines.

Unfortunately I found that the win32 build stuff have had broken "make distcheck" for quite a while, so had to nuke some of that so I can test.
Comment 9 Khaled Hosny 2015-04-05 17:35:32 UTC
I did some testing (editing an Arabic file in Gedit) and all is fine so far.
Comment 10 Behdad Esfahbod 2015-04-05 19:27:15 UTC
Thanks Khaled.

Since without modules, building pango on Windows, etc, is much easier now, I like to see if we can remove the msvc build system or at least drastically simplify it.  Chun-wei, what do you think?

I guess my question really is: why do you need to build using MSVC, as opposed to, say, various GNU toolchain configurations?

I'll go ahead and merge this in master, we can fix things up there.
Comment 11 Behdad Esfahbod 2015-04-05 20:04:07 UTC
In master now!

commit 3a510689764e54516b309be1c542ea0f2263d177
Merge: 2ae5739 f734d7c
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Sun Apr 5 12:58:46 2015 -0700

    Merge branch 'kill-modules'
    
    Bug 733882 - Kill Pango modules, engines, and config files
    
    This removes use of gmodule modules, config files, and deprecates
    engine and module APIs, although for compatibility reasons, those
    are still used internally to wire things up.  ~2500 lines gone.
Comment 12 Dieter Verfaillie 2015-04-05 21:25:43 UTC
(In reply to Behdad Esfahbod from comment #10)
> I guess my question really is: why do you need to build using MSVC, as
> opposed to, say, various GNU toolchain configurations?

One reason would be to maintain the ability of building a stack using the same msvcrX.dll version as used by the official Python windows binaries. Language bindings are already hard enough as they are without the messy business of mixing multiple c runtimes in the same address space (bringing with it the joys of fileno translations, multiple env var tables, etc)...
Comment 13 Khaled Hosny 2015-04-05 22:04:53 UTC
I pushed a fix for building with libthai, but now I’m getting a test failure in testboundaries.
Comment 14 Khaled Hosny 2015-04-05 22:13:56 UTC
This apparently is a regression from commit 137832fa931e875b900dedd1c8909b46d1f7e8ad.
Comment 15 Behdad Esfahbod 2015-04-05 22:18:59 UTC
Thanks.  I'll take a look.  I think I know what's going on.  Will see how hard it is to fix.
Comment 16 Behdad Esfahbod 2015-04-05 22:19:19 UTC
Humm.  For me only happens with libthai installed.  Same for you, right?
Comment 17 Fan, Chun-wei 2015-04-06 01:19:31 UTC
Hi,

(In reply to Dieter Verfaillie from comment #12)
> (In reply to Behdad Esfahbod from comment #10)
> > I guess my question really is: why do you need to build using MSVC, as
> > opposed to, say, various GNU toolchain configurations?
> 
> One reason would be to maintain the ability of building a stack using the
> same msvcrX.dll version as used by the official Python windows binaries.
> Language bindings are already hard enough as they are without the messy
> business of mixing multiple c runtimes in the same address space (bringing
> with it the joys of fileno translations, multiple env var tables, etc)...

Thanks Dieter, this a major reason for this, this removes lots of debugging that is caused by this sort of thing.

There are also few other reasons, which include:
-Faster build speeds, autotools run really slow
 on Windows, and cross-compiling is likely not an
 option for some people (and perhaps using MinGW
 itself even).  The recent addition of using
 multiprocessor support for building release
 builds makes this even more apparent.
-Debugger support for Windows, the Microsoft debugger
 is indeed more reliable and useful than GDB on Windows.
 The Microsoft debugger would not work very well with GCC
 builds, which, AFAIK, is the toolchain that most Windows
 developers in general would use.  This is where the .pdb
 files come to place.
-Better binary optimization.

There are also requests that I (and Nacho) receive to be able to build the entire GTK+ stack with Visual Studio in an all-in-one Visual Studio project set, which am I working on in guthub, so to build and use the components of the stack quickly.

About making the project file generation more simplified, I had bug 735039 there, but perhaps people didn't notice it, by adding an autotools file that can be used by various projects.  After the commits for this (733882) bug, the patches there need to be updated, which I will do in the next few days if this is the way to go.  They were checked with 'make distcheck', though...:)

p.s. I heard that we are planning to use harfbuzz-ng for all backends, and I did notice that there is Uniscribe (which PangoWin32 uses) support in there.  Would harfbuzz-ng (or PangoWin32) be the right place if we want to move the code eventually to DirectWrite, as Uniscribe is replaced by it, especially as we are likely to stop XP support this dev cycle?  I do understand that someone else requested for moving PangoWin32 to DirectWrite, but Behdad mentioned that DirectWrite support is in Cairo, but it's probably done and (only) used by the Mozilla folks, not in the Cairo releases.

With blessings, thank you!
Comment 18 Khaled Hosny 2015-04-06 02:57:26 UTC
(In reply to Behdad Esfahbod from comment #16)
> Humm.  For me only happens with libthai installed.  Same for you, right?

Yes.
Comment 19 Behdad Esfahbod 2015-04-06 20:23:38 UTC
Thanks Khaled.  Regression is fixed.
Comment 20 Behdad Esfahbod 2015-04-06 20:26:49 UTC
(In reply to Fan, Chun-wei from comment #17) 
> 
> p.s. I heard that we are planning to use harfbuzz-ng for all backends, and I

Correct.  That's what both Firefox and Chrome do these days.


> did notice that there is Uniscribe (which PangoWin32 uses) support in there.
> Would harfbuzz-ng (or PangoWin32) be the right place if we want to move the
> code eventually to DirectWrite, as Uniscribe is replaced by it, especially

No.  While HarfBuzz has a Uniscribe backend, and can use a DirectWrite backend as well, those are for testing only.  The OpenType implementation in HarfBuzz will be used, regardless of the platform implementation.


> as we are likely to stop XP support this dev cycle?  I do understand that
> someone else requested for moving PangoWin32 to DirectWrite, but Behdad
> mentioned that DirectWrite support is in Cairo, but it's probably done and
> (only) used by the Mozilla folks, not in the Cairo releases.

Right.  That's still my understanding.

> With blessings, thank you!
Comment 21 Fan, Chun-wei 2015-04-06 21:51:20 UTC
Hello Behdad,

(In reply to Behdad Esfahbod from comment #20)

Thanks for the answers...

(In reply to Fan, Chun-wei from comment #17)

> About making the project file generation more simplified, I had bug 735039
> there, ....  They were checked with 'make distcheck',
> though...:)

For double confirmation, can I know whether the approach in 735039 for the autotools part for maintaining Visual Studio support be okay with you, before I go ahead updating that, or a different approach be used (such as going back to NMake Makefiles, or so).  Can I also know if you have additional flags when running 'make distcheck', so that I can check with those flags as well, just in case.

With blessings, thank you!
Comment 22 Behdad Esfahbod 2015-04-06 23:19:01 UTC
I don't have time right now to thoroughly review the patch in bug 735039.  But as long as pango passes "make distcheck", I'm ok with it.  I can't imagine the previous code in master to have ever passed that.
Comment 23 Fan, Chun-wei 2015-04-07 10:42:24 UTC
Hello Behdad,

Sorry, apparently I thought the scripts were ok as I ran 'make distcheck' without parallelism, which passed.

I updated the patches in bug 735039, so that:
-it produces the 2008/2010 projects in
 the way you suggested last time, I.e.
 Using a common autotools module, to
 make things cleaner.
-fix the generation of the 2012/2013
 projects, to make it parallelism
 safe.
-make distcheck passes with or
 without parallelism.

With blessings.

P.s. Things do seem alright on Windows as well
Comment 24 Fan, Chun-wei 2015-04-07 14:29:35 UTC
Hi,

(In reply to Fan, Chun-wei from comment #23)
 
> P.s. Things do seem alright on Windows as well (but only on the surface).
Unfortunately looking further into things, I am now getting a bunch of:
Pango-CRITICAL **: pango_font_find_shaper: assertion 'font!= NULL' failed

which means I need to go further into the code, as a regression popped up somewhere, which caused the testiter test to fail.  Perhaps need to wire things up with pangowin32-shape.c (the former basic Win32 module)?

With blessings, thank you!
Comment 25 Behdad Esfahbod 2015-04-07 23:09:18 UTC
Thanks.  make distcheck looks good.

Humm.  The assertion seems unrelated to my changes.  Please debug and advise.
Comment 26 Fan, Chun-wei 2015-04-10 10:06:20 UTC
Hi Behdad,

(In reply to Behdad Esfahbod from comment #25)
> Humm.  The assertion seems unrelated to my changes.  Please debug and advise.

It seems to me that the assertion occurred because we were doing pango_fontset_foreach() in get_shaper_and_font() in pango-context.c, when fallbacks are being enabled, and we didn't get a proper PangoFont* after doing pango_fontset_foreach() (in which the acquired PangoFont* NULL), after 10 calls to the function pango_fontset_foreach() in the testiter test (and 22 times when we run gtk3-demo).

As the Win32 backend is the only backend that does not have a foreach() implementation, the base pango_fontset_simple_foreach() is being used for foreach(), and it seems that pango_fontset_simple_foreach() is not enough to ensure that we have a proper PangoFont* after looping through the fontset.  A workaround for this is to only get the shaper for the PangoFont* if it is not NULL, but I think the proper fix is to do a proper implementation of ->foreach, etc, for Windows (but, I am aware that we are replacing the backends with HardBuzz-NG, so may have to see how things will go along with this).

Also, most probably something like this is the cause of the pango_font_description_*() assertions when we use GDK as the Clutter backend.

With blessings, thank you!
Comment 27 Behdad Esfahbod 2015-04-10 17:16:57 UTC
Ok I see.  So I broke-up the hexboxes.  Let me try to crash on Linux and then fix.
Comment 28 Behdad Esfahbod 2015-04-10 17:17:25 UTC
Note that even when we use HarfBuzz on Windows, we still need to do font discovery and fallback separately.  HarfBuzz doesn't do any of that.
Comment 29 Behdad Esfahbod 2015-04-10 17:27:16 UTC
Fixed.