Closed Bug 550468 Opened 14 years ago Closed 14 years ago

Fix support for Pango font engine on Qt platform

Categories

(Core Graveyard :: Widget: Qt, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.3a3

People

(Reporter: romaxa, Assigned: romaxa)

References

Details

Attachments

(3 files, 2 obsolete files)

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 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+
Attached patch Updated patch (obsolete) — Splinter Review
PANGO_CHECK moved to separate MOZ_WIDGET_QT ifdef
cairo_reset moved back to first position
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
Attachment #430812 - Attachment is obsolete: true
Fixed:
http://hg.mozilla.org/mozilla-central/rev/26e6af3b3df7
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee: nobody → romaxa
Target Milestone: --- → mozilla1.9.3a3
Attachment #430952 - Flags: review?(karlt) → review+
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...
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-
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)
Blocks: 553964
Attachment #430953 - Attachment is obsolete: true
Attachment #430953 - Flags: review?(bugspam.Callek)
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.

Attachment

General

Created:
Updated:
Size: