Bug to track implementation of the CSS2 @font-face rule.
This should get done :)
Here are the corresponding Mozilla and KHTML bugs: https://bugzilla.mozilla.org/show_bug.cgi?id=70132 http://bugs.kde.org/show_bug.cgi?id=128920 And here is some Qt code to download and use a font: http://labs.trolltech.com/blogs/2006/08/04/fun-with-fonts/
Created attachment 14324 [details] Snapshot of work in progress What I've done so far (doesn't compile).
Created attachment 14349 [details] Another snapshot (compiles at least)
Created attachment 14351 [details] More work in progress This patch gets loading notification hooked up correctly. Fonts will now properly delay the display of a page in the same way that stylesheets do.
Created attachment 14368 [details] Work in progress Handle checking for the existence of local fonts to avoid downloading remote resources. Make sure to hash bold and italic variants separately so that they don't blow away their counterparts.
Created attachment 14370 [details] Make local() work This patch actually compiles and gets the remapping of local fonts completely working. Now a stylesheet can actually define what families to use for serif, sans-serif, etc.
Created attachment 14373 [details] And with this patch, downloadable fonts live. Downloadable fonts live! There's still some cleanup work to do. (1) Should look for the .eot extension and short-circuit EOT loads to prevent clashing with Internet Explorer most of the time. (2) Should add a FontData cache for various sizes to CSSFontFaceSources. (3) When CSSFontFaceSources die, they need to delete these FontDatas and prune the glyph tree. (4) The glyph tree should ideally be decorated with a count of deletable branches, so that pruning will be quick. (5) Other platforms need to have some stuff stubbed out to keep building, since there are a number of new methods/classes that require native implementations.
Also, don't try to apply this or build it, as WebKitSystemInterface had to be changed to allow for display/measuring with no NSFont present.
Ahh, I was ust about to try :-) Does this allow one to set the -webkit-body font from CSS too?
No, that's just an internal value that we use when resetting tables. It's not important.
I thought it was the font selected in the preferences, the one above monospace, and was used for rendering runs that have no author-specified font. (e.g. "<html>This text will be in the body font.") The reason I would like this to work with @font-face is so that I can restrict the unicode range of my body font, Gentium, to exclude Cyrillic characters, because many of the Cyrillic characters are missing from Gentium Italic, and a run of italic will often have odd characters revert to Lucida Grande. I would rather force Gentium never to be selected for Cyrillic so that this doesn't happen. It also has several missing characters used in Serbian and Old Church Slavonic, even in the upright, with the same ugly substitutions occuring.
I don't have unicode-range working yet anyway heh. :)
Yep, I know. Just thinking ahead :-) One more thing that should be easy to do, a way to disable use of a font by websites (yet still have it enabled in FontBook) by setting the src to nil. You can also implement the remapping functions (e.g. Arial to Helvetica) as CSS now, instead of hard-coding them into WebKit.
Created attachment 14377 [details] Add support for format(), don't download urls that end with .eot This patch implements support for the format() qualifier as a way of avoiding the download of unsupported font format types. In addition, just for compatibility with WinIE, we go ahead and assume that URLs ending with ".eot" are Embedded OpenType and refuse to download them.
"You can also implement the remapping functions (e.g. Arial to Helvetica) as CSS now, instead of hard-coding them into WebKit." Yeah, that would be neat, except that I didn't make @font-face work in user agent sheets. The difficulty is that I have this CSSFontSelector object that is per-document, but font face rules in the user agent sheet are parsed before you have a document. I'm also a bit concerned that having a font selector instantiated will cause a slight performance hit (it's nice that in the common case the object is null, so a simple pointer check avoids doing any more work). I'll add that to the list of open issues.
"The reason I would like this to work with @font-face is so that I can restrict the unicode range of my body font, Gentium, to exclude Cyrillic characters." Added unicode-range to the list of open issues. I think I'll morph -webkit-body into what you suggest (I'll change the name to -webkit-default or -webkit-standard or something a little less HTML-centric in name).
Created attachment 14381 [details] Start work on memory cleanup This patch implements the FontData size cache for each CSSFontFaceSource. This allows repeated requests for the same size of custom font to avoid making extra font datas. New in this patch also is a general memory cleanup. All structures now delete themselves properly on document teardown. Custom fonts are now also deactivated the moment they are no longer referenced by a document. The one open issue with memory usage is that FontDatas need to be pruned out of the glyph tree. I even have a page that demonstrates the bug, since a new FontData can be malloc'd with the exact same address of an old one, and suddenly you start handing back the wrong glyphs.
Created attachment 14383 [details] Patch that cleans up the GlyphPageTreeNodes. This patch implements the cleanup of FontDatas in the GlyphPageTreeNode tree. Everything seems to clean up properly now. I implemented my "custom font count" optimization for avoiding branches of the tree that don't have custom fonts in them.
Created attachment 14384 [details] Patch that improves page load when custom fonts are present This patch is cool. Instead of holding up the display of the page, I now make a loading placeholder FontData*. Once the font does load, the placeholder is swapped out for the real thing. The placeholder font will measure but will not render. The overall effect is similar to what you might see if images had been used in the same place as the downloadable fonts.
Created attachment 14385 [details] Include cache statistic reporting for CachedFonts
A document body is a body regardless of whether it's HTML or not. That's why TimBL chose <body> to represent it ;-) I don't think it needs changing to something else just because that other name is not an HTML element. It also makes it easier to understand for web authors, just leaving it as -webkit-body. FWIW Gecko makes users select either "serif" or "sans-serif" as the default body text, and then uses the font for that style, since all five generic families must be defined. "This patch is cool." :o) Regarding CSS3 being smelly, or however you phrased it on public-html, you can always implement it your preferred way, and then advocate that to the css3-web-fonts editors. With an existing implementation behind you, your opinions will be much stronger. But you also have to remain compatible with IE4-7 and Prince, the only extant implementations I know of. I have no idea how much of IE's design the W3C took on board, but Prince seems to follow CSS 2 closely AFAICT. And regarding restricting the unicode-range of a font, it would be lovely to have a unicode-range-exclusions property whereby I can just define U+400 to U+52F as excluded, and allow the font to be updated and add new codepoints, without having to update my CSS. Or alternativly allow an inverse() function for the unicode-range property (probably better for inheritance). Without either of these, I have to enumerate every codepoint in the font that I want to be allowed, a nightmare for large fonts.
Deleting the GlyphTreePageNode's m_page is bad here, since a child can be pointing to a parent's page. I think I'll need to fix glyph pages to be reference counted to resolve this.
Created attachment 14408 [details] Make GlyphPage refcounted. Glyph tree pruning doesn't work right unless glyph pages are reference counted. The code reads better with this change anyway! :)
Open issues list: (1) Need to support "all" value for font-weight, font-style as well as lists of values. (2) Need to cull out unsupported properties from @font-face rules. (3) Need to support font-size in the descriptor (4) Need to support unicode-range in the descriptor (5) Rename -webkit-body to -webkit-default and make it set on the document so that it's possible to override from a user sheet (6) Need to make sure other platforms build
(In reply to comment #24) > Created an attachment (id=14408) [edit] > Make GlyphPage refcounted. > Isn't it enough to check for ownership by testing if you share the page with your parent?
While I think you are right, I think the code reads bettter with the reference counting in place (and it made the three spots that deleted pages read more nicely).
Actually I think this case gets you in trouble if you don't refcount: else if (parentPage && parentPage->owner() != m_parent) { // The page we're overriding may not be owned by our parent node. // This happens when our parent node provides no useful overrides // and just copies the pointer to an already-existing page (see // below). // // We want our override to be shared by all nodes that reference // that page to avoid duplication, and so standardize on having the // page's owner collect all the overrides. Call getChild on the // page owner with the desired font data (this will populate // the page) and then reference it. m_page = parentPage->owner()->getChild(fontData, pageNumber)->page(); } In the above case you aren't sharing a page with your parent but with, e.g., a cousin.
Created attachment 14446 [details] Fix memory management model for the font selector. This patch passes all the layout tests. An SVG layout test that tried to use an SVG font-face exposed a major bug in the lifetime of FontSelectors. I made the FontSelector reference counted (both by the CSSStyleSelector and by FontFallbackLists). The Font == operator now compares the font selectors in an old and new font and will return false if they are different. I also made sure to delete the font selector if it ends up not containing any real font-face rules (e.g., because none of the formats are supported).
Created attachment 14448 [details] Make other platforms build Fix a bunch of stupidity in CSSFontFaceSource where I was just using Mac code directly. Push it down behind FontCustomPlatformData. Add a few ifdefs to CachedFont for now so that every other platform can just ignore FontCustomPlatformData.
Comment on attachment 14448 [details] Make other platforms build This is ready for a first round of comments and suggestions.
I'm looking for review to land on the new feature branch.
Hunk #9 of WebCore/WebCore.xcodeproj/project.pbxproj fails when applying to revision 21342. Can be overcome fairly easily using a text editor. Other hunks are offset by a few lines too. Will run build in the morning.
You still won't be able to build. I'm going to produce a patch that can be landed on trunk that updates WebKitSystemInterface to use the changed APIs. Then I'll make a new patch that should easily apply to trunk.
Okay, noted. Be aware there is at least one other build problem: FontPlatformDataMac.mm is missing. It would also be considerate if you could have the bold synthesis use unsigned ints representing weight instead of a bool, to be compatible with bug #6484 when that gets finished.
Comment on attachment 14448 [details] Make other platforms build Clear review bit, since I missed a file.
Created attachment 16419 [details] Patch that can be applied to trunk on Mac only This patch can be applied to trunk (for anyone interested in trying out font-face). I'm working now on making font-face work on Win32.
Created attachment 16434 [details] Patch that implements font-face on Mac and Windows This is ready for an initial review as basic examples now work on both Mac and Windows.
Comment on attachment 16434 [details] Patch that implements font-face on Mac and Windows This is unused (in CSSFontFaceSource.h) + bool m_tableInvalid; (In CSSFontSelector::addFontFaceRule) I think this should be initialized to false! + bool foundLocal = true; WHOA! + if (familyName.lower() == "port credit") + printf("WHOA!\n"); Can this leak the old fontFace if you have another rule for the same family and style? + m_fonts.set(hashForFont(familyName.lower(), fontDescription.bold(), fontDescription.italic()), fontFace);
Comment on attachment 16434 [details] Patch that implements font-face on Mac and Windows Additional comments... This needs at least a comment about the secret value 3 + ATSFontActivateFromMemory((void*)buffer->data(), buffer->size(), 3, kATSFontFormatUnspecified, NULL, kATSOptionFlagsDefault, &containerRef); Don't you mean (fontCount != 1) here? + // Don't support multiple fonts for now. + if (fontCount == 0) {
I wonder if this should return true as long as one of the sources is valid? It seems getFontData() only cares about the first source which returns font data. bool CSSFontFace::isValid() const 57 { 58 unsigned size = m_sources.size(); 59 if (!size) 60 return false; 61 for (unsigned i = 0; i < size; i++) { 62 if (!m_sources[i]->isValid()) 63 return false; 64 } 65 return true; 66 }
Comment on attachment 16434 [details] Patch that implements font-face on Mac and Windows Is there a Ptr type (like OwnPtr) which could make more clear the fact that: void addSource(CSSFontFaceSource*); takes ownership of the source? Yet another area of code where bug 15331 would be useful. This comment makes the if feel backwards: // If we are still loading, then we let the system pick a font. I think you could just change the return type here to be AtomicString and it would still perform the same "uniquing" trick that you seem to be after: static String hashForFont(const String& familyName, bool bold, bool italic) 69 { 70 String familyHash(familyName); 71 if (bold) 72 familyHash += "-webkit-bold"; 73 if (italic) 74 familyHash += "-webkit-italic"; 75 return AtomicString(familyHash); 76 } I believe these .get() calls are unnecessary: 84 if (!fontFamily.get() || !src.get() || !fontFamily->isValueList() || !src->isValueList()) AFAIK RefPtr has a ! operator, as well as should be able to convert to a bool automagically for things like ptr && otherPtr. I would almost write a "static bool isNonEmptyValueList(CSSValue*)" function to perform the checks, since it seems to take several lines each. But such is certainly not necessary. Typo: 251 else if (familyName == "-webkit-fantasy") 252 genericFamily = settings->cursiveFontFamily(); It's hard to imagine we don't have a cleaner way to express this with the string classes: 2480 parsedValue = new CSSFontFaceSrcValue(String(KURL(styleElement->baseURL().deprecatedString(), value.deprecatedString()).url()), m_systemFallbackChild must have been leaking before and we didn't notice. The deletion in the destructor is new in this patch. Isn't there a CFRetainPtr or something? I could have sworn... if (f && objc_collecting_enabled()) 34 CFRetain(f); // fix for <rdar://problems/4223619> That's all. Other than Mitz's comments, it looks great. I very much look forward to seeing this on feature-branch. :)
Created attachment 16483 [details] Address mitz and eric comments This patch addresses comments. I don't think the hash can leak, Mitz, since it's a hash of RefPtrs?
Created attachment 16484 [details] Maybe I should attach something that actually compiles.
Ahha! There is actually a RetainPtr, which you could certainly use in this patch to get rid of the need for explicit CFRetain/CFRelease. RetainPtr.h is the file in question.
(In reply to comment #42) > Is there a Ptr type (like OwnPtr) which could make more clear the fact that: > void addSource(CSSFontFaceSource*); > takes ownership of the source? Yes, std::auto_ptr.
Fixed on feature branch.
Dave: how many of the to-dos mentioned in comment #25 are left open? Have you filed bugs for each of them?