From d50dbbaf278c3dd9558a097124147e353a98c32a Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 17 Sep 2017 10:16:48 -0400 Subject: [PATCH 01/25] Make _intern_string_hash safe for "" The loop was unnecessarily written in a way that fails to terminate if len is 0 (ie for the empty string). Avoid that by checking for len > 0 explicitly. --- src/cairo-misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cairo-misc.c b/src/cairo-misc.c index 07ffa9ffb..4c516edce 100644 --- a/src/cairo-misc.c +++ b/src/cairo-misc.c @@ -986,7 +986,7 @@ _intern_string_hash (const char *str, int len) const signed char *p = (const signed char *) str; unsigned int h = *p; - for (p += 1; --len; p++) + for (p += 1; len > 0; len--, p++) h = (h << 5) - h + *p; return h; From 4ece386149ccd8fbad5f8dff5d792ad2b49d5915 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 17 Sep 2017 10:20:06 -0400 Subject: [PATCH 02/25] Make _intern_string_hash non-static We will use this function in cairo-font-options.c in the following commits. --- src/cairo-misc.c | 6 +++--- src/cairoint.h | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cairo-misc.c b/src/cairo-misc.c index 4c516edce..1ecbabf33 100644 --- a/src/cairo-misc.c +++ b/src/cairo-misc.c @@ -980,8 +980,8 @@ typedef struct _cairo_intern_string { static cairo_hash_table_t *_cairo_intern_string_ht; -static unsigned long -_intern_string_hash (const char *str, int len) +unsigned long +_cairo_string_hash (const char *str, int len) { const signed char *p = (const signed char *) str; unsigned int h = *p; @@ -1016,7 +1016,7 @@ _cairo_intern_string (const char **str_inout, int len) if (len < 0) len = strlen (str); - tmpl.hash_entry.hash = _intern_string_hash (str, len); + tmpl.hash_entry.hash = _cairo_string_hash (str, len); tmpl.len = len; tmpl.string = (char *) str; diff --git a/src/cairoint.h b/src/cairoint.h index 154270656..a51d2f937 100644 --- a/src/cairoint.h +++ b/src/cairoint.h @@ -912,6 +912,9 @@ _cairo_validate_text_clusters (const char *utf8, int num_clusters, cairo_text_cluster_flags_t cluster_flags); +cairo_private unsigned long +_cairo_string_hash (const char *str, int len); + cairo_private cairo_status_t _cairo_intern_string (const char **str_inout, int len); From edf9497c3a28bc4e110ecf08069c8d48b995a0ea Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 17 Sep 2017 10:40:13 -0400 Subject: [PATCH 03/25] Add font variations to font options Add a font option for OpenType font variations, specified as a string in a format similar to the proposed css font-variation-settings property. --- src/cairo-font-options.c | 87 +++++++++++++++++++++++++++++++++++++-- src/cairo-types-private.h | 1 + src/cairo.h | 7 ++++ 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/src/cairo-font-options.c b/src/cairo-font-options.c index ad28745e6..ea703423b 100644 --- a/src/cairo-font-options.c +++ b/src/cairo-font-options.c @@ -55,7 +55,8 @@ static const cairo_font_options_t _cairo_font_options_nil = { CAIRO_LCD_FILTER_DEFAULT, CAIRO_HINT_STYLE_DEFAULT, CAIRO_HINT_METRICS_DEFAULT, - CAIRO_ROUND_GLYPH_POS_DEFAULT + CAIRO_ROUND_GLYPH_POS_DEFAULT, + NULL }; /** @@ -73,6 +74,7 @@ _cairo_font_options_init_default (cairo_font_options_t *options) options->hint_style = CAIRO_HINT_STYLE_DEFAULT; options->hint_metrics = CAIRO_HINT_METRICS_DEFAULT; options->round_glyph_positions = CAIRO_ROUND_GLYPH_POS_DEFAULT; + options->variations = NULL; } void @@ -85,6 +87,7 @@ _cairo_font_options_init_copy (cairo_font_options_t *options, options->hint_style = other->hint_style; options->hint_metrics = other->hint_metrics; options->round_glyph_positions = other->round_glyph_positions; + options->variations = other->variations ? strdup (other->variations) : NULL; } /** @@ -166,6 +169,7 @@ cairo_font_options_destroy (cairo_font_options_t *options) if (cairo_font_options_status (options)) return; + free (options->variations); free (options); } @@ -226,6 +230,24 @@ cairo_font_options_merge (cairo_font_options_t *options, options->hint_metrics = other->hint_metrics; if (other->round_glyph_positions != CAIRO_ROUND_GLYPH_POS_DEFAULT) options->round_glyph_positions = other->round_glyph_positions; + + if (other->variations) { + if (options->variations) { + char *p; + + /* 'merge' variations by concatenating - later entries win */ + p = malloc (strlen (other->variations) + strlen (options->variations) + 2); + p[0] = 0; + strcat (p, options->variations); + strcat (p, ","); + strcat (p, other->variations); + free (options->variations); + options->variations = p; + } + else { + options->variations = strdup (other->variations); + } + } } slim_hidden_def (cairo_font_options_merge); @@ -259,7 +281,10 @@ cairo_font_options_equal (const cairo_font_options_t *options, options->lcd_filter == other->lcd_filter && options->hint_style == other->hint_style && options->hint_metrics == other->hint_metrics && - options->round_glyph_positions == other->round_glyph_positions); + options->round_glyph_positions == other->round_glyph_positions && + ((options->variations == NULL && other->variations == NULL) || + (options->variations != NULL && other->variations != NULL && + strcmp (options->variations, other->variations) == 0))); } slim_hidden_def (cairo_font_options_equal); @@ -280,14 +305,19 @@ slim_hidden_def (cairo_font_options_equal); unsigned long cairo_font_options_hash (const cairo_font_options_t *options) { + unsigned long hash = 0; + if (cairo_font_options_status ((cairo_font_options_t *) options)) options = &_cairo_font_options_nil; /* force default values */ + if (options->variations) + hash = _cairo_string_hash (options->variations, strlen (options->variations)); + return ((options->antialias) | (options->subpixel_order << 4) | (options->lcd_filter << 8) | (options->hint_style << 12) | - (options->hint_metrics << 16)); + (options->hint_metrics << 16)) ^ hash; } slim_hidden_def (cairo_font_options_hash); @@ -533,3 +563,54 @@ cairo_font_options_get_hint_metrics (const cairo_font_options_t *options) return options->hint_metrics; } + +/** + * cairo_font_options_set_variations: + * @options: a #cairo_font_options_t + * @variations: the new font variations, or %NULL + * + * Sets the OpenType font variations for the font options object. + * Font variations are specified as a string with a format that + * is similar to the CSS font-variation-settings. The string contains + * a comma-separated list of axis assignments, which each assignment + * consists of a 4-character axis name and a value, separated by + * whitespace and optional equals sign. + * + * Examples: + * + * wght=200,wdth=140.5 + * + * wght 200 , wdth 140.5 + * + * Since: 1.16 + **/ +void +cairo_font_options_set_variations (cairo_font_options_t *options, + const char *variations) +{ + char *tmp = variations ? strdup (variations) : NULL; + free (options->variations); + options->variations = tmp; +} + +/** + * cairo_font_options_get_variations: + * @options: a #cairo_font_options_t + * + * Gets the OpenType font variations for the font options object. + * See cairo_font_options_set_variations() for details about the + * string format. + * + * Return value: the font variations for the font options object. The + * returned string belongs to the @options and must not be modified. + * It is valid until either the font options object is destroyed or + * the font variations in this object is modified with + * cairo_font_options_set_variations(). + * + * Since: 1.16 + **/ +const char * +cairo_font_options_get_variations (cairo_font_options_t *options) +{ + return options->variations; +} diff --git a/src/cairo-types-private.h b/src/cairo-types-private.h index 3d15d968e..da373030c 100644 --- a/src/cairo-types-private.h +++ b/src/cairo-types-private.h @@ -194,6 +194,7 @@ struct _cairo_font_options { cairo_hint_style_t hint_style; cairo_hint_metrics_t hint_metrics; cairo_round_glyph_positions_t round_glyph_positions; + char *variations; }; struct _cairo_glyph_text_info { diff --git a/src/cairo.h b/src/cairo.h index e48157252..c252c7141 100644 --- a/src/cairo.h +++ b/src/cairo.h @@ -1430,6 +1430,13 @@ cairo_font_options_set_hint_metrics (cairo_font_options_t *options, cairo_public cairo_hint_metrics_t cairo_font_options_get_hint_metrics (const cairo_font_options_t *options); +cairo_public const char * +cairo_font_options_get_variations (cairo_font_options_t *options); + +cairo_public void +cairo_font_options_set_variations (cairo_font_options_t *options, + const char *variations); + /* This interface is for dealing with text as text, not caring about the font object inside the the cairo_t. */ From ac5acc45383f7ada422dec3e7c8a71bdb041fc8a Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 17 Sep 2017 10:54:04 -0400 Subject: [PATCH 04/25] Load font variations from fontconfig too When extracting font options from a fontconfig pattern, also extract font variation information if present. --- src/cairo-ft-font.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 7f23eb787..40bc57bed 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -1714,6 +1714,7 @@ _get_pattern_ft_options (FcPattern *pattern, cairo_ft_options_t *ret) #ifdef FC_HINT_STYLE int hintstyle; #endif + char *variations; _cairo_font_options_init_default (&ft_options.base); ft_options.load_flags = FT_LOAD_DEFAULT; @@ -1855,6 +1856,13 @@ _get_pattern_ft_options (FcPattern *pattern, cairo_ft_options_t *ret) if (embolden) ft_options.synth_flags |= CAIRO_FT_SYNTHESIZE_BOLD; +#ifndef FC_FONT_VARIATIONS +#define FC_FONT_VARIATIONS "fontvariations" +#endif + if (FcPatternGetString (pattern, FC_FONT_VARIATIONS, 0, (FcChar8 **) &variations) == FcResultMatch) { + ft_options.base.variations = strdup (variations); + } + *ret = ft_options; } #endif From a8ae2eafc85fda76fde79defa40fe06000d907db Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 17 Sep 2017 11:17:13 -0400 Subject: [PATCH 05/25] Use strtod_l when available Using strtod_l and newlocale is a nicer way to have provide a C-locale-only strtod. Since these APIs are not available everywhere, keep the old code as a fallback. --- configure.ac | 3 +++ src/cairo-misc.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/configure.ac b/configure.ac index d82db4578..6b4ffa22d 100644 --- a/configure.ac +++ b/configure.ac @@ -76,6 +76,9 @@ if test "x$have_dlsym" = "xyes"; then fi AM_CONDITIONAL(CAIRO_HAS_DLSYM, test "x$have_dlsym" = "xyes") +AC_CHECK_HEADERS(xlocale.h) +AC_CHECK_FUNCS(newlocale strtod_l) + dnl =========================================================================== CAIRO_ENABLE_SURFACE_BACKEND(xlib, Xlib, auto, [ diff --git a/src/cairo-misc.c b/src/cairo-misc.c index 1ecbabf33..cf0d6d160 100644 --- a/src/cairo-misc.c +++ b/src/cairo-misc.c @@ -43,6 +43,10 @@ #include #include +#include +#ifdef HAVE_XLOCALE_H +#include +#endif COMPILE_TIME_ASSERT ((int)CAIRO_STATUS_LAST_STATUS < (int)CAIRO_INT_STATUS_UNSUPPORTED); COMPILE_TIME_ASSERT (CAIRO_INT_STATUS_LAST_STATUS <= 127); @@ -789,6 +793,38 @@ _cairo_get_locale_decimal_point (void) } #endif +#if defined (HAVE_NEWLOCALE) && defined (HAVE_STRTOD_L) + +static locale_t C_locale; + +static locale_t +get_C_locale (void) +{ + locale_t C; + +retry: + C = (locale_t) _cairo_atomic_ptr_get (&C_locale); + + if (unlikely (!C)) { + C = newlocale (LC_ALL_MASK, "C", NULL); + + if (!_cairo_atomic_ptr_cmpxchg (&C_locale, NULL, C)) { + freelocale (C_locale); + goto retry; + } + } + + return C; +} + +double +_cairo_strtod (const char *nptr, char **endptr) +{ + return strtod_l (nptr, endptr, get_C_locale ()); +} + +#else + /* strtod replacement that ignores locale and only accepts decimal points */ double _cairo_strtod (const char *nptr, char **endptr) @@ -844,6 +880,7 @@ _cairo_strtod (const char *nptr, char **endptr) return value; } +#endif /** * _cairo_fopen: From 721b7ea0a785afaa04b6da63f970c3c57666fdfe Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 17 Sep 2017 10:54:58 -0400 Subject: [PATCH 06/25] Apply font variations when loading fonts Parse font variation settings and pass them on to freetype when loading fonts. --- src/cairo-ft-font.c | 69 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 40bc57bed..d6f9d1aef 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -3628,6 +3628,70 @@ cairo_ft_font_face_get_synthesize (cairo_font_face_t *font_face) return ft->ft_options.synth_flags; } +static void +cairo_ft_apply_variations (FT_Face face, + const char *variations) +{ + FT_MM_Var *ft_mm_var; + FT_Error ret; + + ret = FT_Get_MM_Var (face, &ft_mm_var); + if (ret == 0) { + FT_Fixed *coords; + unsigned int i; + const char *p; + + coords = malloc (sizeof (FT_Fixed) * ft_mm_var->num_axis); + + for (i = 0; i < ft_mm_var->num_axis; i++) + coords[i] = ft_mm_var->axis[i].def; + + p = variations; + while (p && *p) { + char *start; + char *end, *end2; + FT_ULong tag; + double value; + + while (_cairo_isspace (*p)) p++; + + start = p; + end = strchr (p, ','); + if (end && (end - p < 6)) + goto skip; + + tag = FT_MAKE_TAG(p[0], p[1], p[2], p[3]); + + p += 4; + while (_cairo_isspace (*p)) p++; + if (*p == '=') p++; + + if (p - start < 5) + goto skip; + + value = _cairo_strtod (p, &end2); + + while (end2 && _cairo_isspace (*end2)) end2++; + + if (end2 && (*end2 != ',' && *end2 != '\0')) + goto skip; + + for (i = 0; i < ft_mm_var->num_axis; i++) { + if (ft_mm_var->axis[i].tag == tag) { + coords[i] = (FT_Fixed)(value*65536); + break; + } + } + +skip: + p = end ? end + 1 : NULL; + } + + FT_Set_Var_Design_Coordinates (face, ft_mm_var->num_axis, coords); + free (coords); + } +} + /** * cairo_ft_scaled_font_lock_face: * @scaled_font: A #cairo_scaled_font_t from the FreeType font backend. Such an @@ -3636,7 +3700,8 @@ cairo_ft_font_face_get_synthesize (cairo_font_face_t *font_face) * cairo_ft_font_face_create_for_ft_face()). * * cairo_ft_scaled_font_lock_face() gets the #FT_Face object from a FreeType - * backend font and scales it appropriately for the font. You must + * backend font and scales it appropriately for the font and applies OpenType + * font variations if applicable. You must * release the face with cairo_ft_scaled_font_unlock_face() * when you are done using it. Since the #FT_Face object can be * shared between multiple #cairo_scaled_font_t objects, you must not @@ -3689,6 +3754,8 @@ cairo_ft_scaled_font_lock_face (cairo_scaled_font_t *abstract_font) return NULL; } + cairo_ft_apply_variations (face, scaled_font->base.options.variations); + /* Note: We deliberately release the unscaled font's mutex here, * so that we are not holding a lock across two separate calls to * cairo function, (which would give the application some From 38b6e23609085ecb736530e47ffd04564304415e Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 16 Sep 2017 13:22:22 -0400 Subject: [PATCH 07/25] Add a test for font variations This test checks that passing font variation settings via font options has the desired effect. It checks this by reading the effective axis values out of the FT_Face after creating a font with these options. --- test/Makefile.am | 1 + test/Makefile.sources | 1 + test/font-variations.c | 196 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 test/font-variations.c diff --git a/test/Makefile.am b/test/Makefile.am index f03c21b08..e3c42ea88 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -158,6 +158,7 @@ ps-features \ svg-clip \ svg-surface \ toy-font-face \ +font-variations \ user-data # A target to summarise the failures diff --git a/test/Makefile.sources b/test/Makefile.sources index a3134d51f..2f53c572b 100644 --- a/test/Makefile.sources +++ b/test/Makefile.sources @@ -143,6 +143,7 @@ test_sources = \ font-face-get-type.c \ font-matrix-translation.c \ font-options.c \ + font-variations.c \ glyph-cache-pressure.c \ get-and-set.c \ get-clip.c \ diff --git a/test/font-variations.c b/test/font-variations.c new file mode 100644 index 000000000..18821726c --- /dev/null +++ b/test/font-variations.c @@ -0,0 +1,196 @@ +/* + * Copyright © 2017 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software + * and its documentation for any purpose is hereby granted without + * fee, provided that the above copyright notice appear in all copies + * and that both that copyright notice and this permission notice + * appear in supporting documentation, and that the name of + * Red Hat, Inc. not be used in advertising or publicity pertaining to + * distribution of the software without specific, written prior + * permission. Red Hat, Inc. makes no representations about the + * suitability of this software for any purpose. It is provided "as + * is" without express or implied warranty. + * + * RED HAT, INC. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL RED HAT, INC. BE LIABLE FOR ANY SPECIAL, + * INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR + * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + * Author: MAtthias Clasen + */ + +#include "cairo-test.h" + +#include + +#if CAIRO_HAS_FC_FONT +#include +#include +#include "cairo-ft.h" +#endif + +#define FloatToFixed(f) ((FT_Fixed)((f)*65536)) + +static cairo_test_status_t +test_variation (cairo_test_context_t *ctx, + const char *input, + const char *tag, + int def, + float expected_value) +{ + cairo_font_face_t *font_face; + cairo_scaled_font_t *scaled_font; + cairo_matrix_t matrix; + cairo_font_options_t *options; + cairo_status_t status; + + FT_Face ft_face; + FT_MM_Var *ft_mm_var; + FT_Error ret; + FT_Fixed coords[20]; + unsigned int i; + +#if CAIRO_HAS_FC_FONT + FcPattern *pattern; + + /* we need a font that has variations */ + pattern = FcPatternBuild (NULL, + FC_FAMILY, FcTypeString, (FcChar8*)"Adobe Variable Font Prototype", + NULL); + font_face = cairo_ft_font_face_create_for_pattern (pattern); + status = cairo_font_face_status (font_face); + FcPatternDestroy (pattern); + + if (status != CAIRO_STATUS_SUCCESS) { + cairo_test_log (ctx, "Failed to create font face"); + return CAIRO_TEST_FAILURE; + } + + cairo_matrix_init_identity (&matrix); + options = cairo_font_options_create (); + if (cairo_font_options_status (options) != CAIRO_STATUS_SUCCESS) { + cairo_test_log (ctx, "Failed to create font options"); + return CAIRO_TEST_FAILURE; + } + + cairo_font_options_set_variations (options, "wdth=200,wght=300"); + if (cairo_font_options_status (options) != CAIRO_STATUS_SUCCESS) { + cairo_test_log (ctx, "Failed to set variations"); + return CAIRO_TEST_FAILURE; + } + + if (strcmp (cairo_font_options_get_variations (options), "wdth=200,wght=300") != 0) { + cairo_test_log (ctx, "Failed to verify variations"); + return CAIRO_TEST_FAILURE; + } + + scaled_font = cairo_scaled_font_create (font_face, &matrix, &matrix, options); + status = cairo_scaled_font_status (scaled_font); + + if (status != CAIRO_STATUS_SUCCESS) { + cairo_test_log (ctx, "Failed to create scaled font"); + return CAIRO_TEST_FAILURE; + } + + ft_face = cairo_ft_scaled_font_lock_face (scaled_font); + if (cairo_scaled_font_status (scaled_font) != CAIRO_STATUS_SUCCESS) { + cairo_test_log (ctx, "Failed to get FT_Face"); + return CAIRO_TEST_FAILURE; + } + if (strcmp (ft_face->family_name, "Adobe Variable Font Prototype") != 0) { + cairo_test_log (ctx, "This test requires the font \"Adobe Variable Font Prototype\" (https://github.com/adobe-fonts/adobe-variable-font-prototype/releases)"); + return CAIRO_TEST_FAILURE; + } + + ret = FT_Get_MM_Var (ft_face, &ft_mm_var); + if (ret != 0) { + cairo_test_log (ctx, "Failed to get MM"); + return CAIRO_TEST_FAILURE; + } + + ret = FT_Get_Var_Design_Coordinates (ft_face, 20, coords); + if (ret != 0) { + cairo_test_log (ctx, "Failed to get coords"); + return CAIRO_TEST_FAILURE; + } + + for (i = 0; i < ft_mm_var->num_axis; i++) { + FT_Var_Axis *axis = &ft_mm_var->axis[i]; + cairo_test_log (ctx, "axis %s, value %ld\n", axis->name, coords[i]); + } + for (i = 0; i < ft_mm_var->num_axis; i++) { + FT_Var_Axis *axis = &ft_mm_var->axis[i]; + if (axis->tag == FT_MAKE_TAG(tag[0], tag[1], tag[2], tag[3])) { + if (def) { + if (coords[i] != axis->def) { + cairo_test_log (ctx, "Axis %s: not default value (%ld != %ld)", + axis->name, coords[i], axis->def); + return CAIRO_TEST_FAILURE; + } + } + else { + if (coords[i] != FloatToFixed(expected_value)) { + cairo_test_log (ctx, "Axis %s: not expected value (%ld != %ld)", + axis->name, coords[i], FloatToFixed(expected_value)); + return CAIRO_TEST_FAILURE; + } + } + } + else { + } + } + + cairo_ft_scaled_font_unlock_face (scaled_font); + + cairo_scaled_font_destroy (scaled_font); + cairo_font_options_destroy (options); + cairo_font_face_destroy (font_face); + + return CAIRO_TEST_SUCCESS; +#else + return CAIRO_TEST_UNTESTED; +#endif +} + +static cairo_test_status_t +preamble (cairo_test_context_t *ctx) +{ + cairo_test_status_t status = CAIRO_TEST_SUCCESS; + struct { const char *input; + const char *tag; + int expected_default; + float expected_value; + } tests[] = { + { "wdth=200,wght=300", "wght", 0, 200.0 }, // valid + { "wdth=200.5,wght=300", "wght", 0, 200.5 }, // valid, using decimal dot + { "wdth 200 , wght=300", "wght", 0, 200.0 }, // valid, without = + { "wdth = 200", "wght", 0, 200.0 }, // valid, whitespace and = + { "CNTR=20", "wght", 1, 0.0 }, // valid, not setting wght + { "weight=100", "wght", 1, 0.0 }, // not a valid tag + { NULL, 0 } + }; + int i; + + for (i = 0; tests[i].input; i++) { + status = test_variation (ctx, + tests[i].input, + tests[i].tag, + tests[i].expected_default, + tests[i].expected_value); + if (status != CAIRO_TEST_SUCCESS) + return status; + } + + return CAIRO_TEST_SUCCESS; +} + +CAIRO_TEST (font_variations, + "Test font variations", + "fonts", /* keywords */ + NULL, /* requirements */ + 9, 11, + preamble, NULL) From 7cc07f184bf705891bfb6b7f3b4f4b425370b5cd Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 16 Sep 2017 13:23:57 -0400 Subject: [PATCH 08/25] Work around a freetype bug The first call to FT_Get_MM_Var on a newly created FT_Face fails. So just repeat the call. --- src/cairo-ft-font.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index d6f9d1aef..9eeb80ba6 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -3636,6 +3636,8 @@ cairo_ft_apply_variations (FT_Face face, FT_Error ret; ret = FT_Get_MM_Var (face, &ft_mm_var); + if (ret != 0) /* FIXME: the first FT_Get_MM_Var call on an FT_Face fails, so try again */ + ret = FT_Get_MM_Var (face, &ft_mm_var); if (ret == 0) { FT_Fixed *coords; unsigned int i; From 6accf16093b3d3451eca10ee194c00a1107b8861 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 20 Sep 2017 14:53:10 -0700 Subject: [PATCH 09/25] [variations] Towards fixing test --- test/font-variations.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/test/font-variations.c b/test/font-variations.c index 18821726c..47dcd3706 100644 --- a/test/font-variations.c +++ b/test/font-variations.c @@ -28,7 +28,9 @@ #include #if CAIRO_HAS_FC_FONT -#include +#include +#include FT_FREETYPE_H +#include FT_MULTIPLE_MASTERS_H #include #include "cairo-ft.h" #endif @@ -57,9 +59,14 @@ test_variation (cairo_test_context_t *ctx, #if CAIRO_HAS_FC_FONT FcPattern *pattern; +#ifndef FC_FONT_VARIATIONS +#define FC_FONT_VARIATIONS "fontvariations" +#endif + /* we need a font that has variations */ pattern = FcPatternBuild (NULL, FC_FAMILY, FcTypeString, (FcChar8*)"Adobe Variable Font Prototype", + FC_FONT_VARIATIONS, FcTypeString, input, NULL); font_face = cairo_ft_font_face_create_for_pattern (pattern); status = cairo_font_face_status (font_face); @@ -120,22 +127,22 @@ test_variation (cairo_test_context_t *ctx, for (i = 0; i < ft_mm_var->num_axis; i++) { FT_Var_Axis *axis = &ft_mm_var->axis[i]; - cairo_test_log (ctx, "axis %s, value %ld\n", axis->name, coords[i]); + cairo_test_log (ctx, "axis %s, value %g\n", axis->name, coords[i] / 65536.); } for (i = 0; i < ft_mm_var->num_axis; i++) { FT_Var_Axis *axis = &ft_mm_var->axis[i]; if (axis->tag == FT_MAKE_TAG(tag[0], tag[1], tag[2], tag[3])) { if (def) { if (coords[i] != axis->def) { - cairo_test_log (ctx, "Axis %s: not default value (%ld != %ld)", - axis->name, coords[i], axis->def); + cairo_test_log (ctx, "Axis %s: not default value (%g != %g)", + axis->name, coords[i] / 65536., axis->def / 65536.); return CAIRO_TEST_FAILURE; } } else { if (coords[i] != FloatToFixed(expected_value)) { - cairo_test_log (ctx, "Axis %s: not expected value (%ld != %ld)", - axis->name, coords[i], FloatToFixed(expected_value)); + cairo_test_log (ctx, "Axis %s: not expected value (%g != %g)", + axis->name, coords[i] / 65536., expected_value); return CAIRO_TEST_FAILURE; } } From 34047d11db847777d32b5eb49dfcb64bc08e33d0 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 20 Sep 2017 15:11:27 -0700 Subject: [PATCH 10/25] [variations] Fix test This does not exercise merging of variations from font-options and from pattern. Before this commit the code was more towards doing that. --- test/font-variations.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/test/font-variations.c b/test/font-variations.c index 47dcd3706..36724f05b 100644 --- a/test/font-variations.c +++ b/test/font-variations.c @@ -59,14 +59,9 @@ test_variation (cairo_test_context_t *ctx, #if CAIRO_HAS_FC_FONT FcPattern *pattern; -#ifndef FC_FONT_VARIATIONS -#define FC_FONT_VARIATIONS "fontvariations" -#endif - /* we need a font that has variations */ pattern = FcPatternBuild (NULL, FC_FAMILY, FcTypeString, (FcChar8*)"Adobe Variable Font Prototype", - FC_FONT_VARIATIONS, FcTypeString, input, NULL); font_face = cairo_ft_font_face_create_for_pattern (pattern); status = cairo_font_face_status (font_face); @@ -84,13 +79,13 @@ test_variation (cairo_test_context_t *ctx, return CAIRO_TEST_FAILURE; } - cairo_font_options_set_variations (options, "wdth=200,wght=300"); + cairo_font_options_set_variations (options, input); if (cairo_font_options_status (options) != CAIRO_STATUS_SUCCESS) { cairo_test_log (ctx, "Failed to set variations"); return CAIRO_TEST_FAILURE; } - if (strcmp (cairo_font_options_get_variations (options), "wdth=200,wght=300") != 0) { + if (strcmp (cairo_font_options_get_variations (options), input) != 0) { cairo_test_log (ctx, "Failed to verify variations"); return CAIRO_TEST_FAILURE; } @@ -172,10 +167,10 @@ preamble (cairo_test_context_t *ctx) int expected_default; float expected_value; } tests[] = { - { "wdth=200,wght=300", "wght", 0, 200.0 }, // valid - { "wdth=200.5,wght=300", "wght", 0, 200.5 }, // valid, using decimal dot - { "wdth 200 , wght=300", "wght", 0, 200.0 }, // valid, without = - { "wdth = 200", "wght", 0, 200.0 }, // valid, whitespace and = + { "wdth=200,wght=300", "wdth", 0, 200.0 }, // valid + { "wdth=200.5,wght=300", "wdth", 0, 200.5 }, // valid, using decimal dot + { "wdth 200 , wght=300", "wdth", 0, 200.0 }, // valid, without = + { "wdth = 200", "wdth", 0, 200.0 }, // valid, whitespace and = { "CNTR=20", "wght", 1, 0.0 }, // valid, not setting wght { "weight=100", "wght", 1, 0.0 }, // not a valid tag { NULL, 0 } From 374ee5a2fd1f749de8d52a2536dc8fb23ef17121 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 20 Sep 2017 15:14:54 -0700 Subject: [PATCH 11/25] [variations] Merge variations in cairo-ft font option merging This function should be rewritten to call cairo_font_options_merge(), but currently it doesn't. Speculative. Untested. --- src/cairo-ft-font.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 9eeb80ba6..b9c1aa683 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -1940,6 +1940,24 @@ _cairo_ft_options_merge (cairo_ft_options_t *options, } } + if (other->base.variations) { + if (options->base.variations) { + char *p; + + /* 'merge' variations by concatenating - later entries win */ + p = malloc (strlen (other->base.variations) + strlen (options->base.variations) + 2); + p[0] = 0; + strcat (p, options->base.variations); + strcat (p, ","); + strcat (p, other->base.variations); + free (options->base.variations); + options->base.variations = p; + } + else { + options->base.variations = strdup (other->base.variations); + } + } + options->load_flags = load_flags | load_target; options->synth_flags = other->synth_flags; } From 7125c4bf5e95164f4202a0f16fff07198e578668 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 20 Sep 2017 18:50:57 -0700 Subject: [PATCH 12/25] [varfonts] Use blend, not design, coordinates to check for non-base variation If all blend coordinates are zero, we are at base font. This is more robust. --- src/cairo-ft-font.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index b9c1aa683..44a78478a 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -2704,17 +2704,17 @@ _cairo_ft_is_synthetic (void *abstract_font, } #if FREETYPE_MAJOR > 2 || ( FREETYPE_MAJOR == 2 && FREETYPE_MINOR >= 8) - /* If FT_Get_Var_Design_Coordinates() is available, we can check if the + /* If FT_Get_Var_Blend_Coordinates() is available, we can check if the * current design coordinates are the default coordinates. In this case * the current outlines match the font tables. */ { int i; - FT_Get_Var_Design_Coordinates (face, num_axis, coords); + FT_Get_Var_Blend_Coordinates (face, num_axis, coords); *is_synthetic = FALSE; for (i = 0; i < num_axis; i++) { - if (coords[i] != mm_var->axis[i].def) { + if (coords[i]) { *is_synthetic = TRUE; break; } From db946d1788b8be1aef39102efe93826886b6addf Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 20 Sep 2017 18:51:36 -0700 Subject: [PATCH 13/25] [varfonts] Correctly (re)set variations of named instances --- src/cairo-ft-font.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 44a78478a..e95d37991 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -3660,11 +3660,19 @@ cairo_ft_apply_variations (FT_Face face, FT_Fixed *coords; unsigned int i; const char *p; + unsigned int instance_id = face->face_index >> 16; coords = malloc (sizeof (FT_Fixed) * ft_mm_var->num_axis); + /* FIXME check coords. */ - for (i = 0; i < ft_mm_var->num_axis; i++) - coords[i] = ft_mm_var->axis[i].def; + if (instance_id && instance_id <= ft_mm_var->num_namedstyles) + { + FT_Var_Named_Style *instance = &ft_mm_var->namedstyle[instance_id - 1]; + memcpy (coords, instance->coords, ft_mm_var->num_axis & sizeof (*coords)); + } + else + for (i = 0; i < ft_mm_var->num_axis; i++) + coords[i] = ft_mm_var->axis[i].def; p = variations; while (p && *p) { From 46034b0547ea9b69392bde5b334f1891bd51c98a Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 20 Sep 2017 22:29:30 -0400 Subject: [PATCH 14/25] Make the font-variations test pass Not sure what I was thinking - the test is checking the width axis, but the font we're using has only weight and contrast as axes... --- test/font-variations.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/font-variations.c b/test/font-variations.c index 36724f05b..68071d077 100644 --- a/test/font-variations.c +++ b/test/font-variations.c @@ -167,10 +167,10 @@ preamble (cairo_test_context_t *ctx) int expected_default; float expected_value; } tests[] = { - { "wdth=200,wght=300", "wdth", 0, 200.0 }, // valid - { "wdth=200.5,wght=300", "wdth", 0, 200.5 }, // valid, using decimal dot - { "wdth 200 , wght=300", "wdth", 0, 200.0 }, // valid, without = - { "wdth = 200", "wdth", 0, 200.0 }, // valid, whitespace and = + { "wdth=200,wght=300", "wght", 0, 300.0 }, // valid + { "wdth=200.5,wght=300.5", "wght", 0, 300.5 }, // valid, using decimal dot + { "wdth 200 , wght 300", "wght", 0, 300.0 }, // valid, without = + { "wght = 200", "wght", 0, 200.0 }, // valid, whitespace and = { "CNTR=20", "wght", 1, 0.0 }, // valid, not setting wght { "weight=100", "wght", 1, 0.0 }, // not a valid tag { NULL, 0 } From 1735fc41d3ac7015a584d73aa2c983f44f008788 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Thu, 28 Sep 2017 15:40:22 -0400 Subject: [PATCH 15/25] Apply font variation options consistently We must not look at the instance_id field of the cached ft_face, since freetype changes it underneath us (if we set the axis values to match a named instance, the instance_id gets changed to reflect that). Thankfully, we have the original instance_id stashed away in the cairo_unscaled_font, so we can just use that to decide if we are dealing with a named instance or not. This makes the font variations tests pass. --- src/cairo-ft-font.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index e95d37991..4b6c74ab1 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -3648,6 +3648,7 @@ cairo_ft_font_face_get_synthesize (cairo_font_face_t *font_face) static void cairo_ft_apply_variations (FT_Face face, + int instance_id, const char *variations) { FT_MM_Var *ft_mm_var; @@ -3660,7 +3661,6 @@ cairo_ft_apply_variations (FT_Face face, FT_Fixed *coords; unsigned int i; const char *p; - unsigned int instance_id = face->face_index >> 16; coords = malloc (sizeof (FT_Fixed) * ft_mm_var->num_axis); /* FIXME check coords. */ @@ -3668,7 +3668,7 @@ cairo_ft_apply_variations (FT_Face face, if (instance_id && instance_id <= ft_mm_var->num_namedstyles) { FT_Var_Named_Style *instance = &ft_mm_var->namedstyle[instance_id - 1]; - memcpy (coords, instance->coords, ft_mm_var->num_axis & sizeof (*coords)); + memcpy (coords, instance->coords, ft_mm_var->num_axis * sizeof (*coords)); } else for (i = 0; i < ft_mm_var->num_axis; i++) @@ -3782,7 +3782,7 @@ cairo_ft_scaled_font_lock_face (cairo_scaled_font_t *abstract_font) return NULL; } - cairo_ft_apply_variations (face, scaled_font->base.options.variations); + cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->base.options.variations); /* Note: We deliberately release the unscaled font's mutex here, * so that we are not holding a lock across two separate calls to From 42f07ef9037ea871dc0e58200437aab9c702ad6d Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Thu, 28 Sep 2017 16:24:13 -0400 Subject: [PATCH 16/25] Always save the origin face index Save the original face_index also for the from_face case, so we can always rely on it when applying font variations. This prevents us from running into the same trap we've seen before where ft_face->face_index changes underneath us as font variations are applied. --- src/cairo-ft-font.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 4b6c74ab1..ec1fb7c12 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -428,7 +428,7 @@ _cairo_ft_unscaled_font_init (cairo_ft_unscaled_font_t *unscaled, if (from_face) { unscaled->from_face = TRUE; - _cairo_ft_unscaled_font_init_key (unscaled, TRUE, NULL, 0, face); + _cairo_ft_unscaled_font_init_key (unscaled, TRUE, NULL, face->face_index, face); unscaled->have_color = FT_HAS_COLOR (face) != 0; unscaled->have_color_set = TRUE; @@ -487,7 +487,7 @@ _cairo_ft_unscaled_font_keys_equal (const void *key_a, if (unscaled_a->id == unscaled_b->id && unscaled_a->from_face == unscaled_b->from_face) - { + { if (unscaled_a->from_face) return unscaled_a->face == unscaled_b->face; @@ -3758,6 +3758,7 @@ cairo_ft_scaled_font_lock_face (cairo_scaled_font_t *abstract_font) { cairo_ft_scaled_font_t *scaled_font = (cairo_ft_scaled_font_t *) abstract_font; FT_Face face; + int instance_id; cairo_status_t status; if (! _cairo_scaled_font_is_ft (abstract_font)) { From f2b6fac43f2fcd362eced1c87c5760c763aa963f Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Mon, 18 Dec 2017 13:00:18 -0500 Subject: [PATCH 17/25] Trivial: code movement Move cairo_ft_apply_font_variations() earlier in the code since we will be using it in another place soon. --- src/cairo-ft-font.c | 148 ++++++++++++++++++++++---------------------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index ec1fb7c12..cc9779258 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -2260,6 +2260,80 @@ _cairo_ft_scaled_glyph_vertical_layout_bearing_fix (void *abstract_font, } } +static void +cairo_ft_apply_variations (FT_Face face, + int instance_id, + const char *variations) +{ + FT_MM_Var *ft_mm_var; + FT_Error ret; + + ret = FT_Get_MM_Var (face, &ft_mm_var); + if (ret != 0) /* FIXME: the first FT_Get_MM_Var call on an FT_Face fails, so try again */ + ret = FT_Get_MM_Var (face, &ft_mm_var); + if (ret == 0) { + FT_Fixed *coords; + unsigned int i; + const char *p; + + coords = malloc (sizeof (FT_Fixed) * ft_mm_var->num_axis); + /* FIXME check coords. */ + + if (instance_id && instance_id <= ft_mm_var->num_namedstyles) + { + FT_Var_Named_Style *instance = &ft_mm_var->namedstyle[instance_id - 1]; + memcpy (coords, instance->coords, ft_mm_var->num_axis * sizeof (*coords)); + } + else + for (i = 0; i < ft_mm_var->num_axis; i++) + coords[i] = ft_mm_var->axis[i].def; + + p = variations; + while (p && *p) { + char *start; + char *end, *end2; + FT_ULong tag; + double value; + + while (_cairo_isspace (*p)) p++; + + start = p; + end = strchr (p, ','); + if (end && (end - p < 6)) + goto skip; + + tag = FT_MAKE_TAG(p[0], p[1], p[2], p[3]); + + p += 4; + while (_cairo_isspace (*p)) p++; + if (*p == '=') p++; + + if (p - start < 5) + goto skip; + + value = _cairo_strtod (p, &end2); + + while (end2 && _cairo_isspace (*end2)) end2++; + + if (end2 && (*end2 != ',' && *end2 != '\0')) + goto skip; + + for (i = 0; i < ft_mm_var->num_axis; i++) { + if (ft_mm_var->axis[i].tag == tag) { + coords[i] = (FT_Fixed)(value*65536); + break; + } + } + +skip: + p = end ? end + 1 : NULL; + } + + FT_Set_Var_Design_Coordinates (face, ft_mm_var->num_axis, coords); + free (coords); + } +} + static cairo_int_status_t _cairo_ft_scaled_glyph_load_glyph (cairo_ft_scaled_font_t *scaled_font, cairo_scaled_glyph_t *scaled_glyph, @@ -3646,80 +3720,6 @@ cairo_ft_font_face_get_synthesize (cairo_font_face_t *font_face) return ft->ft_options.synth_flags; } -static void -cairo_ft_apply_variations (FT_Face face, - int instance_id, - const char *variations) -{ - FT_MM_Var *ft_mm_var; - FT_Error ret; - - ret = FT_Get_MM_Var (face, &ft_mm_var); - if (ret != 0) /* FIXME: the first FT_Get_MM_Var call on an FT_Face fails, so try again */ - ret = FT_Get_MM_Var (face, &ft_mm_var); - if (ret == 0) { - FT_Fixed *coords; - unsigned int i; - const char *p; - - coords = malloc (sizeof (FT_Fixed) * ft_mm_var->num_axis); - /* FIXME check coords. */ - - if (instance_id && instance_id <= ft_mm_var->num_namedstyles) - { - FT_Var_Named_Style *instance = &ft_mm_var->namedstyle[instance_id - 1]; - memcpy (coords, instance->coords, ft_mm_var->num_axis * sizeof (*coords)); - } - else - for (i = 0; i < ft_mm_var->num_axis; i++) - coords[i] = ft_mm_var->axis[i].def; - - p = variations; - while (p && *p) { - char *start; - char *end, *end2; - FT_ULong tag; - double value; - - while (_cairo_isspace (*p)) p++; - - start = p; - end = strchr (p, ','); - if (end && (end - p < 6)) - goto skip; - - tag = FT_MAKE_TAG(p[0], p[1], p[2], p[3]); - - p += 4; - while (_cairo_isspace (*p)) p++; - if (*p == '=') p++; - - if (p - start < 5) - goto skip; - - value = _cairo_strtod (p, &end2); - - while (end2 && _cairo_isspace (*end2)) end2++; - - if (end2 && (*end2 != ',' && *end2 != '\0')) - goto skip; - - for (i = 0; i < ft_mm_var->num_axis; i++) { - if (ft_mm_var->axis[i].tag == tag) { - coords[i] = (FT_Fixed)(value*65536); - break; - } - } - -skip: - p = end ? end + 1 : NULL; - } - - FT_Set_Var_Design_Coordinates (face, ft_mm_var->num_axis, coords); - free (coords); - } -} - /** * cairo_ft_scaled_font_lock_face: * @scaled_font: A #cairo_scaled_font_t from the FreeType font backend. Such an From 1b5677ad6d86a6e67ba99bf3df963a3789816dd5 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Mon, 18 Dec 2017 13:01:23 -0500 Subject: [PATCH 18/25] Apply font variations when loading glyphs So far, we only call cairo_ft_apply_variations() in cairo_ft_scaled_font_lock_face(), but this function is not used internally. We need to apply it also when loading glyphs. This makes pango-view show font variations correctly. --- src/cairo-ft-font.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index cc9779258..1889986a2 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -2356,6 +2356,8 @@ _cairo_ft_scaled_glyph_load_glyph (cairo_ft_scaled_font_t *scaled_font, if (unlikely (status)) return status; + cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->base.options.variations); + error = FT_Load_Glyph (face, _cairo_scaled_glyph_index(scaled_glyph), load_flags); From 5c7f07d4ea831dff42244782bd583ab5fe7817b7 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Mon, 18 Dec 2017 13:50:23 -0500 Subject: [PATCH 19/25] fixup: remove a hack This bug that this hack is working around has been fixed in freetype now. --- src/cairo-ft-font.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 1889986a2..0e352d435 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -2269,8 +2269,6 @@ cairo_ft_apply_variations (FT_Face face, FT_Error ret; ret = FT_Get_MM_Var (face, &ft_mm_var); - if (ret != 0) /* FIXME: the first FT_Get_MM_Var call on an FT_Face fails, so try again */ - ret = FT_Get_MM_Var (face, &ft_mm_var); if (ret == 0) { FT_Fixed *coords; unsigned int i; From 976b34c31cb6e70a902d57c705d92ea5f35577e3 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Mon, 18 Dec 2017 18:51:03 -0500 Subject: [PATCH 20/25] fixup We were passing a face index to cairo_ft_apply_variations, but that function was expecting an instance id, which is the face index shifted by 16. --- src/cairo-ft-font.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 0e352d435..f76db71aa 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -2262,11 +2262,12 @@ _cairo_ft_scaled_glyph_vertical_layout_bearing_fix (void *abstract_font, static void cairo_ft_apply_variations (FT_Face face, - int instance_id, + int face_index, const char *variations) { FT_MM_Var *ft_mm_var; FT_Error ret; + int instance_id = face_index >> 16; ret = FT_Get_MM_Var (face, &ft_mm_var); if (ret == 0) { From 82107467f0c52fee1368246aa9d931a82bcfadc4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 19 Dec 2017 00:38:25 -0500 Subject: [PATCH 21/25] [ft] Use variations from ft_options, not scaled-font Otherwise the variations from FcPattern won't be applied. --- src/cairo-ft-font.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index f76db71aa..8db04747a 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -2355,7 +2355,7 @@ _cairo_ft_scaled_glyph_load_glyph (cairo_ft_scaled_font_t *scaled_font, if (unlikely (status)) return status; - cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->base.options.variations); + cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->ft_options.base.variations); error = FT_Load_Glyph (face, _cairo_scaled_glyph_index(scaled_glyph), @@ -3784,7 +3784,7 @@ cairo_ft_scaled_font_lock_face (cairo_scaled_font_t *abstract_font) return NULL; } - cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->base.options.variations); + cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->ft_options.base.variations); /* Note: We deliberately release the unscaled font's mutex here, * so that we are not holding a lock across two separate calls to From d8ddadc5c3d5a74c433995d8bd82ba395bcd6973 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 19 Dec 2017 15:29:13 -0500 Subject: [PATCH 22/25] [ft] When merging font options, order variations correctly Later wins, and we want "options" to win over "other". --- src/cairo-ft-font.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 8db04747a..e8a9e20d5 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -1947,9 +1947,9 @@ _cairo_ft_options_merge (cairo_ft_options_t *options, /* 'merge' variations by concatenating - later entries win */ p = malloc (strlen (other->base.variations) + strlen (options->base.variations) + 2); p[0] = 0; - strcat (p, options->base.variations); - strcat (p, ","); strcat (p, other->base.variations); + strcat (p, ","); + strcat (p, options->base.variations); free (options->base.variations); options->base.variations = p; } From c32a5c741f8fbb21912edc7fc7763a361415b914 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 19 Dec 2017 17:36:36 -0500 Subject: [PATCH 23/25] Shortcut FT_Set_Var_Design_Coordinates We currently (have to) apply font variations for every glyph. Calling FT_Set_Var_Design_Coordinates has expensive side-effects, like resetting the autohinter, so we should avoid doing it fht he coordinates don't actually change. --- src/cairo-ft-font.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index e8a9e20d5..83d05ffe1 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -2271,6 +2271,7 @@ cairo_ft_apply_variations (FT_Face face, ret = FT_Get_MM_Var (face, &ft_mm_var); if (ret == 0) { + FT_Fixed *current_coords; FT_Fixed *coords; unsigned int i; const char *p; @@ -2328,8 +2329,21 @@ skip: p = end ? end + 1 : NULL; } + current_coords = malloc (sizeof (FT_Fixed) * ft_mm_var->num_axis); + ret = FT_Get_Var_Design_Coordinates (face, ft_mm_var->num_axis, current_coords); + if (ret == 0) { + for (i = 0; i < ft_mm_var->num_axis; i++) { + if (coords[i] != current_coords[i]) + break; + } + if (i == ft_mm_var->num_axis) + goto done; + } + FT_Set_Var_Design_Coordinates (face, ft_mm_var->num_axis, coords); +done: free (coords); + free (current_coords); } } From e8ddc505b3d4fd8492910e05e99ce2fb3c32bd2b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 19 Dec 2017 15:59:50 -0500 Subject: [PATCH 24/25] [ft] Fix warnings --- src/cairo-ft-font.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 83d05ffe1..19f1f286b 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -93,7 +93,6 @@ #define FT_LCD_FILTER_LEGACY 16 #endif -#define DOUBLE_TO_26_6(d) ((FT_F26Dot6)((d) * 64.0)) #define DOUBLE_FROM_26_6(t) ((double)(t) / 64.0) #define DOUBLE_TO_16_16(d) ((FT_Fixed)((d) * 65536.0)) #define DOUBLE_FROM_16_16(t) ((double)(t) / 65536.0) @@ -2261,13 +2260,13 @@ _cairo_ft_scaled_glyph_vertical_layout_bearing_fix (void *abstract_font, } static void -cairo_ft_apply_variations (FT_Face face, - int face_index, - const char *variations) +cairo_ft_apply_variations (FT_Face face, + unsigned int face_index, + const char *variations) { FT_MM_Var *ft_mm_var; FT_Error ret; - int instance_id = face_index >> 16; + unsigned int instance_id = face_index >> 16; ret = FT_Get_MM_Var (face, &ft_mm_var); if (ret == 0) { @@ -2290,8 +2289,8 @@ cairo_ft_apply_variations (FT_Face face, p = variations; while (p && *p) { - char *start; - char *end, *end2; + const char *start; + const char *end, *end2; FT_ULong tag; double value; @@ -2311,7 +2310,7 @@ cairo_ft_apply_variations (FT_Face face, if (p - start < 5) goto skip; - value = _cairo_strtod (p, &end2); + value = _cairo_strtod (p, (char **) &end2); while (end2 && _cairo_isspace (*end2)) end2++; @@ -2966,8 +2965,7 @@ _cairo_ft_has_color_glyphs (void *scaled) cairo_ft_unscaled_font_t *unscaled = ((cairo_ft_scaled_font_t *)scaled)->unscaled; if (!unscaled->have_color_set) { - FT_Face face; - face = _cairo_ft_unscaled_font_lock_face (unscaled); + _cairo_ft_unscaled_font_lock_face (unscaled); _cairo_ft_unscaled_font_unlock_face (unscaled); } @@ -3773,7 +3771,6 @@ cairo_ft_scaled_font_lock_face (cairo_scaled_font_t *abstract_font) { cairo_ft_scaled_font_t *scaled_font = (cairo_ft_scaled_font_t *) abstract_font; FT_Face face; - int instance_id; cairo_status_t status; if (! _cairo_scaled_font_is_ft (abstract_font)) { From a18c0decdacc3846588f424edb6d45ddc9be9560 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 2 Jan 2018 19:25:18 -0500 Subject: [PATCH 25/25] [ft] Remember variations set on FT_Face and apply them For this to work correctly with hb-view, needs a patch that went into FreeType today. --- src/cairo-ft-font.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 19f1f286b..caa101537 100644 --- a/src/cairo-ft-font.c +++ b/src/cairo-ft-font.c @@ -168,8 +168,9 @@ struct _cairo_ft_unscaled_font { cairo_matrix_t current_shape; FT_Matrix Current_Shape; - unsigned int have_color_set : 1; - unsigned int have_color : 1; /* true if the font contains color glyphs */ + unsigned int have_color_set : 1; + unsigned int have_color : 1; /* true if the font contains color glyphs */ + FT_Fixed *variations; /* variation settings that FT_Face came */ cairo_mutex_t mutex; int lock_count; @@ -425,12 +426,25 @@ _cairo_ft_unscaled_font_init (cairo_ft_unscaled_font_t *unscaled, _cairo_unscaled_font_init (&unscaled->base, &cairo_ft_unscaled_font_backend); + unscaled->variations = NULL; + if (from_face) { unscaled->from_face = TRUE; _cairo_ft_unscaled_font_init_key (unscaled, TRUE, NULL, face->face_index, face); + unscaled->have_color = FT_HAS_COLOR (face) != 0; unscaled->have_color_set = TRUE; + + { + FT_MM_Var *ft_mm_var; + if (0 == FT_Get_MM_Var (face, &ft_mm_var)) + { + unscaled->variations = calloc (ft_mm_var->num_axis, sizeof (FT_Fixed)); + if (unscaled->variations) + FT_Get_Var_Design_Coordinates (face, ft_mm_var->num_axis, unscaled->variations); + } + } } else { char *filename_copy; @@ -443,7 +457,7 @@ _cairo_ft_unscaled_font_init (cairo_ft_unscaled_font_t *unscaled, _cairo_ft_unscaled_font_init_key (unscaled, FALSE, filename_copy, id, NULL); - unscaled->have_color_set = FALSE; + unscaled->have_color_set = FALSE; } unscaled->have_scale = FALSE; @@ -474,6 +488,8 @@ _cairo_ft_unscaled_font_fini (cairo_ft_unscaled_font_t *unscaled) free (unscaled->filename); unscaled->filename = NULL; + free (unscaled->variations); + CAIRO_MUTEX_FINI (unscaled->mutex); } @@ -2260,13 +2276,12 @@ _cairo_ft_scaled_glyph_vertical_layout_bearing_fix (void *abstract_font, } static void -cairo_ft_apply_variations (FT_Face face, - unsigned int face_index, - const char *variations) +cairo_ft_apply_variations (FT_Face face, + cairo_ft_scaled_font_t *scaled_font) { FT_MM_Var *ft_mm_var; FT_Error ret; - unsigned int instance_id = face_index >> 16; + unsigned int instance_id = scaled_font->unscaled->id >> 16; ret = FT_Get_MM_Var (face, &ft_mm_var); if (ret == 0) { @@ -2278,7 +2293,11 @@ cairo_ft_apply_variations (FT_Face face, coords = malloc (sizeof (FT_Fixed) * ft_mm_var->num_axis); /* FIXME check coords. */ - if (instance_id && instance_id <= ft_mm_var->num_namedstyles) + if (scaled_font->unscaled->variations) + { + memcpy (coords, scaled_font->unscaled->variations, ft_mm_var->num_axis * sizeof (*coords)); + } + else if (instance_id && instance_id <= ft_mm_var->num_namedstyles) { FT_Var_Named_Style *instance = &ft_mm_var->namedstyle[instance_id - 1]; memcpy (coords, instance->coords, ft_mm_var->num_axis * sizeof (*coords)); @@ -2287,7 +2306,7 @@ cairo_ft_apply_variations (FT_Face face, for (i = 0; i < ft_mm_var->num_axis; i++) coords[i] = ft_mm_var->axis[i].def; - p = variations; + p = scaled_font->ft_options.base.variations; while (p && *p) { const char *start; const char *end, *end2; @@ -2368,7 +2387,7 @@ _cairo_ft_scaled_glyph_load_glyph (cairo_ft_scaled_font_t *scaled_font, if (unlikely (status)) return status; - cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->ft_options.base.variations); + cairo_ft_apply_variations (face, scaled_font); error = FT_Load_Glyph (face, _cairo_scaled_glyph_index(scaled_glyph), @@ -3795,7 +3814,7 @@ cairo_ft_scaled_font_lock_face (cairo_scaled_font_t *abstract_font) return NULL; } - cairo_ft_apply_variations (face, scaled_font->unscaled->id, scaled_font->ft_options.base.variations); + cairo_ft_apply_variations (face, scaled_font); /* Note: We deliberately release the unscaled font's mutex here, * so that we are not holding a lock across two separate calls to