Fix support for Pango font engine on Qt platform

RESOLVED FIXED in mozilla1.9.3a3

Status

Core Graveyard
Widget: Qt
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: romaxa, Assigned: romaxa)

Tracking

Trunk
mozilla1.9.3a3
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
Created attachment 430612 [details] [diff] [review]
Patch to enable MOZ_PANGO define for Qt platform

FT2 font engine seems not supported very well, and currently we have minor problems with crashes on exit, kerning problems e.t.c.

I think it would be nice to have option to compile Qt platform with PangoFont engine same way as in Gtk Platform.
Attachment #430612 - Flags: review?(pavlov)

Comment 1

9 years ago
Comment on attachment 430612 [details] [diff] [review]
Patch to enable MOZ_PANGO define for Qt platform

this looks pretty reasonable, but should get karlt to review
Attachment #430612 - Flags: review?(pavlov) → review?(karlt)
Comment on attachment 430612 [details] [diff] [review]
Patch to enable MOZ_PANGO define for Qt platform

>@@ -5203,7 +5203,7 @@ MOZ_ARG_DISABLE_BOOL(pango,
> dnl ========================================================
> dnl = Pango
> dnl ========================================================
>-if test "$MOZ_ENABLE_GTK2"
>+if test "$MOZ_ENABLE_GTK2" || test "$MOZ_ENABLE_QT"
> then

This will PKG_CHECK_MODULES for pango setting MOZ_PANGO* even when MOZ_PANGO
is not set.  I don't think you want that.

http://hg.mozilla.org/mozilla-central/file/c05d0ac56307/configure.in#l5217

I'm guessing the check when ! test "$MOZ_PANGO" can be removed.

>+#ifdef MOZ_PANGO
>+#define PANGO_ENABLE_BACKEND
>+#define PANGO_ENABLE_ENGINE
>+#endif

These shouldn't actually be necessary (probably left over from the past).

>-    cairo_debug_reset_static_data();
>-
>     FT_Done_FreeType(gPlatformFTLibrary);
>     gPlatformFTLibrary = NULL;
>+#endif
> 
>+    cairo_debug_reset_static_data();

IIRC the FT2 backend uses gPlatformFTLibrary to create FT_Faces from which
cairo fonts are created, so cairo_debug_reset_static_data needs to be called
first to release these fonts that remain in its caches before FT_Done_FreeType
is called.

r=karlt with those things sorted out
Attachment #430612 - Flags: review?(karlt) → review+
(Assignee)

Comment 3

9 years ago
Created attachment 430812 [details] [diff] [review]
Updated patch

PANGO_CHECK moved to separate MOZ_WIDGET_QT ifdef
cairo_reset moved back to first position
(Assignee)

Comment 4

9 years ago
Created attachment 430824 [details] [diff] [review]
Added g_type_init() if pango font engine enabled

Added g_type_init, otherwise we crash in:
#5  gfxPangoFcFont::NewFont (aRequestedPattern=0xb0171440, aFontPattern=0xb454c250)
    at gfx/thebes/src/gfxPangoFonts.cpp:587
#6  0xb73899ac in gfxFcPangoFontSet::GetFontAt (this=0xb01f11e0, i=0)
    at gfx/thebes/src/gfxPangoFonts.cpp:959
(Assignee)

Updated

9 years ago
Attachment #430812 - Attachment is obsolete: true
(Assignee)

Comment 5

9 years ago
Fixed:
http://hg.mozilla.org/mozilla-central/rev/26e6af3b3df7
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Assignee: nobody → romaxa
Target Milestone: --- → mozilla1.9.3a3
Created attachment 430952 [details] [diff] [review]
(Bv1) Make MOZ_PANGO overridable from confvars.sh, Merge duplicated code
[Checkin: Comment 9]
Attachment #430952 - Flags: review?(karlt)
Created attachment 430953 [details] [diff] [review]
(Cv1-CC) Port (the useful part of) it to comm-central, Restore (now) useful MOZ_PANGO support
[Moved to bug 553964]
Attachment #430953 - Flags: review?(bugspam.Callek)
Attachment #430952 - Flags: review?(karlt) → review+
(Assignee)

Comment 8

9 years ago
Why we are making duplicate fixes for m-c and comm-c all the time? can we just build common-central by using SDK (build tree m-c without .cpp files) from m-c?
Comment on attachment 430952 [details] [diff] [review]
(Bv1) Make MOZ_PANGO overridable from confvars.sh, Merge duplicated code
[Checkin: Comment 9]


http://hg.mozilla.org/mozilla-central/rev/9bab574bd936
Attachment #430952 - Attachment description: (Bv1) Make MOZ_PANGO overridable from confvars.sh, Merge duplicated code → (Bv1) Make MOZ_PANGO overridable from confvars.sh, Merge duplicated code [Checkin: Comment 9]
(In reply to comment #8)

> Why we are making duplicate fixes for m-c and comm-c all the time? can we just

Because, afaik, that's how it works currently.

> build common-central by using SDK (build tree m-c without .cpp files) from m-c?

I'd be happy if the process could be improved: input welcomed...
Duplicate of this bug: 549921
Comment on attachment 430953 [details] [diff] [review]
(Cv1-CC) Port (the useful part of) it to comm-central, Restore (now) useful MOZ_PANGO support
[Moved to bug 553964]

Why do we need Pango in c-c configure.in. if confvars.sh allows us to set it; and we still pass the --enable/disable args we should be safe here, no?

If there isn't really a reason, can you submit me a new patch that just drops the following from c-c.

> dnl = Pango
> dnl ========================================================
> if test "$MOZ_ENABLE_GTK2"
> then
>     PKG_CHECK_MODULES(MOZ_PANGO, pango >= $PANGO_VERSION pangoft2 >= $PANGO_VERSION)
>     AC_SUBST(MOZ_PANGO_LIBS)
> fi
Attachment #430953 - Flags: review?(bugspam.Callek) → review-
(Assignee)

Comment 13

9 years ago
I was not touching GTk2 port, if you want some changes for existing code, then file new bug with patch, and ask it for review from module owner.
Comment on attachment 430953 [details] [diff] [review]
(Cv1-CC) Port (the useful part of) it to comm-central, Restore (now) useful MOZ_PANGO support
[Moved to bug 553964]


(In reply to comment #12)

> (From update of attachment 430953 [details] [diff] [review])
> Why do we need Pango in c-c configure.in. if confvars.sh allows us to set it;
> and we still pass the --enable/disable args we should be safe here, no?

Why disallow mozconfig use?

> If there isn't really a reason, can you submit me a new patch that just drops
> the following from c-c.
> 
> > dnl = Pango
> > dnl ========================================================
> > if test "$MOZ_ENABLE_GTK2"
> > then
> >     PKG_CHECK_MODULES(MOZ_PANGO, pango >= $PANGO_VERSION pangoft2 >= $PANGO_VERSION)
> >     AC_SUBST(MOZ_PANGO_LIBS)
> > fi

This block is the very reason for this port: c-c uses MOZ_PANGO_LIBS.
http://mxr.mozilla.org/comm-central/search?string=MOZ_PANGO_LIBS&case=on
Attachment #430953 - Flags: review- → review?(bugspam.Callek)

Updated

8 years ago
Blocks: 553964

Updated

8 years ago
Attachment #430953 - Attachment is obsolete: true
Attachment #430953 - Flags: review?(bugspam.Callek)

Updated

8 years ago
Attachment #430953 - Flags: review+
Attachment #430953 - Attachment description: (Cv1-CC) Port (the useful part of) it to comm-central, Restore (now) useful MOZ_PANGO support → (Cv1-CC) Port (the useful part of) it to comm-central, Restore (now) useful MOZ_PANGO support [Moved to bug 553964]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.