Re: OpenType font variations and cairo
Adrian Johnson <[email protected]>
| Newsgroups | gmane.comp.lib.cairo |
|---|---|
| Message-ID | <[email protected]> |
On 17/09/17 02:57, Matthias Clasen wrote: > On Fri, Sep 15, 2017 at 6:51 AM, Adrian Johnson <[email protected] > <mailto:[email protected]>> wrote: > > > > We really need a test case for this. It is hard to review code I > can't test. > > > I've added a minimal testcase here: > > https://github.com/matthiasclasen/cairo/tree/wip/matthiasc/font-variations > > It promptly found a freetype bug :-) I built the latest freetype, Behdad's fontconfig branch, and your branch. First problem I had was the variation test ignores the status. See attached patch. After fixing that and ensuring I had the correct font installed I got a crash in fontconfig. fcfreetype.c line 1313 divide by zero 1312: double default_value = master->axis[i].def / (double) (1 << 16); 1313: double mult = value / default_value; The Adobe Variable Font Prototype font has a 'CNTR' axis with a default value of 0. Some more comments on your patch: In cairo-font-options.c you are adding a new function _intern_string_hash() which is almost identical to the _intern_string_hash() in cairo-misc.c Any reason why the cairo-misc.c version can't be used? I've attached a patch that refactors the float parsing code from cff-subset into a new function: _cairo_strtod(). So you can now use this for a C locale strtod. In cairo_ft_apply_variations(), strtod() returns a double so just make 'value' have type double instead of float. +/** + * 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 the @options struct is modified. Should this be "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()"? -- cairo mailing list [email protected] https://lists.cairographics.org/mailman/listinfo/cairo
font-variation.diff
(text/x-patch, 1.1 KB)
diff --git a/test/font-variations.c b/test/font-variations.c
index df222e328..d57a45bdd 100644
--- a/test/font-variations.c
+++ b/test/font-variations.c
@@ -102,7 +102,7 @@ test_variation (cairo_test_context_t *ctx,
return CAIRO_TEST_FAILURE;
}
if (strcmp (ft_face->family_name, "Adobe Variable Font Prototype") != 0) {
- cairo_test_log (ctx, "Wrong font");
+ 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;
}
@@ -176,11 +176,12 @@ preamble (cairo_test_context_t *ctx)
int i;
for (i = 0; tests[i].input; i++) {
- if (test_variation (ctx,
- tests[i].input,
- tests[i].tag,
- tests[i].expected_default,
- tests[i].expected_value) != CAIRO_TEST_SUCCESS)
+ 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;
}
0001-factor-out-ascii-to-double-code-in-cff-subset-into-_.patch
(text/x-patch, 5.1 KB)
From 315cf7b79b84a8b5763542a98e470727484112ff Mon Sep 17 00:00:00 2001 From: Adrian Johnson <[email protected]> Date: Sun, 17 Sep 2017 14:15:25 +0930 Subject: [PATCH] factor out ascii to double code in cff-subset into _cairo_strtod --- src/cairo-cff-subset.c | 27 +++------------------ src/cairo-misc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++-- src/cairo-output-stream.c | 2 +- src/cairo-type1-subset.c | 2 +- src/cairoint.h | 5 +++- 5 files changed, 67 insertions(+), 29 deletions(-) diff --git a/src/cairo-cff-subset.c b/src/cairo-cff-subset.c index 49d981199..13fee5058 100644 --- a/src/cairo-cff-subset.c +++ b/src/cairo-cff-subset.c @@ -295,20 +295,11 @@ decode_nibble (int n, char *buf) static unsigned char * decode_real (unsigned char *p, double *real) { - const char *decimal_point; - int decimal_point_len; - int n; char buffer[100]; - char buffer2[200]; - char *q; char *buf = buffer; char *buf_end = buffer + sizeof (buffer); - - decimal_point = cairo_get_locale_decimal_point (); - decimal_point_len = strlen (decimal_point); - - assert (decimal_point_len != 0); - assert (sizeof(buffer) + decimal_point_len < sizeof(buffer2)); + char *end; + int n; p++; while (buf + 2 < buf_end) { @@ -324,19 +315,7 @@ decode_real (unsigned char *p, double *real) }; *buf = 0; - buf = buffer; - if (strchr (buffer, '.')) { - q = strchr (buffer, '.'); - strncpy (buffer2, buffer, q - buffer); - buf = buffer2 + (q - buffer); - strncpy (buf, decimal_point, decimal_point_len); - buf += decimal_point_len; - strcpy (buf, q + 1); - buf = buffer2; - } - - if (sscanf(buf, "%lf", real) != 1) - *real = 0.0; + *real = _cairo_strtod (buffer, &end); return p; } diff --git a/src/cairo-misc.c b/src/cairo-misc.c index e9b0ab6be..19629feca 100644 --- a/src/cairo-misc.c +++ b/src/cairo-misc.c @@ -771,7 +771,7 @@ _cairo_half_from_float (float f) # include <locale.h> const char * -cairo_get_locale_decimal_point (void) +_cairo_get_locale_decimal_point (void) { struct lconv *locale_data = localeconv (); return locale_data->decimal_point; @@ -780,12 +780,68 @@ cairo_get_locale_decimal_point (void) #else /* Android's Bionic libc doesn't provide decimal_point */ const char * -cairo_get_locale_decimal_point (void) +_cairo_get_locale_decimal_point (void) { return "."; } #endif +/* strtod replacement that ignores locale and only accepts decimal points */ +double +_cairo_strtod (const char *nptr, char **endptr) +{ + const char *decimal_point; + int decimal_point_len; + const char *p; + char buf[100]; + char *bufptr; + char *bufend = buf + sizeof(buf) - 1; + double value; + char *end; + int delta; + cairo_bool_t have_dp; + + decimal_point = _cairo_get_locale_decimal_point (); + decimal_point_len = strlen (decimal_point); + assert (decimal_point_len != 0); + + p = nptr; + bufptr = buf; + delta = 0; + have_dp = FALSE; + while (*p && _cairo_isspace (*p)) { + p++; + delta++; + } + + while (*p && (bufptr + decimal_point_len < bufend)) { + if (_cairo_isdigit (*p)) { + *bufptr++ = *p; + } else if (*p == '.') { + if (have_dp) + break; + strncpy (bufptr, decimal_point, decimal_point_len); + bufptr += decimal_point_len; + delta -= decimal_point_len - 1; + have_dp = TRUE; + } else { + break; + } + p++; + } + *bufptr = 0; + + value = strtod (buf, &end); + if (endptr) { + if (end == buf) + *endptr = (char*)(nptr); + else + *endptr = (char*)(nptr + (end - buf) + delta); + } + + return value; +} + #ifdef _WIN32 #define WIN32_LEAN_AND_MEAN diff --git a/src/cairo-output-stream.c b/src/cairo-output-stream.c index 07991703b..76d718aa7 100644 --- a/src/cairo-output-stream.c +++ b/src/cairo-output-stream.c @@ -312,7 +312,7 @@ _cairo_dtostr (char *buffer, size_t size, double d, cairo_bool_t limited_precisi if (d == 0.0) d = 0.0; - decimal_point = cairo_get_locale_decimal_point (); + decimal_point = _cairo_get_locale_decimal_point (); decimal_point_len = strlen (decimal_point); assert (decimal_point_len != 0); diff --git a/src/cairo-type1-subset.c b/src/cairo-type1-subset.c index 810dc9f74..4dd7ac17e 100644 --- a/src/cairo-type1-subset.c +++ b/src/cairo-type1-subset.c @@ -311,7 +311,7 @@ cairo_type1_font_subset_get_matrix (cairo_type1_font_subset_t *font, const char *decimal_point; int decimal_point_len; - decimal_point = cairo_get_locale_decimal_point (); + decimal_point = _cairo_get_locale_decimal_point (); decimal_point_len = strlen (decimal_point); assert (decimal_point_len != 0); diff --git a/src/cairoint.h b/src/cairoint.h index f64355921..8573ae616 100644 --- a/src/cairoint.h +++ b/src/cairoint.h @@ -913,7 +913,10 @@ cairo_private void _cairo_intern_string_reset_static_data (void); cairo_private const char * -cairo_get_locale_decimal_point (void); +_cairo_get_locale_decimal_point (void); + +cairo_private double +_cairo_strtod (const char *nptr, char **endptr); /* cairo-path-fixed.c */ cairo_private cairo_path_fixed_t * -- 2.11.0