Last Comment Bug 458160 - [PATCH] Can't use .otf fonts via @font-face on Windows
: [PATCH] Can't use .otf fonts via @font-face on Windows
Status: RESOLVED FIXED
: fixed1.9.1
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Windows XP
: P2 major with 3 votes (vote)
: mozilla1.9.1b3
Assigned To: John Daggett (:jtd)
:
: Milan Sreckovic [:milan]
Mentors:
http://opentype.info/demo/webfontdemo...
Depends on:
Blocks: 70132
  Show dependency treegraph
 
Reported: 2008-10-01 20:30 PDT by John Daggett (:jtd)
Modified: 2009-05-29 07:02 PDT (History)
25 users (show)
vladimir: blocking1.9.1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, v.0.2c, load .otf fonts using AddFontMemResourceEx on Windows (20.90 KB, patch)
2008-11-20 01:16 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
screenshot, shifted glyph rendering (6.33 KB, image/png)
2008-11-20 01:27 PST, John Daggett (:jtd)
no flags Details
test program, use GDI and Uniscribe to draw text (12.21 KB, text/plain)
2008-11-30 23:49 PST, John Daggett (:jtd)
no flags Details
screenshot, test program output (60.13 KB, image/png)
2008-11-30 23:53 PST, John Daggett (:jtd)
no flags Details
patch, v.0.2d, force rendering via GDI for now (22.26 KB, patch)
2008-12-02 00:37 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, v.0.3, rework underline offset handling (24.93 KB, patch)
2008-12-04 03:07 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, simple otf reftest (2.38 KB, patch)
2008-12-09 00:30 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, v.0.4, add code to catch fNoGlyphIndex being set (28.07 KB, patch)
2008-12-14 23:33 PST, John Daggett (:jtd)
roc: review+
vladimir: superreview+
Details | Diff | Splinter Review
reftest patch, v.0.2, use .otf versions of the mark fonts instead (3.26 KB, patch)
2008-12-15 22:11 PST, John Daggett (:jtd)
dbaron: review+
dbaron: superreview+
Details | Diff | Splinter Review
reftest patch, v.0.3a, use larger font size (4.93 KB, patch)
2008-12-16 00:20 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
ifdefs around offending code (2.96 KB, patch)
2008-12-22 10:25 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
texttest.cpp (25.13 KB, text/plain)
2009-05-29 07:00 PDT, Adrian Johnson
no flags Details
screen shot (56.13 KB, image/png)
2009-05-29 07:02 PDT, Adrian Johnson
no flags Details

Description John Daggett (:jtd) 2008-10-01 20:30:51 PDT
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.
Comment 1 John Daggett (:jtd) 2008-10-01 20:35:16 PDT
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.
Comment 2 John Daggett (:jtd) 2008-11-20 01:16:32 PST
Created attachment 349150 [details] [diff] [review]
patch, v.0.2c, load .otf fonts using AddFontMemResourceEx on Windows

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.
Comment 3 John Daggett (:jtd) 2008-11-20 01:27:54 PST
Created attachment 349151 [details]
screenshot, shifted glyph rendering

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.
Comment 4 John Daggett (:jtd) 2008-11-25 01:15:20 PST
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.
Comment 5 John Daggett (:jtd) 2008-11-30 23:49:13 PST
Created attachment 350727 [details]
test program, use GDI and Uniscribe to draw text

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.
Comment 6 John Daggett (:jtd) 2008-11-30 23:53:14 PST
Created attachment 350728 [details]
screenshot, test program output

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).
Comment 7 John Daggett (:jtd) 2008-12-02 00:37:59 PST
Created attachment 350939 [details] [diff] [review]
patch, v.0.2d, force rendering via GDI for now

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.
Comment 8 John Daggett (:jtd) 2008-12-02 00:39:46 PST
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*).
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-12-02 01:12:42 PST
The Harfbuzz route is looking better and better!
Comment 10 John Daggett (:jtd) 2008-12-04 03:07:38 PST
Created attachment 351355 [details] [diff] [review]
patch, v.0.3, rework underline offset handling

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.
Comment 11 John Daggett (:jtd) 2008-12-04 18:08:04 PST
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
Comment 12 Bill Gianopoulos [:WG9s] 2008-12-05 11:51:28 PST
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.
Comment 13 John Daggett (:jtd) 2008-12-07 18:24:03 PST
(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.
Comment 14 John Daggett (:jtd) 2008-12-09 00:30:29 PST
Created attachment 352071 [details] [diff] [review]
patch, simple otf reftest

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
Comment 15 John Daggett (:jtd) 2008-12-14 23:33:02 PST
Created attachment 352992 [details] [diff] [review]
patch, v.0.4, add code to catch fNoGlyphIndex being set

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 16 John Daggett (:jtd) 2008-12-14 23:33:54 PST
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?
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-12-15 00:58:17 PST
Can we get, or make, a free OTF font so we can check in a reftest?
Comment 18 John Daggett (:jtd) 2008-12-15 01:06:08 PST
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 19 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-12-15 01:25:43 PST
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 :-(.
Comment 20 Jonathan Kew (:jfkthame) 2008-12-15 02:40:45 PST
(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.
Comment 21 John Daggett (:jtd) 2008-12-15 22:11:42 PST
Created attachment 353163 [details] [diff] [review]
reftest patch, v.0.2, use .otf versions of the mark fonts instead

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 22 David Baron :dbaron: ⌚️UTC-8 2008-12-15 22:17:28 PST
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.
Comment 23 John Daggett (:jtd) 2008-12-15 23:09:54 PST
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 24 David Baron :dbaron: ⌚️UTC-8 2008-12-15 23:19:34 PST
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?)
Comment 25 John Daggett (:jtd) 2008-12-15 23:23:17 PST
I'm running into rendering differences on Linux at the default font size, but everything is fine at a larger font size.  Patch shortly...
Comment 26 Karl Tomlinson (:karlt) 2008-12-16 00:03:35 PST
(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...
Comment 27 John Daggett (:jtd) 2008-12-16 00:20:21 PST
Created attachment 353177 [details] [diff] [review]
reftest patch, v.0.3a, use larger font size

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 28 John Daggett (:jtd) 2008-12-18 17:21:42 PST
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.
Comment 29 Jonathan Kew (:jfkthame) 2008-12-19 15:29:33 PST
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.
Comment 30 John Daggett (:jtd) 2008-12-22 00:48:27 PST
(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.
Comment 31 John Daggett (:jtd) 2008-12-22 00:49:43 PST
checked in to trunk
http://hg.mozilla.org/mozilla-central/rev/28eea1633fe1
Comment 32 Benjamin Smedberg [:bsmedberg] 2008-12-22 06:52:32 PST
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
Comment 33 Benjamin Smedberg [:bsmedberg] 2008-12-22 06:54:10 PST
also line 901 and 942
Comment 34 Brad Lassey [:blassey] (use needinfo?) 2008-12-22 10:14:42 PST
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'
Comment 35 Brad Lassey [:blassey] (use needinfo?) 2008-12-22 10:25:15 PST
Created attachment 354190 [details] [diff] [review]
ifdefs around offending code

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.
Comment 36 John Daggett (:jtd) 2008-12-23 17:08:31 PST
(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?
Comment 37 John Daggett (:jtd) 2008-12-23 17:11:39 PST
(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.
Comment 38 Brad Lassey [:blassey] (use needinfo?) 2008-12-23 17:29:05 PST
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.
Comment 39 John Daggett (:jtd) 2008-12-23 18:10:50 PST
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.
Comment 40 Brad Lassey [:blassey] (use needinfo?) 2008-12-24 07:10:50 PST
It returns a FontEntry, as declared in gfxFT2Fonts.h.  Admittedly, that's very confusing because FontEntry doesn't get subclassed.
Comment 41 John Daggett (:jtd) 2009-01-04 18:44:43 PST
(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.
Comment 42 Brad Lassey [:blassey] (use needinfo?) 2009-01-05 06:46:57 PST
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.
Comment 43 Vladimir Vukicevic [:vlad] [:vladv] 2009-01-05 23:14:40 PST
(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.
Comment 44 Gabriele Best [:Gabri] 2009-01-06 04:22:12 PST
I think it should be also included in 1.9.1 branch...
Comment 45 Brad Lassey [:blassey] (use needinfo?) 2009-01-06 05:55:12 PST
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.
Comment 46 Jonathan Kew (:jfkthame) 2009-01-06 06:11:31 PST
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.
Comment 47 John Daggett (:jtd) 2009-01-06 19:10:12 PST
Checked into 1.9.1 branch
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/16af31d9b105
Comment 48 John Daggett (:jtd) 2009-01-28 22:57:07 PST
Comment on attachment 354190 [details] [diff] [review]
ifdefs around offending code

no review needed, changes done as part of bug 462908
Comment 49 John Daggett (:jtd) 2009-02-01 18:31:07 PST
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."
Comment 50 Adrian Johnson 2009-05-29 07:00:07 PDT
Created attachment 380418 [details]
texttest.cpp

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.
Comment 51 Adrian Johnson 2009-05-29 07:02:29 PDT
Created attachment 380419 [details]
screen shot

Screenshot of output from test program in comment 50.

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