The Windows API we're using to load downloaded fonts does not appear to support Postscript CFF OpenType (.otf) fonts. Need to figure out a work around for this. Steps: 1. Open the URL with the latest nightly on Windows Result: only the font for the last example loads (a .ttf font). All the other examples use .otf and none of them load. From bug 441473 comment 130: Did some fiddling around with .otf font support on Windows today. Poking around on the Microsoft support site, I noticed the KB below stating that .otf fonts are not supported, even in the latest version of Microsoft Office: KB 908475 You cannot embed an Adobe OpenType font in a document in an Office program http://support.microsoft.com/kb/908475 The curious thing is that you can save documents in Word that reference .otf fonts, it's obviously embedding the font (the size changes dramatically) but the fonts fail to display when opened on a machine without the underlying font. For .ttf fonts, the embedded fonts render fine. Webkit also recently switched to supporting GDI font handling and in their latest Windows builds, they also don't appear to support .otf fonts: https://bugs.webkit.org/show_bug.cgi?id=21127 Note: with Safari 3.1.2, downloaded .otf fonts render just fine. Same problem for Chrome. I suspect this may be a limitation of the TTLoadEmbeddedFont API, I'm going to fiddle around a bit more with that.

Recent Webkit builds using GDI on Windows had the same problem: https://bugs.webkit.org/show_bug.cgi?id=21127 Looks like they worked around it by appending a swizzled name table to the font data and using AddFontMemResourceEx instead of TTLoadEmbeddedFont. The name swizzling is needed to avoid name collisions between documents.

Load .otf fonts on Windows using AddFontMemResourceEx. To keep the font name from colliding with other existing fonts, rename font by appending a new name table to the font data before loading. Code is functional but it looks like there's a bug in our glyph handling code, in a very simple example all the rendered glyphs, have shifted glyph indices so text appears as jibberish.

This is how the word "Cheltenham" renders with ITC Cheltenham Std Book, an .otf font. Each of the glyph indices is off by 31, the first character is glyph 67 (b), it should be glyph 36 (C), the second glyph is 104 ('), it should be 73 (h), etc. I hacked around with this a bit, our code renders like this even with the unmodified font data, so this rendering behavior is not an artifact of name table changes.

I debugged this some more today, Uniscribe appears to be squawking at the use of fonts activated via AddFontMemResourceEx. Using an Adobe .otf font from Font Folio 11 for testing, I installed the font on my system, then compared the results using the name of the installed font and a separate copy activated via an @font-face rule. The trouble appears to be within UniscribeItem::ShapeUniscribe - http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxWindowsFonts.cpp#1386 All the flags in the SCRIPT_ANALYSIS struct are set to false except for the logical order flag. When ScriptShape is called with a DC containing the downloaded font, the fNoGlyphIndex flag is set upon return. Instead of glyph indices, mGlyphs contains the original code values instead. From the Uniscribe docs: http://www.microsoft.com/typography/developers/uniscribe/uniscribe.htm "fNoGlyphIndex May be set TRUE on input to ScriptShape to disable use of glyphs for this item. Additionally, ScriptShape will set it TRUE for hdcs containing symbolic, unrecognised and device fonts. Disabling glyphing disables complex script shaping. When set, shaping and placing for this item is implemented directly by calls to GetTextExtentExPoint and ExtTextOut." When the *same* font is used as an installed platform font, this flag never gets set and text renders as expected. If I set the new FontEntry's mForceGDI to true for downloaded CFF fonts in MakePlatformFont, text renders correctly. But this effectively disables shaping for .otf fonts, which sucks.

Draws text with GDI (DoDrawGDI) and Uniscribe (DoDrawUniscribe) for comparison. The Uniscribe drawing code is essentially similar to what is used internally witin FF. The code draws the same two strings with three different fonts: a dynamically loaded TrueType font that contains kerning information, an installed .otf font and an .otf font dynamically loaded using AddFontMemResourceEx. For some reason, Uniscribe is fine with the installed .otf font but not happy with the dynamically loaded one (switching the two fonts results in the same behavior, the dynamically loaded one is not shaped by Uniscribe). The only indication that something is amiss is the fNoGlyphIndex flag that sets in the analysis structure.

Red text is drawn with GDI, green text with Uniscribe. The first four lines use a dynamically loaded TrueType font, the second four use an installed .otf font and the last four use a dynamically loaded .otf font. Note the differences in the kerning of "To" and the fi ligatures, kerning and ligatures only fails in the dynamically loaded .otf case (all three fonts have kerning and ligature information).

Same as the last patch but for CFF fonts mForceGDI is set to force GDI rendering. This works around the Uniscribe problem until we can figure out if there's a workaround for that problem. Also, commented out the UpdateFontList assertion in GetFontAt, it's firing within UpdateFontList itself. Need to rework the logic for the bad underline setting in UpdateFontList.

With the latest patch, all the fonts in the test page will load except for Vollkorn, for some reason AddFontResourceMemEx refuses to load that font, not exactly sure why. Since shaping is disabled, all kerning/ligatures are disabled (*sigh*).

The Harfbuzz route is looking better and better!

Calculate the underline offset lazily, this eliminates the need to call GetFontAt from within InitFontList, these were causing assertions to fire within GetFontAt. Still needs more testing.

Mail below was sent to Microsoft folks regarding this problem. They didn't seem to be aware of the problem, said they would investigate when they had a chance. Mail sent to MS: -----Original Message----- From: John Daggett Sent: Monday, December 01, 2008 1:23 AM Subject: using Uniscribe with .otf fonts I'm wondering if you know of some internal limitation within Uniscribe that restricts the use of dynamically loaded .otf fonts? The situation I'm running into is that ScriptShape refuses to shape .otf fonts loaded via AddFontResourceMemEx, the fNoGlyphIndex flag is set on return to indicate that the font "cannot support glyph indexes" even though the same font does fine when installed in the fonts folder. Is this a known Uniscribe limitation/bug? Something that's already fixed in more recent versions of Uniscribe? Regards, John Daggett Mozilla Japan Test details: Windows XP SP2, Uniscribe 1.420.2600.2180 Windows Vista, Uniscribe 1.626.6001.18000 Text: "The quick brown fox jumps over the lazy dog" / "Total Technology finally" Rendered via Uniscribe with: * TTF loaded via AddFontResourceMemEx --> ScriptShape succeeds * system-installed OTF --> ScriptShape succeeds * OTF loaded via AddFontResourceMemEx --> ScriptShape fails, fNoGlyphIndex set on return Screenshot (red text via GDI, green text via Uniscribe, 3 fonts described above): https://bugzilla.mozilla.org/attachment.cgi?id=350728 Sample test code: https://bugzilla.mozilla.org/attachment.cgi?id=350727 Fonts: both OTF fonts are from Adobe's Font Folio 11

With this patch applied, some of the examples form the URL specified in the URL file of this bug (http://opentype.info/demo/webfontdemo.html) display italicized text when they should not. This occurs in the Fontin, Fontin Sans and Kaffeesatz examples.

(In reply to comment #12) > With this patch applied, some of the examples form the URL specified in the URL > file of this bug (http://opentype.info/demo/webfontdemo.html) display > italicized text when they should not. This occurs in the Fontin, Fontin Sans > and Kaffeesatz examples. I've logged this as a separate bug, bug 468387. The way we use GDI means that synthetic bolding/italics will kick in when it shouldn't.

Simple reftest for .otf fonts. Only tests whether the font is picked up or not, doesn't test rendering correctness. Requires Goudy_Bookletter_1911_by_chemoelectric.otf in layout/reftests/fonts/. http://chemoelectric.deviantart.com/art/Goudy-Bookletter-1911-74918469

Within UniscribeItem, force GDI shaping/placement when fNoGlyphIndex is set by ScriptShape. For downloaded CFF fonts on Windows, the mForceGDI flag is set on the font entry so these will automatically use GDI so this code is for proper error handling in other situations. At this point I'm not sure what those are so I just added a warning rather than an assertion.

Comment on attachment 352992 [details] [diff] [review] patch, v.0.4, add code to catch fNoGlyphIndex being set Stuart, could I ask you to review just the changes to UniscribeItem?

Can we get, or make, a free OTF font so we can check in a reftest?

There's a reftest attached. In addition, it looks like it would be trivial to generate the markA, markB, etc. test fonts with David's script, the only thing needed is to generate a ".otf" version in addition to a .ttf version. That might be simpler than the reftest attached already, we could even do comparisons of .ttf to .otf as a way of assuring that both .otf and .ttf fonts are supported.

Comment on attachment 352992 [details] [diff] [review] patch, v.0.4, add code to catch fNoGlyphIndex being set + mUnderlineOffset = UNDERLINE_OFFSET_NOT_SET; + +} Unnecessary blank line + const PRUint32 neededNameIDs[] = {NameRecord::NAME_ID_FAMILY, static const + if (!aNewFont->AppendElements(adjFontDataSize)) + return NS_ERROR_OUT_OF_MEMORY; + + // copy the old font data + PRUint8 *newFontData = reinterpret_cast<PRUint8*> (aNewFont->Elements()); + + memcpy(newFontData, aFontData, aFontDataLength); Just use AppendElements(aFontData, aFontDataLength). + PRUint8 *newFontData = reinterpret_cast<PRUint8*> (aNewFont->Elements()); + NameRecord *nameRecord = reinterpret_cast<NameRecord*> (nameHeader + 1); + PRUnichar *strData = reinterpret_cast<PRUnichar*> (nameRecord); + AutoSwap_PRUint32 *nameData = reinterpret_cast<AutoSwap_PRUint32*> (nameHeader); + PRUint8 *fontData = reinterpret_cast<PRUint8*> (newFontData.Elements()); Unnecessary space in > ( + } + + } else { Unnecessary blank line. + PRUnichar fontName[LF_FACESIZE]; + + PRUint32 nameLen = PR_MIN(uniqueName.Length(), LF_FACESIZE - 1); + memcpy(fontName, nsPromiseFlatString(uniqueName).get(), nameLen * 2); + fontName[nameLen] = 0; Instead of doing this, better to use our string classes and do nsPromiseFlatString fontName(Substring(uniqueName, 0, nameLen)); and use fontName.get() below. Basically looks good other than those nits. Sad though :-(.

(In reply to comment #18) > we could even do > comparisons of .ttf to .otf as a way of assuring that both .otf and .ttf fonts > are supported. We could try this, though we might run into "random" pixel differences between the rasterization and antialiasing of the different font formats, even if the basic glyph outlines match.

Add some tests that use .otf versions of the mark fonts, including two that test .ttf vs. .otf results. (In reply to comment #20) > We could try this, though we might run into "random" pixel differences between > the rasterization and antialiasing of the different font formats, even if the > basic glyph outlines match. In general I think you're right but in this special case the test fonts are essentially boxes without curved edges so that should reduce the chance of running into rendering artifacts due to format differences.

Comment on attachment 353163 [details] [diff] [review] reftest patch, v.0.2, use .otf versions of the mark fonts instead The part that's here looks fine. However, half the patch is missing. I'd sort of like to see that half -- I'm interested in whether you were able to generate the .otf fonts with the same script... Also, please actually test that the test passes on Mac, Linux, and Windows.

I've already pushed in a set of .otf fonts, I just added a line to the script to generate files with .otf extensions. http://hg.mozilla.org/mozilla-central/rev/37c2721a512d Will post a new reftest patch shortly.

Comment on attachment 353163 [details] [diff] [review] reftest patch, v.0.2, use .otf versions of the mark fonts instead ok, looks good then, assuming it's tested on Mac, Linux, and Windows. (Or was there some other reason you were posting a new patch?)

I'm running into rendering differences on Linux at the default font size, but everything is fine at a larger font size. Patch shortly...

(In reply to comment #25) > I'm running into rendering differences on Linux at the default font size, but > everything is fine at a larger font size. Just a wild stab in the dark, but it looks like FreeType considers a bit in TrueType fonts when deciding whether to round em sizes to integer numbers of pixels. I haven't found similar code in the cff backend. http://cvs.savannah.gnu.org/viewvc/freetype/freetype2/src/truetype/ttobjs.c?revision=1.122&view=markup /* This bit flag, if set, indicates that the ppems must be */ /* rounded to integers. Nearly all TrueType fonts have this bit */ /* set, as hinting won't work really well otherwise. */ /* */ if ( face->header.Flags & 8 ) { metrics->x_scale = FT_DivFix( metrics->x_ppem << 6, face->root.units_per_EM ); metrics->y_scale = FT_DivFix( metrics->y_ppem << 6, face->root.units_per_EM ); But then, I thought our default font sizes were integer pixels so I don't know why this would make a difference...

For some reason, when comparing .ttf vs. .otf the .otf versions display shifted down under Windows. Need to do some more testing to figure out why this is happening. Linux, OS X 10.4, 10.5 all pass using font-size: 50px.

Comment on attachment 352992 [details] [diff] [review] patch, v.0.4, add code to catch fNoGlyphIndex being set Jonathan, could you look over the changes within UniscribeItem related to the fNoGlyphIndex flag? Thanks.

Yes, the fNoGlyphIndex stuff looks reasonable (although regrettable!). However, while I was there I noticed this in FontFamily::FamilyAddStylesProc... if (nmetrics->ntmFontSig.fsUsb[0] != 0x00000000 && nmetrics->ntmFontSig.fsUsb[1] != 0x00000000 && nmetrics->ntmFontSig.fsUsb[2] != 0x00000000 && nmetrics->ntmFontSig.fsUsb[3] != 0x00000000) { (see http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxWindowsFonts.cpp#237). Shouldn't this have ||, not &&, joining the tests? Not sure what the ultimate impact of this would be, but it looks wrong to me.

(In reply to comment #29) > Yes, the fNoGlyphIndex stuff looks reasonable (although regrettable!). > > However, while I was there I noticed this in FontFamily::FamilyAddStylesProc... > > if (nmetrics->ntmFontSig.fsUsb[0] != 0x00000000 && > nmetrics->ntmFontSig.fsUsb[1] != 0x00000000 && > nmetrics->ntmFontSig.fsUsb[2] != 0x00000000 && > nmetrics->ntmFontSig.fsUsb[3] != 0x00000000) { > > (see > http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxWindowsFonts.cpp#237). > > Shouldn't this have ||, not &&, joining the tests? Not sure what the ultimate > impact of this would be, but it looks wrong to me. Yeah, I think you may be right. I'll log a separate bug tomorrow.

checked in to trunk http://hg.mozilla.org/mozilla-central/rev/28eea1633fe1

New Warning: gfx/thebes/src/gfxFontUtils.cpp:668: multi-character character constant http://hg.mozilla.org/mozilla-central/annotate/28eea1633fe1e890760bb94c6bf72f279b3bd464/gfx/thebes/src/gfxFontUtils.cpp/#l668

also line 901 and 942

I'm reopening because this broke the windows mobile build. c:/hg/mozilla-central/gfx/thebes/src/gfxWindowsPlatform.cpp(759) : error C3861: 'RemoveFontMemResourceEx': identifier not found c:/hg/mozilla-central/gfx/thebes/src/gfxWindowsPlatform.cpp(871) : error C3861: 'AddFontMemResourceEx': identifier not found c:/hg/mozilla-central/gfx/thebes/src/gfxWindowsPlatform.cpp(879) : error C3861: 'RemoveFontMemResourceEx': identifier not found c:/hg/mozilla-central/gfx/thebes/src/gfxWindowsPlatform.cpp(924) : error C2039: 'mForceGDI' : is not a member of 'FontEntry'

this just ifdefs around the offending code to get us building again. I have no idea if this is correct though, please have a look.

(In reply to comment #34) > c:/hg/mozilla-central/gfx/thebes/src/gfxWindowsPlatform.cpp(924) : error C2039: > 'mForceGDI' : is not a member of 'FontEntry' +#ifndef WINCE if (fe && isCFF) fe->mForceGDI = PR_TRUE; - +#endif #ifdef'ing calls to AddFontxxx and RemoveFontxxx looks fine, but the error related to mForceGDI is not. This should not be happening if your sources are in sync. Bad merge maybe?

(In reply to comment #32) > New Warning: gfx/thebes/src/gfxFontUtils.cpp:668: multi-character character > constant > http://hg.mozilla.org/mozilla-central/annotate/28eea1633fe1e890760bb94c6bf72f279b3bd464/gfx/thebes/src/gfxFontUtils.cpp/#l668 I noticed these when looking over the static analysis logs last week. So the concern is that multi-character constants are defined by the standard to be "implementation dependent" and thus not safe to use? I'm fully aware of the endian issues involved, I'm wondering if there are issues with specific compilers or concerns about this in a 64-bit environment. Setting up macros to work around this isn't hard but I'd like to understand what the concern is.

Windows ce is using the freetype font backend and not the uniscribe backend that windows desktop uses. Therefore these are instances of gfxFT2Font and not gfxWindowsFont. In general it's better to use gfxFont in the platform code and not assume any particular subclass. Code specific to gfxWindowsFonts should be in the coresponding source file.

Um, so wait, what exactly is FontEntry *fe = FontEntry::CreateFontEntry() going to produce under Windows CE? Seems like MakePlatformFont needs to be completely different if you're using the FT backend instead of GDI.

It returns a FontEntry, as declared in gfxFT2Fonts.h. Admittedly, that's very confusing because FontEntry doesn't get subclassed.

(In reply to comment #40) > It returns a FontEntry, as declared in gfxFT2Fonts.h. Admittedly, that's very > confusing because FontEntry doesn't get subclassed. So with the changes in your patch, downloadable fonts work on Windows CE? If not, I think these changes should be moved to a separate bug since the issues are different.

No, I don't know that it works. This simply fixes the build bustage caused by the patch you checked in. You either need to fix the bustage or back out what you committed.

(In reply to comment #42) > No, I don't know that it works. This simply fixes the build bustage caused by > the patch you checked in. You either need to fix the bustage or back out what > you committed. No, this should not be backed out; we need a new bug to deal with this -- gfxWindowsPlatform was never intended to be used with freetype fonts, and the platform has a lot of fonts-specific code in it that's just not relevant for windows mobile. Adding #ifdefs around every usage of a field in a windows font entry that's not present in ft2fonts is not a good solution to that. We need a gfxPlatformWindowsMobile that's focused on windows mobile's needs, and bug 462908 is going to need to do that.

I think it should be also included in 1.9.1 branch...

Wouldn't it be cleaner to have the font code in gfxWindowsFonts? gtk/pango put the majority of the implementation of MakePlatformFont in gfxPangoFonts. http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxPlatformGtk.cpp#295 and http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxPangoFonts.cpp#2178 Creating a gfxPlatformWindowsMobile would seems like it would result in a lot of unnecessary code duplication.

We may end up reworking gfxWindowFonts quite a bit, too, as part of implementing HarfBuzz as an alternative shaper; there are also issues with font family/style management. I'm looking at some of this at the moment, intending to provide a better foundation for fixing bugs 454514 and 469656, and at the same time sharing much more of the font management code across all the platforms.

Checked into 1.9.1 branch http://hg.mozilla.org/releases/mozilla-1.9.1/rev/16af31d9b105

Comment on attachment 354190 [details] [diff] [review] ifdefs around offending code no review needed, changes done as part of bug 462908

Follow up from Microsoft engineer: "Well, I can confirm that you report is true. Privately loaded CFF fonts would be rejected by Uniscribe (i.e. no shaping will be performed by Uniscribe). This is related to some checks we are doing, which are done through EnumFontFamilies. Obviously, privately installed fonts are not being enumerated, so we are rejecting the font as not-OpenType. We are not doing this check for TrueType font, so they are working fine. This is Uniscribe problem, which has nothing to do with GDI itself, so fix in Uniscribe should be sufficient. I think we will be able to fix it in Windows7, but it is unlikely we can fix it on existing versions. I'll check it out, though. Thanks, for reporting this. Of course, this is also important for our products too."

A possible work around for issue with cff fonts and uniscribe is to convert the cff to ttf for use with uniscribe to get the glyphs and positions then display with the cff font. I've attached an updated version of the texttest.cpp from comment 5 to demonstrate this. It strips the cff specific tables out of the font and adds a truetype glyf table with empty glyphs. The metrics and opentype tables remain identical to the cff font. This ttf font can then be loaded with AddFontMemResource and used with uniscribe. After calling ScriptShape and ScriptPlace the current font is switched to the cff font before calling ScriptTextOut.

Screenshot of output from test program in comment 50.