Add Xft path to gfxPangoTextRun

RESOLVED FIXED in mozilla1.9alpha1

Status

Core Graveyard
GFX: Gtk
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
mozilla1.9alpha1
x86
Linux
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

It's actually really easy to use Xft if we want to. Instead of having its own textrun class, we can just use Xft for the initial text->glyph conversion, by creating a new method CreateGlyphRunsXft. All the other code can be shared since it hardly uses Pango. (OK, nsTextFrameThebes will use gfxPangoTextRun::MeasureText which currently calls into Pango, but we can worry about that later.)

The attached patch offers three modes: don't use Xft, use Xft for all-8bit-char strings and Pango for the rest, and use Xft for everything. The last one is not recommended as it will likely mangle complex text beyond recognition... In fact I would rather have us ship not using this code at all. However it will be useful for us to help measure the cost of Pango. In particular I suggest we check this in with "use Xft for all-8bit-char strings" enabled, because that's roughly what we were doing before I landed my textrun patch and regressed Linux Tp/Tp2. Also it may be useful for people for whom Pango is just too slow. We may as well all share the same Xft code and get it right.
Created attachment 252758 [details] [diff] [review]
add Xft path, enable it for 8bit strings

As described in the bug ... this gets us close to the behaviour before I checked the new textrun stuff. Not that I like using Xft, but it helps us narrow down the costs of Pango.
Attachment #252758 - Flags: review?
Attachment #252758 - Flags: review? → review?(pavlov)
I think separate Xft engine it is not very bad idea, because it can be used in cairo-xlib build...
Maybe, but we're pretty far away from supporting a cairo-xlib build.

Comment 4

11 years ago
Comment on attachment 252758 [details] [diff] [review]
add Xft path, enable it for 8bit strings

this looks fine.
Attachment #252758 - Flags: review?(pavlov) → review+
checked in.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Target Milestone: --- → mozilla1.9alpha
This cut about 40ms off Tp and Tp2 on bl-bldlnx01 ... most, but probably not quite all, of what we lost from the gfxPangoTextRun landing.
roc:

You forgot to remove the code for creating gfxXftTextRun.

> 174 gfxTextRun *
> 175 gfxPangoFontGroup::MakeTextRun(const PRUint8 *aString, PRUint32 aLength,
> 176                                Parameters *aParams)
> 177 {
> 178 #ifdef USE_XFT_FOR_ASCII
> 179     return new gfxXftTextRun(aString, aLength, aPersistentString);
> 180 #else
> 181     return new gfxPangoTextRun(this, aString, aLength, aParams);
> 182 #endif
> 183 }

Updated

11 years ago
Depends on: 370045

Updated

11 years ago
Blocks: 370045
No longer depends on: 370045
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.