The MathML TrueType fonts use a custom "Unicode" cmap. The Linux TrueType font code does not handle this. When the cmex10/cmsy10 TrueType fonts are found the font search code spends so *long* searching for the glyphs that it appears to be in an infinite loop. To add support for TrueType MathML fonts we need to: add fontEncoding.properties to the Linux distribution add code to detect the MathML fonts add code to instanciate the needed converter add code to get the CCMap from the converter instead of the font's cmap add a subclass to the linux TrueType code to call the converter in the measure/draw code

*** Bug 126465 has been marked as a duplicate of this bug. ***

it is very very slow but not hang. But I think we need to fix this. ->nsbeta1

the "hang" issue is addressed in bug 127283

status report: I believe I am beginning to see MathML using TrueType. I hope to have a patch late today or early tomorrow. Currently I have about -80 lines and +600 lines.

I'm seeing MathML displayed. There are still some issues but I am seeing reasonable (not perfect) display.

Got a preliminary screenshot? The requirements/precision involved in the rendering of MathML are somewhat overkill.

This screen shot shows basic functioning. I have not worked on tuning the metrics yet.

this patch show basic functioning but it still needs some work

To cheer you up... I have already had my hard-disk replaced twice. The numerous compilations that it takes to get these MathML alignment & spacing right took a toll on them (fortunately there were still under warranty).

I believe this is a usable verion. known bugs: Fails to draw the 'less than or equal' char, 0xB7, in the cmsy10 font. Interestingly, the windows client does draw it but the moz web page screenshot shows it, 0xB7, missing. http://www.mozilla.org/projects/mathml/fonts/encoding/cmsy-ttf.gif Layout is not perfect. There is a gap showing when certian chars are stacked vertically. The drawing code is complaining that it is unable to get the background when the text area is partially showing (sub_image). It appears to be drawing okay so this does not seem critical but should be fixed. Given another week I can address these. Roger: would you kindly review this?

Roger: would you kindly review this?

I talked to jag about the string handling

Comment on attachment 71581 [details] [diff] [review] changed string handling per jag's suggestion ====================== +PRBool +nsFT2FontCatalog::UsesCustomEncoder(const char *aFamilyName) -> HasCustomEncoder() + if (ffei) + return PR_TRUE; + else + return PR_FALSE; -> return ffei != nsnull; (But I noted that nobody is using this UsesCustomEncoder() anyway. It might be a remnant as you were coding and trying things out, and might go away if not needed anymore) ====================== +nsTTFontEncoderInfo FEI_Adobe_Symbol_Encoding = { + // I do not have this font so I have not checked the PlatformID/Encoding -> This is the standard Symbol font which you can get from any windows box (same for wingdings & webdings which are mapped to windows_1252) + "Adobe-Symbol-Encoding", 1, 0, nsnull -> Need symbolic constants for these '1', '0', '3', ... ======================== +typedef struct nsTTFontEncoderInfo { + const char *mConverterName; + PRUint8 mCmapPlatformID; + PRUint8 mCmapEncoding; + nsIUnicodeEncoder* mConverter; +} nsTTFontEncoderInfo; -> duplicate /nsTTFontEncoderInfo/ ========================= +#define FCE_FLAGS_UNICODE 1 +#define FCE_FLAGS_SYMBOL 2 -> 0x01, 0x02 (so that a reader can think twice and see that the next isn't the numeral '3') ================= NS_ASSERTION(0, "..."); -> NS_ERROR("...") ========================= + // in this hash each ttc face has a unique key + nsCAutoString faceTag(nsFT2FontCatalog::GetFamilyName(aFce)); nsCStringKey key(faceTag); -> nsCStringKey key(nsFT2FontCatalog::GetFamilyName(aFce)); to avoid multiple string copying Due to the time difference, I may not follow up -- but the patch looks okay and includes some other niceties like pref'ing the libname. I have updated cmsy-ttf.gif with a version that shows up the less-or-equal glyph. The patch didn't tune the metrics (math people will complaint about that -- they are very regardful about such things) but even a long journey starts with a single step. So r=rbs applies from now on.

> +nsFT2FontCatalog::UsesCustomEncoder(const char *aFamilyName) Removed > -> Need symbolic constants for these '1', '0', '3', ... Used FreeType2 defines > +typedef struct nsTTFontEncoderInfo { ... > +} nsTTFontEncoderInfo; > > -> duplicate /nsTTFontEncoderInfo/ Removed struct tag > +#define FCE_FLAGS_UNICODE 1 > +#define FCE_FLAGS_SYMBOL 2 > > -> 0x01, 0x02 (so that a reader can think twice and see that the > next isn't the numeral '3') Done > NS_ASSERTION(0, "..."); > -> NS_ERROR("...") Thanks, fixed > + nsCAutoString faceTag(nsFT2FontCatalog::GetFamilyName(aFce)); > nsCStringKey key(faceTag); > > -> nsCStringKey key(nsFT2FontCatalog::GetFamilyName(aFce)); > to avoid multiple string copying Thanks. fixed > I have updated cmsy-ttf.gif with a version that shows up the less-or-equal > glyph. Thanks. I'll check it out. > The patch didn't tune the metrics (math people will complaint about that -- > they are very regardful about such things) but even a long journey starts with > a single step. If we had more time I'd like to fix this. I've opened bug 128000 to address this. > So r=rbs applies from now on. Thanks

Comment on attachment 71660 [details] [diff] [review] changes for rbs marking it reviewed per rbs' comment

Comment on attachment 71660 [details] [diff] [review] changes for rbs >+ if (gFreeType2SharedLibraryName) { >+ free(gFreeType2SharedLibraryName); >+ gFreeType2SharedLibraryName = nsnull; >+ } I remember when it was safe to pass NULL to free(3). Ah, the good old days. >+ rv = gPref->GetCharPref("font.freetype2.shared-library", >+ &gFreeType2SharedLibraryName); >+ if (NS_FAILED(rv)) { >+ enable_freetype2 = PR_FALSE; >+ FREETYPE_FONT_PRINTF(( >+ "gFreeType2SharedLibraryName missing, FreeType2 disabled")); >+ gFreeType2SharedLibraryName = nsnull; >+ } >+ else if (!gFreeType2SharedLibraryName) { >+ FreeGlobals(); >+ return NS_ERROR_OUT_OF_MEMORY; > } Will the pref service really return a success code and a NULL out param to indicate OOM? Doesn't look that way from a quick scan of nsPrefBranch.cpp. >+nsTTFontEncoderInfo FEI_Adobe_Symbol_Encoding = { >+ // I do not have this font so I have not checked the PlatformID/Encoding >+ "Adobe-Symbol-Encoding", TT_PLATFORM_MACINTOSH, TT_MAC_ID_ROMAN, nsnull >+}; >+nsTTFontEncoderInfo FEI_x_ttf_cmr = { >+ // I do not have this font so I have not checked the PlatformID/Encoding >+ "x-ttf-cmr", TT_PLATFORM_MACINTOSH, TT_MAC_ID_ROMAN, nsnull >+}; >+nsTTFontEncoderInfo FEI_x_ttf_cmmi = { >+ // I do not have this font so I have not checked the PlatformID/Encoding >+ "x-ttf-cmmi", TT_PLATFORM_MACINTOSH, TT_MAC_ID_ROMAN, nsnull >+}; School me: what are the effects of having bogus PlatformID/Encoding? Can you get someone who has the fonts to tell you the correct values? > + nsCOMPtr<nsIAtom> charset; > + charset = getter_AddRefs(NS_NewAtom(fei->mConverterName)); Prefer initialization over assignment: nsCOMPtr<nsIAtom> charset(dont_AddRef(NS_NewAtom(fei->mConverterName))); >+ for (i=0; gFontFamilyEncoderInfo[i].mFamilyName; i++) { >+ nsTTFontFamilyEncoderInfo *ffei = &gFontFamilyEncoderInfo[i]; >+ nsTTFontEncoderInfo *fei = ffei->mEncodingInfo; >+ NS_IF_RELEASE(fei->mConverter); Use nsCOMPtr for fei->mConverter? >+ nsServiceManager::GetService(kPrefCID, NS_GET_IID(nsIPref), >+ (nsISupports**) &mPref); >+ if (!mPref) >+ goto cleanup_and_return; nsCOMPtr<nsIPref> mPref; mPref = do_GetService(NS_PREF_CONTRACTID); ? Better to use the contract ID, and always better to use nsCOMPtr where possible. > strncpy((char*)fce->mVendorID, (char*)tt_os2->achVendID, 4); (Do you assure null termination for the strncpy'd string?) > typedef struct { > const char *mFontFileName; > time_t mMTime; > PRBool mIsValid; >+ PRUint32 mFlags; > const char *mFontType; > int mFaceIndex; > int mNumFaces; Do we have a lot of these structs around? Would it be worthwhile to make mIsValid be a bit in mFlags, and save a word per struct? > nsHashtable *mFreeTypeNodes; >+ nsHashtable *mFontFamilies; Mmm, nsHashtable. Have you considered using PLDHash or PLHash for these, for speed and space wins? > // since the library may not be available on any given system > // failing to load is not considered a fatal error > if (!sSharedLib) { >- NS_ASSERTION(0, "freetype library not found"); >+ NS_ERROR("freetype library not found"); > return PR_FALSE; NS_WARNING, for non-fatal diagnostics? >@@ -449,7 +470,7 @@ > for (p=FtFuncs; p->FuncName; p++) { > func = PR_FindFunctionSymbol(sSharedLib, p->FuncName); > if (!func) { >- NS_ASSERTION(0, "nsFreeType::LoadSharedLib Error"); >+ NS_ERROR("nsFreeType::LoadSharedLib Error"); Likewise. >+gint >+nsFreeTypeXImageSBC::GetWidth(const PRUnichar* aString, PRUint32 aLength) >+{ >+ nsresult res; >+ char buf[512]; >+ PRInt32 bufLen = sizeof(buf); >+ PRInt32 stringLen = aLength; >+ nsFontCatalogEntry* fce = mFaceID->GetFce(); >+ nsTTFontFamilyEncoderInfo *ffei = nsFT2FontCatalog::GetCustomEncoderInfo(fce); >+ NS_ASSERTION(ffei,"failed to find font encoder info"); >+ if (!ffei) >+ return NS_ERROR_FAILURE; >+ res = ffei->mEncodingInfo->mConverter->Convert(aString, &stringLen, >+ buf, &bufLen); >+ NS_ASSERTION((aLength&&bufLen)||(!aLength&&!bufLen), "converter failed"); >+ >+ // >+ // Widen to 16 bit >+ // >+ PRUnichar unibuf[512]; >+ int i; >+ for (i=0; i<bufLen; i++) { >+ unibuf[i] = (unsigned char)buf[i]; >+ } Can GetWidth and GetBoundingMetrics and DrawString share some of this copy-and-pasted conversion code, via a helper?

> >+ if (NS_FAILED(rv)) { ... > >+ } > >+ else if (!gFreeType2SharedLibraryName) { deleted unnecessary code > >+nsTTFontEncoderInfo FEI_x_ttf_cmr = { > >+ // I do not have this font so I have not checked the PlatformID/Encoding > >+ "x-ttf-cmr", TT_PLATFORM_MACINTOSH, TT_MAC_ID_ROMAN, nsnull > >+}; ... > what are the effects of having bogus PlatformID/Encoding? blank or wrong glyphs but no crash > Can you get someone who has the fonts to tell you the correct values? I found the fonts and edited the table as best I understand it. Surprisingly, neither rbs or erik know which table should be used. It appears to just work on windows (I think this lack of knowledge is scary). > Prefer initialization over assignment: > nsCOMPtr<nsIAtom> charset(dont_AddRef(NS_NewAtom(fei->mConverterName))); Done > >+ NS_IF_RELEASE(fei->mConverter); > > Use nsCOMPtr for fei->mConverter? These are in a static table that maps charset names to converter names. The rest of the GTK code does not do this so it might work but it might not. Because Brendan wants this in for 0.9.9 I'm on a fairly tight deadline. This is one of several items I would like to defer this to bug 128000 whick I have already opened for future TrueType MathML work. > nsCOMPtr<nsIPref> mPref; > mPref = do_GetService(NS_PREF_CONTRACTID); Done > > strncpy((char*)fce->mVendorID, (char*)tt_os2->achVendID, 4); > (Do you assure null termination for the strncpy'd string?) added memset to clear the array (it is 5 bytes long) > > PRBool mIsValid; > >+ PRUint32 mFlags; > Would it be worthwhile to make mIsValid be a bit in mFlags, Done > Mmm, nsHashtable. Have you considered using PLDHash or PLHash for these, > for speed and space wins? I would like to defer this to bug 128000 whick I opened for future work in this area. > NS_WARNING, for non-fatal diagnostics? changed > >+gint > >+nsFreeTypeXImageSBC::GetWidth(const PRUnichar* aString, PRUint32 aLength) > >+{ ... > Can GetWidth and GetBoundingMetrics and DrawString share some of this > copy-and-pasted conversion code, via a helper? This needs to also handle strings larger that the fixed stack buffer. I did this for the nsFontMetricsGTK code and others ported it to the other X FEs. Again since I am on a deadline I would like to defer this to bug 128000.

Comment on attachment 71779 [details] [diff] [review] various changes marking has review per rbs' comment

Comment on attachment 71779 [details] [diff] [review] various changes How can I argue with the round-number righteousness of 128000? Make sure you copy all these to-do items into there, though! sr=shaver, thanks for the quick response to review.

I'll add them this evening

Comment on attachment 71779 [details] [diff] [review] various changes a=leaf on behalf of drivers, our math professors will be so proud.

Comment on attachment 71779 [details] [diff] [review] various changes a=shaver for 0.9.9.

checked in

Ok, I am going to probably "upset" people by reopening this... sorry. My only issue is with the unix.js change: +pref("font.freetype2.shared-library", "libfreetype.so.6"); HP's default extension is sl... so on HP this would be libfreetype.sl It is also "hardcoded" to version .6, which I think is a bad idea... (the version of freetype on my hp box is 8... so I have a libfreetype.sl.8) SO my question is there ANY way to put an ifdef in a unix.js file ifdef hpux +pref("font.freetype2.shared-library", "libfreetype.sl"); else +pref("font.freetype2.shared-library", "libfreetype.so.6"); endif Otherwise the solution as it is, ONLY works for for a limited case. I will post either today or tomorrow morning... I am willing to go along with unix.js, IFF we have a fallback in case we can't load this something like PR_LoadLibrary("libfreetype" MOZ_DLL_SUFFIX); as I have mentioned before... diff shortly

Or alternatively, have both pref("font.freetype2.shared-library.hpux", "libfreetype.sl"); pref("font.freetype2.shared-library", "libfreetype.so.6"); and do the #ifdef from the back-end side.

Jim: I think the library name issue belongs in bug 127553. Could we address it there and close this bug?

Having not heard anything in a couple of days I am re-closing this bug. Please reopen if appropiate.