468387 - [@font-face] need to disable synthetic bold/italic for downloadable fonts specified as bold/italic

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[John Daggett (:jtd)]

For a given @font-face rule, an author can specify bold/italic for a given face even though the underlying font data is not a bold/italic face.  In the testpage, the section for Kaffeesatz uses this rule:

@font-face {
  font-family: Kaffeesatz;
  src: url(fonts/YanoneKaffeesatz-Regular.otf) format("opentype");
  font-weight:bold;	
}

This should render with the regular face with no synthetic bolding, since the author has declared this to be a "bold" face.  Same thing for italics, if the author declares a specific face italic, no synthetic slanting should occur.

Currently, this problem exists on Windows and probably on Linux (need to confirm that).  Basically, where we rely on the OS to do synthetic bolding/italics, we need to separate the style characteristics used to match from the style characteristics used to render.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think Karl's patch handled that.

Comment 2

[Bill Gianopoulos [:WG9s]]

This occurs on Windows only.  I first noticed this when comparing the Linux rendering of the test page with the Windows rendering.  Then I compared both to the reference image to see which was correct.

Linux matches the refernece image.

Comment 3

[John Daggett (:jtd)]

Attachment: testcase, check for unwanted synthetics across weight/style combinations

Comment 4

[John Daggett (:jtd)]

Attachment: screenshot, testcase under Ubuntu with latest trunk

Comment 5

[John Daggett (:jtd)]

Attachment: screenshot, testcase under Windows with latest trunk

Both bolding/italics applied (this bug!)

Comment 6

[John Daggett (:jtd)]

Attachment: screenshot, testcase under Mac OS X 10.5 with latest trunk

Don't quite understand why synthetic slanting to weight=400 case...

Comment 7

[John Daggett (:jtd)]

Attachment: patch, v.0.1, set logfont italic/bold so that synthetics are disabled when necessary

In the Windows case, synthetic rendering happens when the weight and italic fields are set to bold/italic for a font that lacks bold/italic.  If bold/italic was defined in font entry (i.e. via @font-face rule), disable synthetic bold/italic by setting logfont bold/italic to normal.

Reftest to follow in a separate patch.

Comment 8

[Bill Gianopoulos [:WG9s]]

(In reply to comment #7)
> Created an attachment (id=364480) [details]
> patch, v.0.1, set logfont italic/bold so that synthetics are disabled when
> necessary

Although this patch appears to fix the attached testcase, the originally reported problem that the rendering of http://opentype.info/demo/webfontdemo.html shows different bolding/italics than the reference image here http://opentype.info/demo/webfontdemo.png still persists.

Comment 9

[John Daggett (:jtd)]

Comment on attachment 364480 [details] [diff] [review]
patch, v.0.1, set logfont italic/bold so that synthetics are disabled when necessary

Hmm, I thought the original testcase looked fine but I'll check this again.

Comment 10

[John Daggett (:jtd)]

Try server build with patch v.0.1:

https://build.mozilla.org/tryserver-builds/2009-02-27_13:[e-mail]-dlfontswinsynbold/

Renders same as Safari 3.2 Windows, except Volkorn.  Comparing with reference image will fail if that image was generated on a different platform (e.g. Mac OS).

Windows refuses to load Volkorn, even though the font renders fine if dropped into the system folder.  Windows voodoo, argh...

Comment 11

[Bill Gianopoulos [:WG9s]]

(In reply to comment #8)
> (In reply to comment #7)
> > Created an attachment (id=364480) [details] [details]
> > patch, v.0.1, set logfont italic/bold so that synthetics are disabled when
> > necessary
> 
> Although this patch appears to fix the attached testcase, the originally
> reported problem that the rendering of
> http://opentype.info/demo/webfontdemo.html shows different bolding/italics than
> the reference image here http://opentype.info/demo/webfontdemo.png still
> persists.

Silly me, should not have been trying to test 2 related patches in the same build.  It appears that the patch on bug 480267 breaks the ability to use .otf fonts via @font-face under Windows.

Comment 12

[John Daggett (:jtd)]

Attachment: patch, v.0.6, lookup fullname in name table

On Windows it's possible to look up a single face with CreateFont or CreateIndirectFont.  This either returns the given face or it returns a fallback font but it's not easy to figure out whether the font returned is the fallback font or not.  Also, these API calls match localized full fontnames, which is undesirable because those same localized names won't match across locales.  I put wording in the spec to specifically require English name matching, which works independent of the underlying locale.

So the lookup algorithm for src local() boils down to (1) call CreateFontIndirect and (2) read the English fullname (or first available if no English name) from the name table.  The name table util routines should be completely cross-platform, they take a byte array with the name table data as input.

I also did a bit of restructuring within the FontEntry creation code but this could still be simplified some more.

To do:

* set up a reftest that tests both English and non-English fullnames on each platform
* cleanup the font entry creation so CreateIndirectFont doesn't get called twice when looking up local font faces

Comment 13

[Bill Gianopoulos [:WG9s]]

(In reply to comment #10)
> Windows refuses to load Volkorn, even though the font renders fine if dropped
> into the system folder.  Windows voodoo, argh...

Was there ever a bug filed on this issue?

Comment 14

[John Daggett (:jtd)]

(In reply to comment #13)
> (In reply to comment #10)
> > Windows refuses to load Volkorn, even though the font renders fine if dropped
> > into the system folder.  Windows voodoo, argh...
> 
> Was there ever a bug filed on this issue?

No, but feel free to do so.  Probably a WONTFIX but good to have it logged anyways.  Not really sure why Windows is fussy about that font.

Comment 15

[John Daggett (:jtd)]

Attachment: patch, v.0.7, add reftest to test against localized names

Add a test that explicitly tests to make sure localized fullnames don't match, since only the "canonical" English name is guaranteed to succeed across locales.

Comment 16

[John Daggett (:jtd)]

Attachment: patch, v.0.7c, cleanup and refresh

Some simple cleanup and deal with switch in Mac headers.

Comment 17

[Karl Tomlinson (:karlt)]

(In reply to comment #15)
> Add a test that explicitly tests to make sure localized fullnames don't match,
> since only the "canonical" English name is guaranteed to succeed across
> locales.

I expect that will fail on Linux systems with a new-enough fontconfig as fontconfig keeps all the fullnames.

We could filter out non-English names.  Has there been must discussion over whether testing all languages or only English is preferred?

Comment 18

[John Daggett (:jtd)]

Attachment: list of fullnames for fonts shipped with Japanese Vista + Office 2007

> > Add a test that explicitly tests to make sure localized fullnames
> > don't match, since only the "canonical" English name is guaranteed
> > to succeed across locales.
> 
> I expect that will fail on Linux systems with a new-enough fontconfig
> as fontconfig keeps all the fullnames.
> 
> We could filter out non-English names.  Has there been must discussion
> over whether testing all languages or only English is preferred?

Yes, that's what the attached patch does on Windows, first lookup using CreateFontIndirect, then compare the name with the English fullname in the name table.  The second step excludes localized names.  I tested the reftest on Ubuntu 8.04, has fontconfig support for these names changed recently?  Or maybe I need to play around with locales?  Or with more fonts containing localized fullnames?

Localized names are problematic for use with a lookup-by-fullname feature because they may succeed in some cases but fail in other cases, even for the same font.  For example, on Windows Japanese names will match when used on a Japanese version of the OS but fail under the English version.  English fullnames will match in both situations.  While some font families in non-Latin language locales have non-English family names, fullnames typically include the style names (e.g. Bold, Italic) so there are a *lot* more localized versions of these. [see attached fullname list for examples]

Microsoft recommends that font vendors always include English names, with other languages optional:

"Each set of name records should appear for US English (language ID = 0x0409 for Microsoft records, language ID = 0 for Macintosh records); additional language strings for the Microsoft set of records (platform ID 3) may be added at the discretion of the font vendor."

  (http://www.microsoft.com/typography/otspec/recom.htm)

It would be nice if we could just always specify font face lookups using Postscript names which are unique and never localized, but that's not really well-supported across platforms (e.g. Windows and fontconfig).

The subject of fullname use came up at the CSS F2F meeting this month and I plan to put more specific language regarding this into the next CSS Fonts draft.  I'll post a message on www-style about this, as I expect to get pushback on this from the W3C Internationalization folks, keeping the world safe for Verdana Félkövér.

Attached is a list of localized fullnames for fonts shipped with Japanese Vista + Office 2007.

You can test the reftest case here:

http://people.mozilla.org/~jdaggett/font-face/src-list-local-localized.html

Comment 19

[Karl Tomlinson (:karlt)]

(In reply to comment #18)

Thanks for the explanation, John.  I can understand your motivation.  We'll see if the i18n folks do to ;)

> I tested the reftest on Ubuntu 8.04, has fontconfig support for these
> names changed recently?

I think this may be the same issue as described in bug 478183 comment 1.
The fix was committed 2007-10-24 so Ubuntu 8.04 may not have the fix.

Check the output of "fc-list : fullnamelang" (or "fc-list : fullname").

With the fullname elided, I think the code is constructing a fullname from the first (preferred, if there) family and style found in the name table, which will often, but not necessarily always, be English.  (I didn't expect fullnames to be missing for OpenType fonts when I wrote the code.)

> You can test the reftest case here:
> 
> http://people.mozilla.org/~jdaggett/font-face/src-list-local-localized.html

Thanks.  I see lots of "A"s.  Maybe it's best to mark this random on Linux at this stage.

Comment 20

[Jonathan Kew [:jfkthame]]

To make name-table access more consistent across platforms, we could consider using something along these lines (extracted from a separate WIP patch, won't apply directly as-is but just to show the idea):

+// determine which charset to use, given font name table data
+static const char*
+GetCharsetForFontName(PRUint16 aPlatform, PRUint16 aScript, PRUint16 aLanguage)
+{
+    switch (aPlatform) {
+    case NameRecord::PLATFORM_ID_MAC:
+        switch (aScript) {
+        case NameRecord::ENCODING_ID_MAC_ROMAN:
+            switch (aLanguage) {
+            default:
+                return "x-mac-roman";
+            case NameRecord::LANG_ID_MAC_ICELANDIC:
+                return "x-mac-icelandic";
+            case NameRecord::LANG_ID_MAC_TURKISH:
+                return "x-mac-turkish";
+            case NameRecord::LANG_ID_MAC_POLISH:
+            case NameRecord::LANG_ID_MAC_CZECH:
+            case NameRecord::LANG_ID_MAC_SLOVAK:
+                return "x-mac-ce";
+            case NameRecord::LANG_ID_MAC_ROMANIAN:
+                return "x-mac-romanian";
+                // TODO: probably more language codes that should be given
+                // special handling here
+            }
+
+        case NameRecord::ENCODING_ID_MAC_JAPANESE:
+            return "Shift_JIS";
+        case NameRecord::ENCODING_ID_MAC_TRAD_CHINESE:
+            return "Big5";
+        case NameRecord::ENCODING_ID_MAC_KOREAN:
+            return "EUC-KR";
+
+        case NameRecord::ENCODING_ID_MAC_ARABIC:
+            switch (aLanguage) {
+            default:
+                return "x-mac-arabic";
+            case NameRecord::LANG_ID_MAC_FARSI:
+            case NameRecord::LANG_ID_MAC_URDU:
+                return "x-mac-farsi";
+            }
+
+        case NameRecord::ENCODING_ID_MAC_HEBREW:
+            return "x-mac-hebrew";
+        case NameRecord::ENCODING_ID_MAC_GREEK:
+            return "x-mac-greek";
+        case NameRecord::ENCODING_ID_MAC_CYRILLIC:
+            return "x-mac-cyrillic";
+        case NameRecord::ENCODING_ID_MAC_DEVANAGARI:
+            return "x-mac-devanagari";
+        case NameRecord::ENCODING_ID_MAC_GURMUKHI:
+            return "x-mac-gurmukhi";
+        case NameRecord::ENCODING_ID_MAC_GUJARATI:
+            return "x-mac-gujarati";
+        case NameRecord::ENCODING_ID_MAC_SIMP_CHINESE:
+            return "GB2312";
+        }
+        break;
+
+    case NameRecord::PLATFORM_ID_UNICODE:
+    case NameRecord::PLATFORM_ID_MICROSOFT:
+        return "UTF-16BE";
+    }
+    
+    return nsnull;
+}
+
+
+// convert a raw name from the name table to an nsString, if possible
+PRBool
+gfxFontUtils::DecodeFontName(const PRUint8 *aBuf, PRInt32 aLength, 
+                             PRUint16 aPlatformCode, PRUint16 aScriptCode,
+                             PRUint16 aLangCode, nsAString& dest)
+{
+    NS_ASSERTION(aLength > 0, "bad length for font name data");
+
+    const char *csName = GetCharsetForFontName(aPlatformCode, aScriptCode, aLangCode);
+    if (csName == nsnull) {
+        NS_WARNING("skipping font name, unknown charset");
+        return PR_FALSE;
+    }
+
+    nsresult rv;
+    nsCOMPtr<nsICharsetConverterManager> ccm =
+        do_GetService(NS_CHARSETCONVERTERMANAGER_CONTRACTID, &rv);
+    NS_ASSERTION(NS_SUCCEEDED(rv), "failed to get charset converter manager");
+
+    nsCOMPtr<nsIUnicodeDecoder> decoder;
+    rv = ccm->GetUnicodeDecoderRaw(csName, getter_AddRefs(decoder));
+    NS_ASSERTION(NS_SUCCEEDED(rv), "failed to get the decoder for a font name string");
+
+    PRInt32 destLength;
+    rv = decoder->GetMaxLength((const char*)aBuf, aLength, &destLength);
+    if (NS_FAILED(rv)) {
+        NS_WARNING("decoder->GetMaxLength failed, invalid font name?");
+        return PR_FALSE;
+    }
+
+    dest.SetLength(destLength); // ensure space for the converted string
+    rv = decoder->Convert((const char*)aBuf, &aLength, dest.BeginWriting(), &destLength);
+    if (NS_FAILED(rv)) {
+        NS_WARNING("decoder->Convert failed, invalid font name?");
+        return PR_FALSE;
+    }
+    dest.SetLength(destLength); // set the actual length
+
+    return PR_TRUE;
+}

This allows our name decoding to behave identically across all platforms, including the ability to read MacJapanese names etc on other machines.

Comment 21

[John Daggett (:jtd)]

(In reply to comment #20)
> This allows our name decoding to behave identically across all platforms,
> including the ability to read MacJapanese names etc on other machines.

Interesting.  I also think it would be important to handle the other MS encodings (Shift-JIS, Big5, etc.) since those exist in actual fonts.  Need to figure out whether this is necessary too.

Comment 22

[Jonathan Kew [:jfkthame]]

True, that should be done for completeness. I don't recall noticing fonts that used different MS-platform encodings in the 'name' table, but that's probably because I haven't normally been looking for them. Anyhow, the spec does allow for it so I think we ought to handle them. A few extra switch cases are cheap. :)

Comment 23

[Jonathan Kew [:jfkthame]]

Also need to support the (deprecated) PLATFORM_ID_ISO, as that's the (only) platform code supported in MT Extra (at least the version I have, which I think comes from MS Office).

I'm currently doing this as part of a larger patch to refactor font management, taking gfxQuartzFontCache as a starting point for a platform-independent font family/style manager to give us uniform handling of families and styles across all platforms. This will eventually need merging with other font-related stuff that's going on.

Comment 24

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 369415 [details] [diff] [review]
patch, v.0.7c, cleanup and refresh

Looks fine to me here.

Comment 25

[John Daggett (:jtd)]

Attachment: patch, fix bustage on windows mobile build

I omitted an #ifdef in one place.

Comment 26

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 370569 [details] [diff] [review]
patch, fix bustage on windows mobile build

I can build locally with this patch

Comment 27

[John Daggett (:jtd)]

Pushed to trunk:
http://hg.mozilla.org/mozilla-central/rev/8d60bedd277b

Fix Windows Mobile bustage:
http://hg.mozilla.org/mozilla-central/rev/eb0b999b2f70

Comment 28

[John Daggett (:jtd)]

Reverted checkins due to tinderbox crash on windows
http://hg.mozilla.org/mozilla-central/rev/b5988827edd7

Tinderbox log of crash:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1238643557.1238650321.26630.gz&fulltext=1

*** 31886 INFO Running /tests/dom/tests/mochitest/ajax/offline/test_xhtmlManifest.xhtml...

Operating system: Windows NT
                  5.2.3790 Service Pack 2
CPU: x86
     AuthenticAMD family 15 model 33 stepping 2
     1 CPU

Crash reason:  EXCEPTION_ACCESS_VIOLATION
Crash address: 0x0

Thread 0 (crashed)
 0  xul.dll!nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:8d60bedd277b : 267 + 0x5]
    eip = 0x101c50bc   esp = 0x0013e11c   ebp = 0x0013e174   ebx = 0x0a43e501
    esi = 0x0a43e588   edi = 0x0a34bdc4   eax = 0x00000000   ecx = 0x0a43e588
    edx = 0x20980012   efl = 0x00010202
 1  xul.dll!gfxTextRun::~gfxTextRun() [gfxFont.cpp:8d60bedd277b : 1409 + 0xf]
    eip = 0x10633b42   esp = 0x0013e124   ebp = 0x0013e174
 2  xul.dll!gfxTextRun::`vector deleting destructor'(unsigned int) + 0x35
    eip = 0x101bdd83   esp = 0x0013e12c   ebp = 0x0013e174
 3  xul.dll!nsTextFrame::ClearTextRun() [nsTextFrameThebes.cpp:8d60bedd277b : 3692 + 0x7]
    eip = 0x102722b1   esp = 0x0013e138   ebp = 0x0013e174   ebx = 0x0a43e578
 4  xul.dll!nsTextFrame::Destroy() [nsTextFrameThebes.cpp:8d60bedd277b : 3345 + 0x4]
    eip = 0x10272b62   esp = 0x0013e144   ebp = 0x0013e174   ebx = 0x00000000
 5  xul.dll!nsLineBox::DeleteLineList(nsPresContext *,nsLineList &) [nsLineBox.cpp:8d60bedd277b : 337 + 0x7]
    eip = 0x1033d670   esp = 0x0013e14c   ebp = 0x0013e174
 6  xul.dll!nsBlockFrame::Destroy() [nsBlockFrame.cpp:8d60bedd277b : 298 + 0x9]
    eip = 0x1033767b   esp = 0x0013e158   ebp = 0x0013e174
 7  xul.dll!nsLineBox::DeleteLineList(nsPresContext *,nsLineList &) [nsLineBox.cpp:8d60bedd277b : 337 + 0x7]
    eip = 0x1033d670   esp = 0x0013e17c   ebp = 0x0013e1a4
 8  xul.dll!nsBlockFrame::Destroy() [nsBlockFrame.cpp:8d60bedd277b : 298 + 0x9]
    eip = 0x1033767b   esp = 0x0013e188   ebp = 0x0013e1a4
 9  xul.dll!nsFrameList::DestroyFrames() [nsFrameList.cpp:8d60bedd277b : 67 + 0x7]
    eip = 0x102a7bb8   esp = 0x0013e1ac   ebp = 0x0013e1cc

Comment 29

[John Daggett (:jtd)]

Relanded on trunk
http://hg.mozilla.org/mozilla-central/rev/5ae3a42f870b
http://hg.mozilla.org/mozilla-central/rev/d3240cd1c610

The crash in comment 28 looks like it's been around for at least a week, digging through build logs for Tinderbox oranges on Windows, I'm seeing this back to 3/20.  Will log separate bug for that.

Comment 30

[John Daggett (:jtd)]

Pushed to 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e222af41889e