[Webkit-unassigned] [Bug 183155] [FreeType] Color emojis in WebKitGTK+ for great justice

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=183155

--- Comment #15 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #12)
> Comment on attachment 334735 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334735&action=review
> 
> I still don't understand why ignoring the scale is correct, but since this
> doesn't break anything, let's do it.
> 
> How hard is it to add a test for this? You could just have an HTML page that
> displays a single character and save the expected result as an image, right?
> Then we could at least see that it is colored.

Again, the problem is not that emojis aren't colored, they are. The problem is that we are applying the wrong matrix and the scale makes the icons tiny, almost invisible. This only happens when the font is used as a system fallback, so a possible test would be to use Emoji One explicitly in the reference and the default font in the test for the same character. Both should render the same emoji at the same scale. I wrote this test indeed, but it didn't work because emojis are not positioned at the same place in both cases. This is because of the font line, used to position the characters in a line. In the case of using Emoji One explicitly the font line is Emoji One, so the metrics used for the line are the Emoji One. But then Emoji One is used as a fallback DejaVu or Liberation are the font line and the metrics used are not the Emoji One. At first I thought it was a WebKit bug, but then I noticed that Firefox and Chromium do exactly the same thing. I'll try to make a non-ref test, but I'm not sure if the render tree is different.

> > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:132
> >      FcResult fontConfigResult;
> > -    RefPtr<FcPattern> resultPattern = adoptRef(FcFontMatch(nullptr, pattern.get(), &fontConfigResult));
> > -    if (!resultPattern)
> > -        return nullptr;
> > -    FontPlatformData alternateFontData(resultPattern.get(), description);
> > -    return fontForPlatformData(alternateFontData);
> > +    if (RefPtr<FcPattern> resultPattern = adoptRef(FcFontMatch(nullptr, pattern.get(), &fontConfigResult))) {
> > +        FontPlatformData alternateFontData(resultPattern.get(), description);
> > +        return fontForPlatformData(alternateFontData);
> > +    }
> 
> I don't think this change is unrelated. It used to be live code, and now
> you've turned it into dead code, right? But very subtle dead code that most
> developers would not notice. So I would address bug #183210 in this same
> patch. Up to you, as long as you follow up promptly so that we don't forget
> about this.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180301/01f32071/attachment-0001.html>