Closed Bug 569531 Opened 15 years ago Closed 14 years ago

[harfbuzz] enable harfbuzz by default

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b1

People

(Reporter: jtd, Assigned: jfkthame)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
Depends on: 569535
Depends on: 569770
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.
Depends on: 570915
Depends on: 570968
No longer depends on: 570968
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.)
Attachment #449017 - Attachment is obsolete: true
Attachment #453544 - Flags: review?(roc)
Comment on attachment 453544 [details] [diff] [review] patch to enable harfbuzz by default on OS X only Let's land this ASAP!
Attachment #453544 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a6
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.
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.
Depends on: 580863
Depends on: 588731
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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?
Oops, s/artistically/artificially/
(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.
Depends on: 590101
(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.
Attached patch part 2 - enable harfbuzz by default on windows (obsolete) (deleted) — Splinter Review
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.)
Attachment #471085 - Flags: review?(roc)
Fixed typo (from manual merging of the reftest manifest - oops) in the previous version of the patch.
Attachment #471085 - Attachment is obsolete: true
Attachment #471092 - Flags: review?(roc)
Attachment #471085 - Flags: review?(roc)
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.
Attachment #471092 - Flags: approval2.0?
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.)
Status: REOPENED → RESOLVED
Closed: 15 years ago14 years ago
Resolution: --- → FIXED
Depends on: 593296
Blocks: 573407
Depends on: 604321
Depends on: 608876
No longer depends on: 608876
Depends on: 605043
Depends on: 609604
Blocks: 463413
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: