← Back to team overview

registry team mailing list archive

[Bug 349914] Re: pango 1.24 breaks pangofc-fontmap API and breaks mozilla build

 

Launchpad has imported 43 comments from the remote bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=478871.

If you reply to an imported comment from within Launchpad, your comment
will be sent to the remote bug automatically. Read more about
Launchpad's inter-bugtracker facilities at
https://help.launchpad.net/InterBugTracking.

------------------------------------------------------------------------
On 2009-02-17T13:48:07+00:00 Kairo-kairo wrote:

Yesterday, I upgraded my system to the newest openSUSE Factory, which
now includes pango 1.23.0, and I get that compile error on both mozilla-
central and 1.9.1:

c++ -o gfxPangoFonts.o -c -I../../../dist/include/system_wrappers -include /mnt/mozilla/hg/comm-central/mozilla/config/gcc_hidden.h -DIMPL_THEBES -DMOZILLA_INTERNAL_API -DMOZ_SUITE=1 -DSUITE_USING_XPFE_DM=1 -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux  -I/mnt/mozilla/hg/comm-central/mozilla/gfx/thebes/src -I. -I../../../dist/include/cairo -I../../../dist/include/string -I../../../dist/include/pref -I../../../dist/include/xpcom -I../../../dist/include/unicharutil -I../../../dist/include/lcms -I../../../dist/include/locale -I../../../dist/include   -I../../../dist/include/thebes -I/mnt/mozilla/build/seamonkey/mozilla/dist/include/nspr     -I/mnt/mozilla/build/seamonkey/mozilla/dist/sdk/include    -fPIC   -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -pedantic -march=pentium4 -mtune=nocona -msse2 -msse3 -mssse3 -mfpmath=sse -fno-strict-aliasing -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os -freorder-blocks -fno-reorder-functions  -I/mnt/mozilla/build/seamonkey/mozilla/dist/include/cairo -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtk-unix-print-2.0   -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2     -DMOZILLA_CLIENT -include ../../../mozilla-config.h -Wp,-MD,.deps/gfxPangoFonts.pp /mnt/mozilla/hg/comm-central/mozilla/gfx/thebes/src/gfxPangoFonts.cpp
/mnt/mozilla/hg/comm-central/mozilla/gfx/thebes/src/gfxPangoFonts.cpp: In function 'void gfx_pango_font_map_class_init(gfxPangoFontMapClass*)':
/mnt/mozilla/hg/comm-central/mozilla/gfx/thebes/src/gfxPangoFonts.cpp:1833: error: 'struct _PangoFcFontMapClass' has no member named 'context_substitute'
/mnt/mozilla/hg/comm-central/mozilla/gfx/thebes/src/gfxPangoFonts.cpp:1834: error: invalid conversion from 'PangoFcFont* (*)(PangoFcFontMap*, PangoContext*, const PangoFontDescription*, FcPattern*)' to 'PangoFcFont* (*)(PangoFcFontMap*, PangoFcFontKey*)'
gmake[7]: *** [gfxPangoFonts.o] Error 1
gmake[7]: *** Waiting for unfinished jobs....
gmake[7]: Leaving directory `/mnt/mozilla/build/seamonkey/mozilla/gfx/thebes/src'
gmake[6]: *** [libs] Error 2
gmake[6]: Leaving directory `/mnt/mozilla/build/seamonkey/mozilla/gfx/thebes'
gmake[5]: *** [libs] Error 2
gmake[5]: Leaving directory `/mnt/mozilla/build/seamonkey/mozilla/gfx'
gmake[4]: *** [libs_tier_gecko] Error 2
gmake[4]: Leaving directory `/mnt/mozilla/build/seamonkey/mozilla'
gmake[3]: *** [tier_gecko] Error 2
gmake[3]: Leaving directory `/mnt/mozilla/build/seamonkey/mozilla'
gmake[2]: *** [default] Error 2
gmake[2]: Leaving directory `/mnt/mozilla/build/seamonkey/mozilla'
gmake[1]: *** [default] Error 2
gmake[1]: Leaving directory `/mnt/mozilla/build/seamonkey'
gmake: *** [build] Error 2


http://svn.gnome.org/viewvc/pango/tags/PANGO_1_23_0/pango/pangofc-fontmap.h?view=log tells me they removed context_substitute in revision 2804, ultimately (in rev. 2808) with fontset_key_substitute, the final 1.23.0 definition is in http://svn.gnome.org/viewvc/pango/tags/PANGO_1_23_0/pango/pangofc-fontmap.h?revision=2830&view=markup#l174

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/0

------------------------------------------------------------------------
On 2009-02-18T04:59:23+00:00 M-kato wrote:

Created attachment 362845
patch v0

1.23 series is unstable version.

Also, I don't test this patch on older version.  After that, I will send
a review.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/1

------------------------------------------------------------------------
On 2009-02-18T15:23:48+00:00 Kairo-kairo wrote:

It sounds reasonable for a development distribution to pick up an
unstable version of that library, I guess. And that's why I'm using that
stuff: Better to catch and fix our code for that change now than when it
comes up in the stable version ;-)

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/2

------------------------------------------------------------------------
On 2009-02-19T03:06:13+00:00 M-kato wrote:

The latest snapshot of fedora rawhide (Fedora 11?) uses Pango-1.23.  So,
some distributions may use 1.23 series.  And, the API Document says that
these APIs are from 1.24 (stable) series.

So, we need fix this even if API is changed by later release.  (I don't
know release data of 1.24.  1.23 was this month.)

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/3

------------------------------------------------------------------------
On 2009-02-20T00:03:08+00:00 Kairo-kairo wrote:

I can confirm that my builds (both 1.9.1-based and mozilla-central)
compile and work with this patch and Pango 1.23.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/4

------------------------------------------------------------------------
On 2009-02-25T01:46:55+00:00 M-kato wrote:

Created attachment 364007
patch v1

add comment

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/5

------------------------------------------------------------------------
On 2009-03-02T07:23:20+00:00 Ginn-chen wrote:

Since "// context_substitute and get_font are not likely to be used" 
and from pango header both context_substitute and fontset_key_substitute may be NULL.
I think the right fix should be remove these functions from mozilla code. Use NULL.
Otherwise we may introduce crashes if Firefox is compiled and run with different versions of pango.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/6

------------------------------------------------------------------------
On 2009-03-26T03:47:36+00:00 M-kato wrote:

Since stable version (1.24) is released, I analyse this.

I tested with pango 1.24, gfx callback function of fontset_key_subtitute
and get_font are never called.

Becaues, the following reasons.  So these call functions are never
called.

- get_font and foreach in gfx_pango_fontset_class_init() is implemented.
- get_resolution in gfx_pango_font_map_class_init() is implemented.

So I will rewrite fix...

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/7

------------------------------------------------------------------------
On 2009-03-27T10:44:49+00:00 M-kato wrote:

Created attachment 369648
patch v2

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/8

------------------------------------------------------------------------
On 2009-03-28T03:16:03+00:00 KUROSAWA Takeshi wrote:

*** Bug 485597 has been marked as a duplicate of this bug. ***

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/12

------------------------------------------------------------------------
On 2009-03-28T09:47:16+00:00 Kairo-kairo wrote:

Comment on attachment 369648
patch v2

>+    if (pango_version() < PANGO_VERSION_ENCODE(1,23,0)) {
>+#if PANGO_VERSION_CHECK(1,23,0)

When do we get inside here to that code? The outer if says we have pango
< 1.23, right?

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/13

------------------------------------------------------------------------
On 2009-03-29T23:15:54+00:00 Wade Menard wrote:

*** Bug 481193 has been marked as a duplicate of this bug. ***

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/14

------------------------------------------------------------------------
On 2009-03-30T03:04:52+00:00 Josh Aas wrote:

This bug also causes bustage on Ubuntu 9.04 Beta.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/15

------------------------------------------------------------------------
On 2009-03-30T04:29:03+00:00 Ginn-chen wrote:

(In reply to comment #10)
> (From update of attachment 369648 [details])
> >+    if (pango_version() < PANGO_VERSION_ENCODE(1,23,0)) {
> >+#if PANGO_VERSION_CHECK(1,23,0)
> 
> When do we get inside here to that code? The outer if says we have pango <
> 1.23, right?

The outer checking is runtime checking.
The inner checking is a compile-time checking.

I think it might be less confusion if we include a private structure
declaration of PangoFcFontMapClass.

Also I'm wondering if context_substitute and create_font are really
called in Firefox with Pango < 1.23.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/16

------------------------------------------------------------------------
On 2009-03-30T04:54:44+00:00 M-kato wrote:

(In reply to comment #10)
> (From update of attachment 369648 [details])
> >+    if (pango_version() < PANGO_VERSION_ENCODE(1,23,0)) {
> >+#if PANGO_VERSION_CHECK(1,23,0)
> 
> When do we get inside here to that code? The outer if says we have pango <
> 1.23, right?

see Ginn's comment. (comment #13)

(In reply to comment #10)
> Also I'm wondering if context_substitute and create_font are really called in
> Firefox with Pango < 1.23

I don't confirm it, yet.  But I have checked code of version 1.23 and
1.24.  Should we check all version of source code? (1.14.0 or greater??)

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/17

------------------------------------------------------------------------
On 2009-03-31T04:46:56+00:00 Karlt wrote:

Comment on attachment 369648
patch v2

create_font and context_key_substitute are not currently used but are provided for a
complete PangoFcFontMap implementation.  This is meant to protect from future
changes to Pango that may result in these functions being used, or, less
likely, shaper modules choosing to use these functions.

create_font can be NULL but in that case "new_font() is used", and should not
be NULL.  Building two different create_font implementations and choosing at
runtime seems unappealing, so I suggest (unless Behdad suggests otherwise)
to change create_font() to new_font().  context and desc are not used anyway
and there is some history of maintaining backward compatibility for the
deprecated new_font().

context/fontset_key_substitute is probably even less likely to be used, but similarly a
default_substitute implementation would be the same for pre/post 1.23.
Without a context, defaults would need to be used for size and usePrinterFont.
usePrinterFont false is probably the better option, and pangocairo uses 18.0
for the default size, so let's do the same.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/18

------------------------------------------------------------------------
On 2009-03-31T14:55:24+00:00 Mozilla-behdad wrote:

Karl's analysis is accurate.  Although in some distant future when pango
1.24 can be assume, would be nice to switch to the new API.  The change
was necessary to get the PangoContext argument out of the callbacks to
enable a range of optimizations.  In general I don't like breaking
backend API.  Sorry that happened this time.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/20

------------------------------------------------------------------------
On 2009-03-31T17:24:46+00:00 Vladimir Vukicevic wrote:

Karl, anything additional needed with your patch?

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/21

------------------------------------------------------------------------
On 2009-04-03T02:06:51+00:00 Daniel Holbert wrote:

(In reply to comment #0)
> Yesterday, I upgraded my system to the newest openSUSE Factory, which now
> includes pango 1.23.0, and I get that compile error 

FWIW: this bug affects Ubuntu 9.04 ("Jaunty") as well, which is going to
ship later this month and includes pango version 1.24.0.  There's a bug
filed (linked to this bug) in Ubuntu's bug-tracker at
https://bugs.launchpad.net/bugs/349914

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/22

------------------------------------------------------------------------
On 2009-04-03T02:09:53+00:00 Chris Double wrote:

It also affects Arch Linux, current version. When you do a pacman update
you get pango version 1.24.0.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/23

------------------------------------------------------------------------
On 2009-04-06T18:36:38+00:00 Zack Weinberg wrote:

pango 1.24.0 is now in Debian unstable as well.  Makoto Kato's patch
gets me compiling again.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/25

------------------------------------------------------------------------
On 2009-04-07T02:15:23+00:00 M-kato wrote:

Created attachment 371367
patch v3

remove unnecessary callback functions
after testing this, send review.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/26

------------------------------------------------------------------------
On 2009-04-07T03:04:06+00:00 Karlt wrote:

Comment on attachment 371367
patch v3

Sorry, I missed some comments here.
(I thought I'd CC'd myself and Behdad, but I guess I didn't.)

This patch works around the compile error, but we should provide at
least fcfontmap_class->new_font if we don't provide create_font.

That would simply involve changing gfx_pango_font_map_create_font to
gfx_pango_font_map_new_font and modifying the parameters appropriately.

And we might as well change gfx_pango_font_map_context_substitute to
gfx_pango_font_map_default_substitute and use default values from
comment 15 where the information is not available for
fcfontmap_class->default_subsitute.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/27

------------------------------------------------------------------------
On 2009-04-07T03:27:02+00:00 M-kato wrote:

(In reply to comment #22)

Karl, do you have test case that context_substitute or create_font is
called?

Even if pango is 1.14 (min version required gecko) in CentOS 5, these callbacks are never called on any case.
Although default_substitute (pango_fc_default_substitute) will call from pango_fc_font_map_get_patterns/pango_fc_font_map_get_resolution into pango, since gfx implementation has a lot of callback functions such as load_font and get_resolution, it is never called.

I would like to know the environment that context_substitute or
create_font is really called.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/28

------------------------------------------------------------------------
On 2009-04-07T04:43:04+00:00 Karlt wrote:

(In reply to comment #23)
> Karl, do you have test case that context_substitute or create_font is called?

No.  As far as I have seen no current Pango version will call these
methods, given the way we use Pango and the PangoFontMap methods that we
override (as you point out).

However, the PangoFcFontMap API docs say that a PangoFcFontMap should
provide these functions.  We create a PangoFcFontMap and pass it to
Pango, so it would be reasonable for any future version of Pango or any
shaper modules to expect these methods to be present.

For example, a call to pango_font_map_load_fontset with a fresh PangoContext that does not have a gfxPangoFontGroup on it will result in a call to these methods.
You can test this by replacing GetFontGroup(context) with NULL in gfx_pango_font_map_load_fontset.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/29

------------------------------------------------------------------------
On 2009-04-07T12:50:02+00:00 John Vivirito wrote:

This bug doesn't only apply to Seamonkey does it?

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/30

------------------------------------------------------------------------
On 2009-04-07T15:26:39+00:00 Zack Weinberg wrote:

No, it's in core gfx, so it applies to everything.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/31

------------------------------------------------------------------------
On 2009-04-08T13:32:04+00:00 John Vivirito wrote:

Zack thanks

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/32

------------------------------------------------------------------------
On 2009-04-08T22:35:16+00:00 Mozilla-behdad wrote:

(In reply to comment #24)

> However, the PangoFcFontMap API docs say that a PangoFcFontMap should provide
> these functions.  We create a PangoFcFontMap and pass it to Pango, so it would
> be reasonable for any future version of Pango or any shaper modules to expect
> these methods to be present.

If your concern future pango versions, then I suggest adding a compile-
time version check on pango and setting the new methods instead of the
deprecated new_font() and default_substitute().

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/33

------------------------------------------------------------------------
On 2009-04-08T22:52:52+00:00 Karlt wrote:

(In reply to comment #28)
> If your concern future pango versions, then I suggest adding a compile-time
> version check on pango and setting the new methods instead of the deprecated
> new_font() and default_substitute().

We can use the new methods, but, if we do that, we need to have pre- and
post-1.24 versions of the methods (regardless of the compile-time
version), and do a run-time Pango version check.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/34

------------------------------------------------------------------------
On 2009-04-08T22:55:30+00:00 Mozilla-behdad wrote:

(In reply to comment #29)
> (In reply to comment #28)
> > If your concern future pango versions, then I suggest adding a compile-time
> > version check on pango and setting the new methods instead of the deprecated
> > new_font() and default_substitute().
> 
> We can use the new methods, but, if we do that, we need to have pre- and
> post-1.24 versions of the methods (regardless of the compile-time version), and
> do a run-time Pango version check.

Thinking about it again, you're right.  If you don't need the context
information anyway, just use the new_font() / default_substitute()
callbacks.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/35

------------------------------------------------------------------------
On 2009-04-13T05:53:47+00:00 Vladimir Vukicevic wrote:

We need a fix for this, but won't block the release for it; if this API
changed, worst case the linux distros will need to ship a patch to fix
it if we don't get some kind of patch in.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/36

------------------------------------------------------------------------
On 2009-04-14T23:39:40+00:00 Karlt wrote:

Created attachment 372739
use new_font
[Checkin: Comment 39 & 42]

Using new_font and default_substitute as discussed above.
This needs testing against Pango-1.24.  (I've tested Pango-1.22.4.)

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/37

------------------------------------------------------------------------
On 2009-04-15T08:50:06+00:00 FredBezies wrote:

It works here with my ubuntu linux jaunty beta up-to-date.

fred@fredo-ubuntu:~$ apt-cache show libpango1.0-0
Package: libpango1.0-0
Priority: optional
Section: libs
Installed-Size: 980
Maintainer: Ubuntu Core Developers <ubuntu-devel-discuss@xxxxxxxxxxxxxxxx>
Original-Maintainer: Sebastien Bacher <seb128@xxxxxxxxxx>
Architecture: amd64
Source: pango1.0
Version: 1.24.1-0ubuntu1

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/38

------------------------------------------------------------------------
On 2009-04-15T15:41:56+00:00 Zack Weinberg wrote:

WFM also with libpango 1.24.0-3 (debian unstable).

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/40

------------------------------------------------------------------------
On 2009-04-15T21:39:25+00:00 Jfelps wrote:

WFM with pango 1.24.1 on Mozilla/5.0 (X11; U; Linux x86_64; en-US;
rv:1.9.1b4pre) Gecko/20090415 Firefox/3.5b4pre.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/41

------------------------------------------------------------------------
On 2009-04-16T12:39:28+00:00 Henning-henkel wrote:

Building Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b4pre)
Gecko/20090416 Lightning/1.0pre Shredder/3.0b3pre from Mercurial on
ArchLinux with Pango 1.24.0-2 worked with the patch for me as well.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/42

------------------------------------------------------------------------
On 2009-04-17T19:39:50+00:00 John Vivirito wrote:

I tried version 3 patch and it failed to apply in Sm 2 version 2 applies
fine and fixes the FTB

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/43

------------------------------------------------------------------------
On 2009-04-17T19:55:52+00:00 Daniel Holbert wrote:

(In reply to comment #37)
> I tried version 3 patch and it failed to apply in Sm 2

Note that "patch v3" is marked as obsolete -- the "use new_font" patch
is the one that's got r+sr & [needs landing].  Give that one a try -- it
applies fine in mozilla-central and mozilla-1.9.1.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/44

------------------------------------------------------------------------
On 2009-04-17T19:57:58+00:00 Daniel Holbert wrote:

"use new_font" patch landed in m-c:
http://hg.mozilla.org/mozilla-central/rev/4aed53dcf692

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/45

------------------------------------------------------------------------
On 2009-04-17T20:04:11+00:00 Daniel Holbert wrote:

Comment on attachment 372739
use new_font
[Checkin: Comment 39 & 42]

Requesting approval1.9.1 for this bug's patch (to fix a messy compile
error on newer linux distros).

Note that comment 31 (in which this was set to blocking1.9.1- &
wanted1.9.1+) indicates that this *needs* fixing on 1.9.1, but it just
wouldn't hold the release.  Now we've got a fix, so it should hopefully
land on 1.9.1 after baking a bit on mozilla-central.

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/46

------------------------------------------------------------------------
On 2009-04-22T15:34:51+00:00 Beltzner wrote:

Comment on attachment 372739
use new_font
[Checkin: Comment 39 & 42]

a191=beltzner

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/47

------------------------------------------------------------------------
On 2009-04-22T16:38:51+00:00 Sgautherie-bz wrote:

Comment on attachment 372739
use new_font
[Checkin: Comment 39 & 42]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9de5cf733d70

Reply at: https://bugs.launchpad.net/pango/+bug/349914/comments/48


** Changed in: pango
   Importance: Unknown => Medium

-- 
pango 1.24 breaks pangofc-fontmap API and breaks mozilla build
https://bugs.launchpad.net/bugs/349914
You received this bug notification because you are a member of Registry
Administrators, which is the registrant for Pango.