Closed Bug 715798 Opened 9 years ago Closed 8 years ago

add color emoji support to Gecko

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: jtd, Assigned: jfkthame)

References

(Depends on 1 open bug, Blocks 2 open bugs, )

Details

(Whiteboard: parity-webkit)

Attachments

(6 files, 4 obsolete files)

227.32 KB, image/png
Details
1.59 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
3.37 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
15.54 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
2.88 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
6.27 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
OSX 10.7 now ships with a font that supports *color* glyphs for emoji codepoints, "Apple Color Emoji".

Testcase:

1. Open Safari/Chrome on 10.7
2. Click on the URL

Result: pile of poo displays in full color

First step is to investigate the special magic that Webkit uses to display these glyphs.
Looks like the glyphs are bitmaps, zooming in shows the pixelation.
(In reply to John Daggett (:jtd) from comment #1)
> Looks like the glyphs are bitmaps, zooming in shows the pixelation.

Yep. They are PNGs see https://bugzilla.mozilla.org/show_bug.cgi?id=119490#c120
The sbix table has apparently been reverse engineered:
http://typophile.com/node/83760#comment-470876

and it looks like this page has some information on the format but your Japanese is probably better than mine :)

http://kanji-database.sourceforge.net/fonts/opentype.html
(In reply to John Daggett (:jtd) from comment #1)
> Looks like the glyphs are bitmaps, zooming in shows the pixelation.

I'm not sure what you mean it's pixelated. In Safari on Emoji's Wikipedia page, I can zoom quite well.
(In reply to tech163 from comment #4)
> (In reply to John Daggett (:jtd) from comment #1)
> > Looks like the glyphs are bitmaps, zooming in shows the pixelation.
> 
> I'm not sure what you mean it's pixelated. In Safari on Emoji's Wikipedia
> page, I can zoom quite well.

Oops. I wasn't zooming enough. Kind of like bug 638335.
Dingbats and Emoji blocks are replaced by graphical representation on iOS devices for some time and probably also on OSX as you have found out, and I guess it is only a matter of time until this will be implemented in Google Chrome as well. 

On Android devices (tested with 3.x) there is no graphical equivalent icons, and there is no supported font in stock ROM as well as in common modifications such as Cyanogen distribution, but there are some applications out there that are currently support these characters for reading and typing (check out the Emoji addon to GO Keyboard).

I think we should make this option available to all operating systems and not just OSX/iOS, and also need to figure if there should be an option in CSS or HTML to turn on and off this feature, like how we can turn off the spellchecker for input fields. 


You can use my web-based character picker for testing this characters. The page is loading a supported font on the browser and for most users it is currently impossible to read it on regular websites as they don't even have Unicode 6.0 supported font. 

http://tomer.github.com/characterPicker/
I was thinking, maybe this is a duplicate of bug 638335?
(In reply to tech163 from comment #8)
> I was thinking, maybe this is a duplicate of bug 638335?

No because this is specifically focused on how to support the Apple Color Emoji font on OSX 10.7.  This font has some special undocumented tables, basically embedded png images of various sizes, so the question whether to/how to support these.
Once bug 719286 lands, another possible approach would be to bundle a font that provides SVG glyphs for the emoji characters. This would enable us to provide uniform support for colour emoji across all platforms.
Blocks: 727276
Blocks: 490986
See also this Chrome bug: http://code.google.com/p/chromium/issues/detail?id=62435 The discussion in the comments might be useful.
It seems that the basic problem here is that the glyph-drawing APIs we use (CGContextShowGlyphsWithAdvances and friends) don't support the 'sbix' color-bitmap glyphs in the Apple Color Emoji font. These glyphs are only supported through the Core Text glyph-drawing function CTFontDrawGlyphs.

As a proof-of-concept, I put together a hack that replaces the use of CGContextShowGlyphsWithAdvances by CTFontDrawGlyphs in cairo-quartz-surface.c. With this patch, the emoji and other symbols from the Apple Color Emoji font can be rendered in Gecko; see for example http://alanwood.net/unicode/miscellaneous-symbols-and-pictographs.html.

This patch is really just for experimental purposes, it's not ready to land in the tree. I expect there are other places where we may need to replace CGContext text-drawing APIs with CTFont versions in order for this to work everywhere; and we probably don't want to create and destroy the CTFont object on every call, but rather cache it in the cairo_scaled_font or something like that.
Tryserver build with this patch should appear at https://tbpl.mozilla.org/?tree=Try&rev=aed5c3450c28, if all goes well.
...well, the Opt build failed because CTFontDrawGlyphs is 10.7-only, and it looks like we build against the 10.6 SDK there. And the debug build crashes on 10.6 for the same reason, but seems to be running OK on 10.7/10.8.
This adds support for the color emoji font in _cairo_quartz_surface_show_glyphs_cg, by using Core Text instead of CGContext APIs to draw the glyphs (when running on 10.7 or later). Tryserver build at https://hg.mozilla.org/try/rev/a6bb7b43cea5.
Attachment #671124 - Flags: review?(jmuizelaar)
Attachment #669605 - Attachment is obsolete: true
One issue with this patch is that the emoji glyphs don't render at huge font sizes, for reasons I don't currently know (at least in my testing on OS X 10.7). Thus, the example from the URL field here doesn't work until you zoom out by several steps. However, at "normal" sizes they work fine; see for example the charts at http://en.wikipedia.org/wiki/Emoji or http://alanwood.net/unicode/emoticons.html.
To be exact, the color emoji font starts failing as soon as the font size exceeds 128px. Ah, I seem to recall that cairo may switch to a different code path at large sizes?
Further testing confirms that the >128px failure threshold only applies in HiDPI mode, where it corresponds to 256 device pixels; in non-HiDPI mode, the color emoji glyphs render OK at sizes up to 256px.
http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-gstate.c#1974 sounds like cairo will try to switch to filling glyph paths at large sizes; this will fail for the color emoji font, as the glyphs have no paths.
This fixes things so that the color emoji font renders even at huge sizes.
Attachment #671142 - Flags: review?(jmuizelaar)
Tryserver job with both these patches: https://tbpl.mozilla.org/?tree=Try&rev=4dfc9cbe0bbc.
And here are a couple of reftests to check that the emoji codepoints render something visible - even though it'll only be a hexbox in the absence of suitable fonts.
Attachment #672746 - Flags: review?(jmuizelaar)
Assignee: nobody → jfkthame
BTW, I assume if we do this, we'll also want to add the patches as separate files in the gfx/cairo directory for tracking/update/upstreaming purposes.
Jonathan, can you also add support to 2d/DrawTargetCG.cpp? It should be easier than cairo because we use CGContextShowGlyphsAtPositions instead of Advances.
Attachment #672746 - Flags: review?(jmuizelaar) → review+
Attachment #671142 - Flags: review?(jmuizelaar) → review+
Comment on attachment 671124 [details] [diff] [review]
support Apple Color Emoji font in cairo-quartz backend

Review of attachment 671124 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/cairo/cairo/src/cairo-quartz-font.c
@@ +369,5 @@
>  
>      font_face->cgFont = CGFontRetain (font);
>  
> +    if (CTFontCreateWithGraphicsFontPtr) {
> +        font_face->ctFont = CTFontCreateWithGraphicsFontPtr (font, 1.0, NULL, NULL);

How long does a call to CTFontCreateWithGraphicsFont typically take?

::: gfx/cairo/cairo/src/cairo-quartz-surface.c
@@ +614,5 @@
>  
>  typedef struct {
>      bool isClipping;
>      CGGlyph *cg_glyphs;
>      CGSize *cg_advances;

I think it would probably be better to have a CGPoint* in a union with the cg_advances. This should make there sharing a little more explicit.

@@ +718,5 @@
> +	    CGContextSetFont (cgc, _cairo_quartz_scaled_font_get_cg_font_ref (op->u.show_glyphs.scaled_font));
> +	    CGContextSetFontSize (cgc, 1.0);
> +	    CGContextSetTextMatrix (cgc, CGAffineTransformIdentity);
> +	    CGContextTranslateCTM (cgc, op->u.show_glyphs.origin.x, op->u.show_glyphs.origin.y);
> +	    CGContextConcatCTM (cgc, op->u.show_glyphs.textTransform);

Can we factor out the TranslateCTM and ConcatCTM calls?

@@ +2891,5 @@
>  	ub.u.show_glyphs.cg_glyphs = cg_glyphs;
>  	ub.u.show_glyphs.cg_advances = cg_advances;
>  	ub.u.show_glyphs.nglyphs = num_glyphs;
>  	ub.u.show_glyphs.textTransform = textTransform;
> +	ub.u.show_glyphs.scaled_font = scaled_font;

I assume there are no lifetime issues here?
Attachment #671124 - Flags: review?(jmuizelaar) → review-
(In reply to Jeff Muizelaar [:jrmuizel] from comment #26)
> Comment on attachment 671124 [details] [diff] [review]
> support Apple Color Emoji font in cairo-quartz backend
> 
> Review of attachment 671124 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/cairo/cairo/src/cairo-quartz-font.c
> @@ +369,5 @@
> >  
> >      font_face->cgFont = CGFontRetain (font);
> >  
> > +    if (CTFontCreateWithGraphicsFontPtr) {
> > +        font_face->ctFont = CTFontCreateWithGraphicsFontPtr (font, 1.0, NULL, NULL);
> 
> How long does a call to CTFontCreateWithGraphicsFont typically take?

By surrounding it with calls to mach_absolute_time, I'm seeing times (on a MacBook Pro) that are typically in the region of 15-30 microseconds.
Attachment #671124 - Attachment is obsolete: true
(In reply to Jeff Muizelaar [:jrmuizel] from comment #26)
> I think it would probably be better to have a CGPoint* in a union with the
> cg_advances. This should make there sharing a little more explicit.

OK.

> Can we factor out the TranslateCTM and ConcatCTM calls?

Yes, I think so.

> > +	ub.u.show_glyphs.scaled_font = scaled_font;
> 
> I assume there are no lifetime issues here?

Nope; this entire struct is a local variable, only being used to pass in to the _cairo_quartz_fixup_unbounded_operation function, so it's not trying to hold on to the scaled_font pointer beyond the current scope.
This seems to work in my (minimal) testing so far. I've pushed it to tryserver to see how that goes... https://tbpl.mozilla.org/?tree=Try&rev=cc035ebfd225
Attachment #673280 - Flags: review?(jmuizelaar)
A simple reftest to verify that an emoji codepoint renders non-blank in canvas. (I suspect this will fail on some platforms due to lack of font coverage; we'll see what tryserver says.)
Attachment #673281 - Flags: review?(jmuizelaar)
Comment on attachment 673280 [details] [diff] [review]
pt 3 - support Color Emoji font in the gfx/2d quartz backend

Review of attachment 673280 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/DrawTargetCG.cpp
@@ +732,5 @@
>    if (isGradient(aPattern)) {
>      CGContextSetTextDrawingMode(cg, kCGTextClip);
> +    if (CTFontDrawGlyphs != nullptr) {
> +      CTFontDrawGlyphs(macFont->mCTFont, &glyphs.front(), &positions.front(),
> +                       aBuffer.mNumGlyphs, cg);

It worries me that we don't need to do CGContextSetFontSize(). How is the size being set?

::: gfx/2d/ScaledFontMac.cpp
@@ +25,5 @@
>  ScaledFontMac::ScaledFontMac(CGFontRef aFont, Float aSize)
>    : ScaledFontBase(aSize)
>  {
>    // XXX: should we be taking a reference
>    mFont = CGFontRetain(aFont);

Can we if (CTFontDrawGlyphs) this call?

@@ +39,5 @@
>  #ifdef USE_SKIA
>  SkTypeface* ScaledFontMac::GetSkTypeface()
>  {
>    if (!mTypeface) {
> +    mTypeface = SkCreateTypefaceFromCTFont(mCTFont);

If we do conditionalize the CTFontCreateWithGraphicsFont call we can just leave the Skia code untouched.
Attachment #673280 - Flags: review?(jmuizelaar) → review+
Attachment #673281 - Flags: review?(jmuizelaar) → review+
Comment on attachment 673281 [details] [diff] [review]
canvas reftest to check that emoji character is non-blank

Review of attachment 673281 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/canvas/reftest.list
@@ +49,5 @@
>  
>  == strokeText-path.html strokeText-path-ref.html
>  
> +# check that emoji character renders as something non-blank (for Apple Color Emoji font, bug 715798)
> +!= text-emoji.html text-emoji-notref.html

This should probably only run on 10.7 right?
Comment on attachment 673247 [details] [diff] [review]
support Apple Color Emoji font in cairo-quartz backend

Review of attachment 673247 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/cairo/cairo/src/cairo-quartz-surface.c
@@ +2893,5 @@
>  	ub.u.show_glyphs.cg_glyphs = cg_glyphs;
> +	if (CTFontDrawGlyphsPtr) {
> +	    /* we're using Core Text API: the cg_advances array was
> +	       reused (above) for glyph positions */
> +            CGPoint *cg_positions = (CGPoint*) cg_advances;

The reuse of cg_advances is still a little awkward, but I don't feel that strongly.
Attachment #673247 - Flags: review?(jmuizelaar) → review+
(In reply to Jeff Muizelaar [:jrmuizel] from comment #32)
> >      CGContextSetTextDrawingMode(cg, kCGTextClip);
> > +    if (CTFontDrawGlyphs != nullptr) {
> > +      CTFontDrawGlyphs(macFont->mCTFont, &glyphs.front(), &positions.front(),
> > +                       aBuffer.mNumGlyphs, cg);
> 
> It worries me that we don't need to do CGContextSetFontSize(). How is the
> size being set?

The CTFontRef (unlike a CGFontRef) has a size specified when it is created - see the call to CTFontCreateWithGraphicsFont.

> ::: gfx/2d/ScaledFontMac.cpp
> @@ +25,5 @@
> >  ScaledFontMac::ScaledFontMac(CGFontRef aFont, Float aSize)
> >    : ScaledFontBase(aSize)
> >  {
> >    // XXX: should we be taking a reference
> >    mFont = CGFontRetain(aFont);
> 
> Can we if (CTFontDrawGlyphs) this call?
> 
> @@ +39,5 @@
> >  #ifdef USE_SKIA
> >  SkTypeface* ScaledFontMac::GetSkTypeface()
> >  {
> >    if (!mTypeface) {
> > +    mTypeface = SkCreateTypefaceFromCTFont(mCTFont);
> 
> If we do conditionalize the CTFontCreateWithGraphicsFont call we can just
> leave the Skia code untouched.

OK, we can do it that way.
Attachment #673280 - Attachment is obsolete: true
(In reply to Jonathan Kew (:jfkthame) from comment #35)

> > @@ +39,5 @@
> > >  #ifdef USE_SKIA
> > >  SkTypeface* ScaledFontMac::GetSkTypeface()
> > >  {
> > >    if (!mTypeface) {
> > > +    mTypeface = SkCreateTypefaceFromCTFont(mCTFont);
> > 
> > If we do conditionalize the CTFontCreateWithGraphicsFont call we can just
> > leave the Skia code untouched.
> 
> OK, we can do it that way.

Well, not quite - in the case (on 10.7+) where we *have* created a CTFontRef in the constructor, we should still use that for the Skia call, rather than creating a second (throwaway) CTFontRef just for that purpose. Updated patch is attached, carrying forward r+.
Attachment #673339 - Flags: review+
(In reply to Jeff Muizelaar [:jrmuizel] from comment #33)
> Comment on attachment 673281 [details] [diff] [review]
> canvas reftest to check that emoji character is non-blank
...
> This should probably only run on 10.7 right?

Yes - tryserver confirmed that it fails on other platforms (but I'll mark it random on them, as it could pass if they acquire a suitable font), and it's expected to fail on 10.6. Will annotate as needed once tryserver is happy with everything.
Jeff, sorry for the noise; marking r? again as I've had to revise this patch. I couldn't persuade tryserver to like my attempt to use weak-linking for CTFontDrawGlyphs, so in the end I've resorted to an explicit dlsym (like we have in the cairo code) instead.
Attachment #673551 - Flags: review?(jmuizelaar)
Attachment #673339 - Attachment is obsolete: true
Comment on attachment 673551 [details] [diff] [review]
pt 3 - support Color Emoji font in the gfx/2d quartz backend

Review of attachment 673551 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/ScaledFontMac.cpp
@@ +60,5 @@
> +    if (mCTFont) {
> +      mTypeface = SkCreateTypefaceFromCTFont(mCTFont);
> +    } else {
> +      CTFontRef fontFace = CTFontCreateWithGraphicsFont(mFont, mSize, nullptr, nullptr);
> +      mTypeface = SkCreateTypefaceFromCTFont(fontFace);			

Ugh, where did that trailing whitespace come from? Removed in local queue.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #33)
> Comment on attachment 673281 [details] [diff] [review]
> canvas reftest to check that emoji character is non-blank
> 
> Review of attachment 673281 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/reftests/canvas/reftest.list
> @@ +49,5 @@
> >  
> >  == strokeText-path.html strokeText-path-ref.html
> >  
> > +# check that emoji character renders as something non-blank (for Apple Color Emoji font, bug 715798)
> > +!= text-emoji.html text-emoji-notref.html
> 
> This should probably only run on 10.7 right?

For some reason, the canvas test is consistently failing on tryserver on OS X 10.7; the emoji glyph doesn't show up at all. However, whenever I run it locally (also on 10.7), it works fine. I have no idea what's causing this difference in behavior.

(Tryserver has no problem with the tests using emoji in HTML content; the Color Emoji glyphs appear as expected. So the font is definitely present there. It's only the <canvas> test that fails.)

The test passes as expected on tryserver/10.8.

My inclination at this point, subject to Jeff's review of the new gfx/2d patch, would be to land this code as it stands, and file a followup regarding the emoji/canvas/10.7 issue. Unless someone has a better idea...?
Attachment #673551 - Flags: review?(jmuizelaar) → review+
Blocks: 804522
Filed bug 804522 regarding the <canvas> emoji failure on tryserver OS X 10.7 test runs.
Depends on: 805702
Depends on: 804928
Just as an FYI in case this gets uplifted to cairo and this code appears targetted to pre-10.5 versions of OS X, 10.4 does have CTFontCreateWithGraphicsFont() in its secret CoreText and the Ptr will be non-null but it doesn't appear to work right. It will happily try to create the font, and then immediately crash. I ended up telling it not to look for the symbol, and then it just sticks with CGFonts.
Depends on: 804934
Blocks: 1153460
You need to log in before you can comment on or make changes to this bug.