Last Comment Bug 745780 - update harfbuzz to latest code from upstream
: update harfbuzz to latest code from upstream
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Jonathan Kew (:jfkthame)
:
:
Mentors:
Depends on: 695857 747834
Blocks: 676068 758236
  Show dependency treegraph
 
Reported: 2012-04-16 09:30 PDT by Jonathan Kew (:jfkthame)
Modified: 2016-04-07 18:19 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, harfbuzz code from upstream (166.77 KB, patch)
2012-04-16 11:45 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
mark.finkle: approval‑mozilla‑central+
Details | Diff | Splinter Review
reftests for arabic presentation-form shaping (86.17 KB, patch)
2012-04-16 11:48 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Splinter Review
patch from bug 727736 rebased on top of the harfbuzz update (4.54 KB, patch)
2012-04-20 00:31 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
mark.finkle: approval‑mozilla‑central+
Details | Diff | Splinter Review

Description Jonathan Kew (:jfkthame) 2012-04-16 09:30:36 PDT
It'd be good to take a harfbuzz update now that Arabic fallback (presentation-form) shaping is implemented; this will be particularly beneficial on Android devices that are shipping with OpenType-less versions of Droid Sans Arabic.
Comment 1 Jonathan Kew (:jfkthame) 2012-04-16 11:45:31 PDT
Created attachment 615402 [details] [diff] [review]
patch, harfbuzz code from upstream
Comment 2 Jonathan Kew (:jfkthame) 2012-04-16 11:48:41 PDT
Created attachment 615403 [details] [diff] [review]
reftests for arabic presentation-form shaping

Note that the testcase involving a diacritic within the lam-alef ligature will currently fail (marked accordingly in the manifest), as Behdad hasn't yet implemented support for that. I expect that'll come in a minor update before long, but would like to get the basic code landed ASAP so that we can get it into the Android product; the use of diacritics within ligatures is a rare enough edge case that it's not critical if we don't have that right away.
Comment 3 John Daggett (:jtd) 2012-04-17 22:22:09 PDT
Comment on attachment 615403 [details] [diff] [review]
reftests for arabic presentation-form shaping

> +fails HTTP(..) != arabic-fallback-4.html arabic-fallback-4-notref.html

Why the failure here?
Comment 4 John Daggett (:jtd) 2012-04-17 22:24:20 PDT
(In reply to John Daggett (:jtd) from comment #3)

> Why the failure here?

Ah that's the lam-alef ligature.  Let's comment that out for now with a comment noting what it's testing.
Comment 5 John Daggett (:jtd) 2012-04-19 23:56:48 PDT
I'm seeing a couple reftest failures on try under XP:
https://tbpl.mozilla.org/php/getParsedLog.php?id=11061026&tree=Try&full=1

bidi/bidi-006.html
bidi/bidi-006-j.html

The usual suspects...
Comment 6 Jonathan Kew (:jfkthame) 2012-04-20 00:30:06 PDT
Oh yes - that's bug 727736 again. As that isn't yet addressed in upstream harfbuzz, we need to re-apply our patch locally. I'll attach it here for reference.
Comment 7 Jonathan Kew (:jfkthame) 2012-04-20 00:31:00 PDT
Created attachment 616894 [details] [diff] [review]
patch from bug 727736 rebased on top of the harfbuzz update
Comment 8 John Daggett (:jtd) 2012-04-20 00:33:24 PDT
Comment on attachment 615403 [details] [diff] [review]
reftests for arabic presentation-form shaping

r+ with failing test commented out for now
Comment 9 John Daggett (:jtd) 2012-04-20 00:35:18 PDT
Comment on attachment 615402 [details] [diff] [review]
patch, harfbuzz code from upstream

We're going to need reftests for the Indic shaping but we don't need those now since Indic shaping isn't on by default.
Comment 10 Jonathan Kew (:jfkthame) 2012-04-20 00:53:16 PDT
(In reply to John Daggett (:jtd) from comment #9)
> Comment on attachment 615402 [details] [diff] [review]
> patch, harfbuzz code from upstream
> 
> We're going to need reftests for the Indic shaping but we don't need those
> now since Indic shaping isn't on by default.

Right - it's known to be incomplete as yet (but Behdad has a test suite and is working on the failing issues).
Comment 11 Jonathan Kew (:jfkthame) 2012-04-20 01:08:25 PDT
Comment on attachment 615402 [details] [diff] [review]
patch, harfbuzz code from upstream

Requesting approval to land for mozilla-14. This *does* affect fennec, as we use harfbuzz for text shaping; the motivation for landing it now is that this update will fix the currently-completely-broken rendering of Arabic script on certain devices (e.g. LG Optimus, see bug 676068). Without this, the fennec product is unusable for Arabic-script readers with such devices.
Comment 12 Jonathan Kew (:jfkthame) 2012-04-20 01:10:27 PDT
Comment on attachment 616894 [details] [diff] [review]
patch from bug 727736 rebased on top of the harfbuzz update

This is just re-applying the patch from bug 727736 - needed along with the harfbuzz update so that we don't regress rendering of old Hebrew/Arabic fonts that lack proper diacritic support.

Note You need to log in before you can comment on or make changes to this bug.