HOST-Oman/libraqm

Spacing of color emojis is much too large

Closed this issue · 5 comments

Consider the following example

#include "cairo.h"
#include "cairo-ft.h"
#include "raqm.h"

int main()
{
    char const* text = "\xf0\x9f\x98\x80\xf0\x9f\x98\x80";
    // Adjust paths accordingly.
#ifdef COLOR
    char const* path = "/usr/share/fonts/google-noto-emoji/NotoColorEmoji.ttf";
#else
    char const* path = "/usr/share/fonts/google-noto-emoji/NotoEmoji-Regular.ttf";
#endif

    FT_Library library = NULL;
    if (FT_Init_FreeType(&library)) { return 1; }
    FT_Face face = NULL;
    if (FT_New_Face(library, path, 0, &face)) { return 2; }

    cairo_surface_t* cs;
    cairo_t* cr;
    cairo_font_face_t* ct;
    cs = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, 400, 400);
    cr = cairo_create(cs);
    ct = cairo_ft_font_face_create_for_ft_face(face, 0);
    cairo_set_font_face(cr, ct);
    cairo_set_font_size(cr, 20);
    cairo_set_source_rgb(cr, 0, 0, 0);

    {
        cairo_translate(cr, 0, 100);
        cairo_move_to(cr, 0, 0);
        cairo_show_text(cr, text);
    }
    {
        cairo_scaled_font_t* sf = cairo_get_scaled_font(cr);
        FT_Face face1 = cairo_ft_scaled_font_lock_face(sf);

        cairo_glyph_t* cglyphs = NULL;
        int count;

        cairo_scaled_font_text_to_glyphs(
                sf, 0, 0, text, strlen(text), &cglyphs, &count, NULL, NULL, NULL);

        cairo_translate(cr, 0, 100);
        cairo_move_to(cr, 0, 0);
        cairo_show_glyphs(cr, cglyphs, count);
        cairo_glyph_free(cglyphs);

        cairo_ft_scaled_font_unlock_face(sf);
    }
    {
        cairo_scaled_font_t* sf = cairo_get_scaled_font(cr);
        FT_Face face1 = cairo_ft_scaled_font_lock_face(sf);

        cairo_glyph_t* cglyphs = NULL;
        size_t count;

        raqm_t* rq = raqm_create();
        raqm_set_text_utf8(rq, text, strlen(text));
        raqm_set_freetype_face(rq, face1);
        raqm_set_par_direction(rq, RAQM_DIRECTION_DEFAULT);
        raqm_set_language(rq, "en", 0, strlen(text));
        raqm_layout(rq);
        raqm_glyph_t* rglyphs = raqm_get_glyphs(rq, &count);
        cglyphs = cairo_glyph_allocate(count);
        double x = 0, y = 0;
        for (size_t i = 0; i < count; ++i) {
            cglyphs[i].index = rglyphs[i].index;
            cglyphs[i].x = x + rglyphs[i].x_offset / 64.;
            cglyphs[i].y = y + rglyphs[i].y_offset / 64.;
            x += rglyphs[i].x_advance / 64.;
            y += rglyphs[i].y_advance / 64.;
        }

        cairo_translate(cr, 0, 100);
        cairo_move_to(cr, 0, 0);
        cairo_show_glyphs(cr, cglyphs, count);
        cairo_glyph_free(cglyphs);

        cairo_ft_scaled_font_unlock_face(sf);
    }

    cairo_surface_write_to_png(cs, "test.png");
    return 0;
}

which can be compiled with cc -o test test.c $(pkg-config --libs --cflags cairo raqm) with or without -DCOLOR.

We draw two grinning face emojis, first using cairo_show_text, then cairo_scaled_font_text_to_glyphs + cairo_show_glyphs, then raqm + cairo_show_glyphs.

Black and white emojis are rendered fine:
bw
Color emojis are spaced much too far away by raqm:
color

Raqm uses the glyph advances returned by FreeType (through HarfBuzz), and IIRC for scalable bitmap fonts like Noto Color Emoji you need to set pixel size to an available bitmap strike and scale the result to the desired size (which I think what Cairo is doing), but Raqm wouldn’t know anything about this.

I think if you try FT_Get_Advance() you would get a similarly large advance. If this is the case, you will need to figure how Cairo does the scaling and do the same.

I opened the same issue on the harfbuzz tracker (harfbuzz/harfbuzz#2428) given that I guess there isn't much to be done about it on raqm's side(?). Feel free to close this, and thanks for your help.

We should use the new FT_Get_Transform() (see harfbuzz/harfbuzz#2428) to fix this issue. We can’t depend on the new FreeType version yet, but I’m open to doing this conditionally based on the presence of this function (see our checks for HarfBuzz functions):

libraqm/configure.ac

Lines 47 to 55 in d978961

_save_libs="$LIBS"
_save_cflags="$CFLAGS"
LIBS="$LIBS $HARFBUZZ_LIBS $FRIBIDI_LIBS"
CFLAGS="$CFLAGS $HARFBUZZ_CFLAGS $FRIBIDI_CFLAGS"
AC_CHECK_FUNCS(hb_buffer_set_invisible_glyph)
AC_CHECK_DECLS([HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES],,,[[#include <hb.h>]])
LIBS="$_save_libs"
CFLAGS="$_save_cflags"

libraqm/src/raqm.c

Lines 1576 to 1579 in d978961

#ifdef HAVE_HB_BUFFER_SET_INVISIBLE_GLYPH
if (rq->invisible_glyph > 0)
hb_buffer_set_invisible_glyph (run->buffer, rq->invisible_glyph);
#endif

From a quick try, it seems that the patch would start with something like

diff --git i/configure.ac w/configure.ac
index b31e9f7..0c4a3b4 100644
--- i/configure.ac
+++ w/configure.ac
@@ -48,7 +48,7 @@ _save_libs="$LIBS"
 _save_cflags="$CFLAGS"
 LIBS="$LIBS $HARFBUZZ_LIBS $FRIBIDI_LIBS"
 CFLAGS="$CFLAGS $HARFBUZZ_CFLAGS $FRIBIDI_CFLAGS"
-AC_CHECK_FUNCS(hb_buffer_set_invisible_glyph)
+AC_CHECK_FUNCS([FT_Get_Transform hb_buffer_set_invisible_glyph])
 AC_CHECK_DECLS([HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES],,,[[#include <hb.h>]])
 
 LIBS="$_save_libs"
diff --git i/src/raqm.c w/src/raqm.c
index ae180b5..77a40d4 100644
--- i/src/raqm.c
+++ w/src/raqm.c
@@ -906,6 +906,7 @@ raqm_get_glyphs (raqm_t *rq,
     size_t len;
     hb_glyph_info_t *info;
     hb_glyph_position_t *position;
+    FT_Matrix ft_matrix = {1 << 16, 0, 0, 1 << 16};
 
     len = hb_buffer_get_length (run->buffer);
     info = hb_buffer_get_glyph_infos (run->buffer, NULL);
@@ -913,13 +914,25 @@ raqm_get_glyphs (raqm_t *rq,
 
     for (size_t i = 0; i < len; i++)
     {
+      FT_Face ftface;
+      FT_Vector ft_vector;
+      ftface = rq->text_info[info[i].cluster].ftface;
+#ifdef HAVE_FT_GET_TRANSFORM
+      FT_Get_Transform (ftface, &ft_matrix, NULL);
+#endif
       rq->glyphs[count + i].index = info[i].codepoint;
       rq->glyphs[count + i].cluster = info[i].cluster;
-      rq->glyphs[count + i].x_advance = position[i].x_advance;
-      rq->glyphs[count + i].y_advance = position[i].y_advance;
-      rq->glyphs[count + i].x_offset = position[i].x_offset;
-      rq->glyphs[count + i].y_offset = position[i].y_offset;
-      rq->glyphs[count + i].ftface = rq->text_info[info[i].cluster].ftface;
+      ft_vector.x = position[i].x_advance;
+      ft_vector.y = position[i].y_advance;
+      FT_Vector_Transform(&ft_vector, &ft_matrix);
+      rq->glyphs[count + i].x_advance = ft_vector.x;
+      rq->glyphs[count + i].y_advance = ft_vector.y;
+      ft_vector.x = position[i].x_offset;
+      ft_vector.y = position[i].y_offset;
+      FT_Vector_Transform(&ft_vector, &ft_matrix);
+      rq->glyphs[count + i].x_offset = ft_vector.x;
+      rq->glyphs[count + i].y_offset = ft_vector.y;
+      rq->glyphs[count + i].ftface = ftface;
 
       RAQM_TEST ("glyph [%d]\tx_offset: %d\ty_offset: %d\tx_advance: %d\tfont: %s\n",
           rq->glyphs[count + i].index, rq->glyphs[count + i].x_offset,

but 1) my autotools-fu is very weak, so I actually had to add some manual declarations (not shown here) instead to make things compile, and 2) similar fixes seem needed in raqm_index_to_position and raqm_position_to_index, but they seem a bit more unwieldy...

Thanks!