add color emoji support to Gecko

RESOLVED FIXED in mozilla19

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: jtd, Assigned: jfkthame)

Tracking

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

Trunk
mozilla19
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: parity-webkit, URL)

Attachments

(6 attachments, 4 obsolete attachments)

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
(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
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
(Reporter)

Updated

6 years ago

Comment 4

6 years ago
(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.

Comment 5

6 years ago
Created attachment 588057 [details]
Demonstration of emoji in Safari

Comment 6

6 years ago
(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.

Comment 7

6 years ago
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/

Comment 8

6 years ago
I was thinking, maybe this is a duplicate of bug 638335?
(Reporter)

Comment 9

6 years ago
(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.
(Assignee)

Comment 10

6 years ago
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.

Updated

6 years ago
Blocks: 727276

Updated

6 years ago
Blocks: 490986

Comment 11

5 years ago
See also this Chrome bug: http://code.google.com/p/chromium/issues/detail?id=62435 The discussion in the comments might be useful.
(Assignee)

Comment 12

5 years ago
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.
(Assignee)

Comment 13

5 years ago
Created attachment 669605 [details] [diff] [review]
[WIP] - support Apple Color Emoji font
(Assignee)

Comment 14

5 years ago
Tryserver build with this patch should appear at https://tbpl.mozilla.org/?tree=Try&rev=aed5c3450c28, if all goes well.
(Assignee)

Comment 15

5 years ago
...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.
(Assignee)

Comment 16

5 years ago
Created attachment 671124 [details] [diff] [review]
support Apple Color Emoji font in cairo-quartz backend

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)
(Assignee)

Updated

5 years ago
Attachment #669605 - Attachment is obsolete: true
(Assignee)

Comment 17

5 years ago
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.
(Assignee)

Comment 18

5 years ago
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?
(Assignee)

Comment 19

5 years ago
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.
(Assignee)

Comment 20

5 years ago
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.
(Assignee)

Comment 21

5 years ago
Created attachment 671142 [details] [diff] [review]
pt 2 - fall back to show_text_glyphs even at huge sizes if scaled_font_glyph_path didn't work

This fixes things so that the color emoji font renders even at huge sizes.
Attachment #671142 - Flags: review?(jmuizelaar)
(Assignee)

Comment 22

5 years ago
Tryserver job with both these patches: https://tbpl.mozilla.org/?tree=Try&rev=4dfc9cbe0bbc.
(Assignee)

Comment 23

5 years ago
Created attachment 672746 [details] [diff] [review]
reftests to check Unicode emoji characters are not rendered blank

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)

Updated

5 years ago
Assignee: nobody → jfkthame
(Assignee)

Comment 24

5 years ago
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-
(Assignee)

Comment 27

5 years ago
(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.
(Assignee)

Comment 28

5 years ago
Created attachment 673247 [details] [diff] [review]
support Apple Color Emoji font in cairo-quartz backend
Attachment #673247 - Flags: review?(jmuizelaar)
(Assignee)

Updated

5 years ago
Attachment #671124 - Attachment is obsolete: true
(Assignee)

Comment 29

5 years ago
(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.
(Assignee)

Comment 30

5 years ago
Created attachment 673280 [details] [diff] [review]
pt 3 - support Color Emoji font in the gfx/2d quartz backend

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)
(Assignee)

Comment 31

5 years ago
Created attachment 673281 [details] [diff] [review]
canvas reftest to check that emoji character is non-blank

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+
(Assignee)

Comment 35

5 years ago
(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.
(Assignee)

Comment 36

5 years ago
Created attachment 673339 [details] [diff] [review]
pt 3 - support Color Emoji font in the gfx/2d quartz backend
(Assignee)

Updated

5 years ago
Attachment #673280 - Attachment is obsolete: true
(Assignee)

Comment 37

5 years ago
(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+.
(Assignee)

Updated

5 years ago
Attachment #673339 - Flags: review+
(Assignee)

Comment 38

5 years ago
(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.
(Assignee)

Comment 39

5 years ago
Created attachment 673551 [details] [diff] [review]
pt 3 - support Color Emoji font in the gfx/2d quartz backend

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)
(Assignee)

Updated

5 years ago
Attachment #673339 - Attachment is obsolete: true
(Assignee)

Comment 40

5 years ago
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.
(Assignee)

Comment 41

5 years ago
(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+
(Assignee)

Comment 42

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/632afb0c06da (part 1)
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b6cac8af784 (add part 1 patch to gfx/cairo)
https://hg.mozilla.org/integration/mozilla-inbound/rev/74f2eb49f209 (part 2)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6988e710b96 (add part 2 patch to gfx/cairo)
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce1cd7478dfe (reftest)
https://hg.mozilla.org/integration/mozilla-inbound/rev/edd03f7a77b7 (part 3)
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a30e45e50e2 (canvas test)
Target Milestone: --- → mozilla19
(Assignee)

Updated

5 years ago
Blocks: 804522
(Assignee)

Comment 43

5 years ago
Filed bug 804522 regarding the <canvas> emoji failure on tryserver OS X 10.7 test runs.
https://hg.mozilla.org/mozilla-central/rev/632afb0c06da
https://hg.mozilla.org/mozilla-central/rev/6b6cac8af784
https://hg.mozilla.org/mozilla-central/rev/74f2eb49f209
https://hg.mozilla.org/mozilla-central/rev/f6988e710b96
https://hg.mozilla.org/mozilla-central/rev/ce1cd7478dfe
https://hg.mozilla.org/mozilla-central/rev/edd03f7a77b7
https://hg.mozilla.org/mozilla-central/rev/3a30e45e50e2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Depends on: 805702
(Reporter)

Updated

5 years ago
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.
(Assignee)

Updated

5 years ago
Depends on: 804934

Updated

2 years ago
Blocks: 1153460
You need to log in before you can comment on or make changes to this bug.