569531 - [harfbuzz] enable harfbuzz by default

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[John Daggett (:jtd)]

The current plan is to land harfbuzz shaping for simple scripts as part of bug 449292 but disabled by default.  This is a meta-bug for other bugs that block enabling harfbuzz by default.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1 - enable harfbuzz by default for simple scripts

This patch flips the preference so that harfbuzz will be used by default. Depends on the patches from bug 449292. Not ready to land, as this would cause a few test failures on Windows; currently under investigation.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: patch to enable harfbuzz by default on OS X only

This turns on harfbuzz by default on OS X, so that we can begin to get some wider real-world testing and check for any regressions.

(On Windows, a couple of reftests and a handful of mochitest failures are still pending, so we're not ready to flip the pref there yet.)

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 453544 [details] [diff] [review]
patch to enable harfbuzz by default on OS X only

Let's land this ASAP!

Comment 4

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/e7d06d7d284b

Comment 5

[(no longer active)]

The OS X 64 Md1 run with this patch leaked one nsStringBuffer:

<http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1277340705.1277342332.5544.gz>

It's intermittent (the next run didn't leak) but I thought I'd mention it here anyway.

Comment 6

[Daniel C]

I think this should either not be RESOLVED FIXED, or have the platform set to OS X and new bugs filed for the other platforms.

Comment 7

[Jonathan Kew [:jfkthame]]

Re-opening this; as per comment 6, it should not have been closed yet as we only switched harfbuzz on for a single platform so far.

Comment 8

[Gábor Stefanik]

On Windows, artistically enabling harfbuzz results in the following testcase failing:
data:text/html,ffffffffffffffffffff
The f's are not spaced out/kerned evenly (rounding error suspected). Is there already a bug about this, or should I file one?

Comment 9

[Gábor Stefanik]

Oops, s/artistically/artificially/

Comment 10

[Behdad Esfahbod]

(In reply to comment #8)
> On Windows, artistically enabling harfbuzz results in the following testcase
> failing:
> data:text/html,ffffffffffffffffffff
> The f's are not spaced out/kerned evenly (rounding error suspected). Is there
> already a bug about this, or should I file one?

The harfbuzz clients are responsible for doing any rounding if necessary.

Comment 11

[Jonathan Kew [:jfkthame]]

(In reply to comment #8)
> On Windows, artistically enabling harfbuzz results in the following testcase
> failing:
> data:text/html,ffffffffffffffffffff
> The f's are not spaced out/kerned evenly (rounding error suspected). Is there
> already a bug about this, or should I file one?

There are a number of questions that could be relevant to this. What version of Windows, and what font are you using? Is DirectWrite/Direct2D rendering enabled or are you using GDI only?

It's possible that you are actually seeing pairs of "f" glyphs forming "ff" ligatures, which would give the impression of uneven spacing.

If that's not the explanation, please file a separate bug with full details of the environment, and a screenshot showing the results you're getting, so that we can investigate more thoroughly.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: part 2 - enable harfbuzz by default on windows

Now that the 4.0b5 freeze is over, I'd like us to try switching to harfbuzz as the default on Windows, so that we can start getting wider testing, and (provided nothing goes wrong!) ship it enabled in the next beta.

This patch flips the preference so that Windows will use HB instead of platform text shaping for "simple" scripts (Latin, Cyrillic, CJK, etc), just as we're already doing on OS X.

There are three reftests that are still giving trouble here, depending on the Windows version, as noted in bug 590101 comment 4; this patch marks them as random on Windows for the time being. (OTOH, it also enables four other reftests that are currently random or known-fail, but pass with the harfbuzz backend.)

Comment 13

[Jonathan Kew [:jfkthame]]

Attachment: part 2 v2 - enable harfbuzz by default on windows

Fixed typo (from manual merging of the reftest manifest - oops) in the previous version of the patch.

Comment 14

[Jonathan Kew [:jfkthame]]

Comment on attachment 471092 [details] [diff] [review]
part 2 v2 - enable harfbuzz by default on windows

Requesting approval to land on trunk, in readiness for the next FF4 beta; we need to get wider testing exposure for this, as well as check Talos numbers. (Tryserver results are inconclusive -- sometimes it seems to show a significant Tp4 win, sometimes a slight regression, but the noise is such that it's hard to be sure what the overall impact will be.)

Switching on the harfbuzz backend is a step in reducing our exposure to platform font-shaping bugs; it will also enable the new CSS font-features stuff, and allow us to resolve issues such as bug 465395, bug 573407, and the longstanding bug 24139.

Comment 15

[Jonathan Kew [:jfkthame]]

Pushed http://hg.mozilla.org/mozilla-central/rev/dc2939f2640d

Comment 16

[Jonathan Kew [:jfkthame]]

Resolving this as FIXED - we now have the harfbuzz backend enabled for simple scripts on the platforms where it's implemented at all (Mac & Win).

We still need an implementation for Linux; that is bug 569770. (We also need additional script support in harfbuzz, but that is an upstream issue.)