Last Comment Bug 755730 - don't mask out Indic codepoints in OpenType fonts supported by OS X 10.7
: don't mask out Indic codepoints in OpenType fonts supported by OS X 10.7
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Jonathan Kew (:jfkthame)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-16 07:01 PDT by Jonathan Kew (:jfkthame)
Modified: 2012-05-24 10:58 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, accept OpenType indic fonts for use with Core Text (11.77 KB, patch)
2012-05-17 09:15 PDT, Jonathan Kew (:jfkthame)
no flags Details | Diff | Splinter Review
patch v2, accept OpenType indic fonts for use with Core Text (12.27 KB, patch)
2012-05-23 02:38 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Splinter Review

Description Jonathan Kew (:jfkthame) 2012-05-16 07:01:20 PDT
Currently, unless AAT layout tables are present, we mask off all the Indic-script blocks from the cmap when we load a font on OS X because we know that AAT tables are required for Core Text to be able to shape the script properly, so we don't want to use fonts that were designed only for OpenType Layout engines.

However, in OS X 10.7, Core Text appears to have gained support for at least some of the Indic scripts. Therefore, when running on 10.7 we should not mask off those ranges, so that OpenType fonts (whether installed locally or provided via @font-face) will work as intended.

Additional testing needed to confirm exactly which scripts are supported by Core Text... I have the impression it may be limited to the northern Indic group at this point.

(With harfbuzz Indic support coming, we'll eventually be able to support all the Indic blocks via OpenType fonts, but that may be a few more cycles yet before we're ready to switch it on by default.)
Comment 1 Jonathan Kew (:jfkthame) 2012-05-17 09:15:48 PDT
Created attachment 624771 [details] [diff] [review]
patch, accept OpenType indic fonts for use with Core Text

AFAICT, Core Text on 10.7 seems to be working for OpenType Indic fonts with the exception of Sinhala, so we shouldn't mask those ranges out of the cmap if there's OpenType support in the font.

There are still a few other scripts where some kind of shaping support is needed for readable text - Khmer, Burmese, Balinese, New Tai Lue, etc. - but we've never attempted to be "smart" about cmap-masking those so I'm not touching them here.
Comment 2 John Daggett (:jtd) 2012-05-22 21:19:35 PDT
Comment on attachment 624771 [details] [diff] [review]
patch, accept OpenType indic fonts for use with Core Text

> +                // On OS X 10.7, or for Arabic on earlier OS, OpenType layout
> +                // can also provide the necessary shaping (except for Sinhala)
> +                if (sr.tags[0] == TRUETYPE_TAG('a','r','a','b') ||
> +                    (osxVersion >= MAC_OS_X_VERSION_10_7_HEX &&
> +                     sr.tags[0] != TRUETYPE_TAG('s','i','n','h'))) {
> +                    // We check for GSUB here, as GPOS alone would not be ok.
> +                    if (hasGSUB && SupportsScriptInGSUB(this, sr.tags)) {
> +                        continue;
>                      }
>                  }

So this is enabling OpenType shaping on 10.7 for *all* the scripts
other than Sinhala.  Did you actually test with *OpenType* fonts for
all these scripts?  Including Lao & Tibetan?  If so, I think the title
for this bug should reflect that you not just enabling Indic handling
but the handling of a wide range of scripts including Indic.

I think it would be better to add an additional 'minOSVersion' field
to the ScriptRange struct, rather than use the logic above.  Arabic
would be set to 0, Sinhala to MAC_OS_X_MAJOR_VERSION_MASK and the
others to MAC_OS_X_VERSION_10_7_HEX.  Then the logic above would
reduce to checking the runtime OS version to the min version level.

It would be ideal to have reftests for at some of these scripts but I
don't know what the availability of open-source fonts for these
scripts is like.
Comment 3 Jonathan Kew (:jfkthame) 2012-05-23 02:38:54 PDT
Created attachment 626376 [details] [diff] [review]
patch v2, accept OpenType indic fonts for use with Core Text

OK, here's a version that uses a minVersion field in the range.

I tested most of these with the Lohit OpenType fonts from RedHat; for Lao, I used Phetsarath OT, and for Tibetan, Tibetan Machine Uni. Obviously, I can't vouch for 100% correct shaping of every possible word, but it did appear that Core Text had the basic support needed (in particular, reordering of pre-base vowels, forming Reph where relevant, etc) in all cases except Sinhala.
Comment 4 Jonathan Kew (:jfkthame) 2012-05-23 05:38:30 PDT
(In reply to John Daggett (:jtd) from comment #2)
> It would be ideal to have reftests for at some of these scripts but I
> don't know what the availability of open-source fonts for these
> scripts is like.

There are open-source fonts (e.g. Lohit) available for many of them. Reftesting is tricky, though, as there's no way to encode a "presentation-form" representation of the expected output, in the way we can do for Arabic.

I suppose in principle we could create special PUA-encoded versions of the fonts so that the expected glyphs could be directly addressed without shaping, but it seems like a lot of effort, and wouldn't really give us in-depth testing of the shapers, only basic verification that there's at least some support present.

A possible follow-up, but not a high priority, IMO. I'd prefer to invest time in moving towards a harfbuzz-everywhere world. (And with harfbuzz, we'd be able to do some more interesting tests, I think, thanks to the ability to control individual features.)
Comment 6 Ed Morley [:emorley] 2012-05-24 10:58:14 PDT
https://hg.mozilla.org/mozilla-central/rev/f93cf0e9ea9a

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