Closed Bug 947379 Opened 6 years ago Closed 6 years ago

Remove MOZ_PANGO

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

Attached patch Remove MOZ_PANGO (obsolete) — Splinter Review
We I don't believe we've been able to build without MOZ_PANGO and with MOZ_WIDGET_GTK since 2008. It feels like it's time for MOZ_PANGO to go.
Attachment #8343913 - Attachment is patch: true
Attachment #8343913 - Attachment mime type: text/x-patch → text/plain
Attachment #8343913 - Flags: review?(jfkthame)
Aww... when I saw this bug's title, my first thought was that you were going to rip out Pango support altogether.

Yes, AFAIK building on Linux/Gtk with --disable-pango has been broken for quite a while. We've looked at this a bit, e.g. in bug 703042 and bug 754506, but never actually spent the time to finish the job. (Sorry!)

Before we decide to do this, I'd like to check with Oleg: do you still have any interest or use-case for the --disable-pango option, or can we assume that it's no longer important, as you haven't been nagging me about it? :)
Flags: needinfo?(romaxa)
Build without pango is not important for me anymore, the only case where it might be needed is LinuxGL port (where I would like to use as minimum dependencies as possible), but with that port we can use Android font backend pretty much as is.

So I'm fine with this define removal.
Flags: needinfo?(romaxa)
Comment on attachment 8343913 [details] [diff] [review]
Remove MOZ_PANGO

Review of attachment 8343913 [details] [diff] [review]:
-----------------------------------------------------------------

OK, let's do it. Less code, more better. :)

Bonus points for a followup bug to entirely remove our dependency on pango, and rename gfxPangoFonts to gfxFcFonts or something like that. I think we're basically only using it for some pango_language APIs these days; would be nice if we could drop it altogether.
Attachment #8343913 - Flags: review?(jfkthame) → review+
Hmm, just a sec - shouldn't you also be removing the --disable-pango option from configure?
(In reply to Jonathan Kew (:jfkthame) from comment #3)
> Comment on attachment 8343913 [details] [diff] [review]
> Remove MOZ_PANGO
> 
> Review of attachment 8343913 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> OK, let's do it. Less code, more better. :)
> 
> Bonus points for a followup bug to entirely remove our dependency on pango,
> and rename gfxPangoFonts to gfxFcFonts or something like that. I think we're
> basically only using it for some pango_language APIs these days; would be
> nice if we could drop it altogether.

The only interesting part that we're using is:
pango_language_includes_script() followed by pango_script_get_sample_language(). This gives us a language string that we can pass to Fontconfig to do the lookups. Is there something different/better we can do to replace this.
Attached patch Remove MOZ_PANGOSplinter Review
This is with the configure changes.
Attachment #8343913 - Attachment is obsolete: true
(In reply to Jonathan Kew (:jfkthame) from comment #3)
> and rename gfxPangoFonts to gfxFcFonts or something like that.

That would make sense.

> I think we're
> basically only using it for some pango_language APIs these days; would be
> nice if we could drop it altogether.

We're going to load libpango* anyway because GTK depends on them, and we will need Pango to get system font info from GTK.
https://hg.mozilla.org/mozilla-central/rev/4ab015ae5eaf
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.