[PATCH] Fix emoji font support

[mawww at kakoune.org]

On 2020-02-11 04:37, Marc Lehmann wrote:
> The reason why we go directly for freetype metrics as because xft often 
> gives
> wrong metrics, which is disastrous for rxvt-unicode, which absolutely 
> needs
> to know if characters overlap and how tall they really are, for 
> example.
> 
> That means that simply using xft metrics is not an option, as these are 
> often
> simply wrong for normal fonts, and normal font rendering cetrainly has
> priority over emoji bitmap fonts.

Looking at the code, if there is no transforms applied, it looks like 
the only
difference between rxvt-unicode and libXft computation is the rounding, 
which I
agree can be an issue.

>> > So it only affects bitmap fonts? Any specific font that exhibits these
>> > issues?
>> 
>> The intention is to enable Noto Color Emoji.
> 
> Well, why doesn't it already work, for example? I still have no idea 
> whatt
> he problem is that the patch tries to fix. Does urxvt cut off 
> characters,
> or what happens?

If we use the emoji font first in the font list, then urxvt computes the 
metrics
pre-scaling, so ends up with a 124x124 font metric when the rendered 
glyphs are
actually say 12x12. This leads to huge spacing between lines.

Without the emoji inside the extent_test_chars, the emoji font is 
ignored because
we cannot compute its width (as none of the characters we try exists in 
the font).

>> In any case, the rest of the patch (everything but the hunk at line 
>> 1239)
>> is IMHO improving the correctness of rxvt-unicode by making it use 
>> text_t
>> for width computation, respecting the UNICODE_3 setting. I think this 
>> could
>> be applied independently.
> 
> It does increase code size and I don't see what would be incorrect with
> the old code - could you elaborate what you perceive as incorrect in 
> the
> existing code?

AFAIK the only increase in code size is in static storage for 
extent_test_chars,
which takes 21 * 4 = 84 bytes instead of 20 * 2 = 40 bytes. It is not 
something
I would worry about (especially as this only takes place *if* 
rxvt-unicode is
compiled with UNICODE_3 support).

I believe it is more "correct" because it removes a potential source of 
error
by making extent_test_chars match the codepoint type that rxvt-unicode 
uses,
instead of having a special case that extent_test_chars only support 
16bit
codepoints even when rxvt-unicode is compiled with support for 32bit 
codepoints.
Also, the addition of the `#if UNICODE_3` line makes it pretty clear for 
future
modifications that where to put eventual additional 32bit codepoints.

Cheers,

Maxime.