2 commits - src/cairo-ft-font.c
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Thu Mar 21 20:58:51 UTC 2024
src/cairo-ft-font.c | 103 ++++++++++++++++++++++++++--------------------------
1 file changed, 53 insertions(+), 50 deletions(-)
New commits:
commit 057a949bb71a94b1e1881fc641a5ebddd820c0e0
Merge: c60489047 6e5e4bd97
Author: Adrian Johnson <ajohnson at redneon.com>
Date: Thu Mar 21 20:58:48 2024 +0000
Merge branch 'ft-font-accuracy-new' into 'master'
Improve accuracy of computed metrics for FT fonts
See merge request cairo/cairo!533
commit 6e5e4bd978b730ddc41dfdf020de401f5d9ee229
Author: Vincent Lefevre <vincent at vinc17.net>
Date: Mon Jan 22 02:48:07 2024 +0100
Improve accuracy of computed metrics for FT fonts
In particular, with bitmap fonts, a floating-point error was affecting
y_bearing, yielding a value close to an integer instead of the integer
exactly. The change consists in replacing some operations of the form
A * (1/B) by A/B. Details of the issue in #503.
This new code will not always avoid rounding errors in final values
when these values could be exact, but it makes some of these errors
disappear.
The changes in the src/cairo-ft-font.c code consists of:
* Define the SCALE() macro (with some explanations in a comment).
* Remove the declarations and definitions of x_factor and y_factor.
* Update the code to use SCALE() instead of x_factor and y_factor.
perl -pi -e 's{(DOUBLE_\w+) ?(\(.*\)) \* ([xy])_factor}
{SCALE (\1 \2, unscaled->\3_scale)}' \
src/cairo-ft-font.c
* Replace the remaining 0 * x_factor and 0 * y_factor by 0.
diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index bf0872e93..7618e37dc 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -109,6 +109,32 @@
#define DOUBLE_TO_16_16(d) ((FT_Fixed)((d) * 65536.0))
#define DOUBLE_FROM_16_16(t) ((double)(t) / 65536.0)
+/* SCALE() mimics code from commit 399b00a99b2bbc1c56a05974c936aa69a08021f5
+ * concerning a potential division by 0, but instead of doing a * (1/b), it
+ * does a/b, thus improving the accuracy. With a * (1/b), for a bitmap font
+ * of size 13, the computed -y_bearing was 0x1.6000000000001p+3 instead of
+ * 0x1.6p+3 (= 11). This triggered a bug in GNU Emacs (when built against
+ * cairo), which rounded the value to an integer with ceil().
+ * Details:
+ * https://gitlab.freedesktop.org/cairo/cairo/-/issues/503
+ * https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44284
+ *
+ * Note that rounding errors are not necessarily expected by applications
+ * in simple cases like the GNU Emacs one (an identity transformation,
+ * which should normally leave the inputs unchanged). However, with the
+ * current cairo code, due to the scaling, there is no guarantee that
+ * rounding errors will always be avoided at the end. For instance,
+ * (a/b)*b may be different from a, but this is still better than doing
+ * (a*(1/b))*b.
+ *
+ * According to the commit mentioned above, avoiding a division by zero
+ * was an attempt to fix
+ * https://bugzilla.gnome.org/show_bug.cgi?id=311299
+ * but this did not actually solve the problem. So it might be possible
+ * to change SCALE() to just do (a) / (b).
+ */
+#define SCALE(a,b) ((b) == 0 ? 0.0 : (a) / (b))
+
/* This is the max number of FT_face objects we keep open at once
*/
#define MAX_OPEN_FACES 10
@@ -2104,27 +2130,15 @@ _cairo_ft_font_face_scaled_font_create (void *abstract_font_face,
*/
if (scaled_font->base.options.hint_metrics != CAIRO_HINT_METRICS_OFF ||
face->units_per_EM == 0) {
- double x_factor, y_factor;
-
- if (unscaled->x_scale == 0)
- x_factor = 0;
- else
- x_factor = 1 / unscaled->x_scale;
-
- if (unscaled->y_scale == 0)
- y_factor = 0;
- else
- y_factor = 1 / unscaled->y_scale;
-
- fs_metrics.ascent = DOUBLE_FROM_26_6(metrics->ascender) * y_factor;
- fs_metrics.descent = DOUBLE_FROM_26_6(- metrics->descender) * y_factor;
- fs_metrics.height = DOUBLE_FROM_26_6(metrics->height) * y_factor;
+ fs_metrics.ascent = SCALE (DOUBLE_FROM_26_6 (metrics->ascender), unscaled->y_scale);
+ fs_metrics.descent = SCALE (DOUBLE_FROM_26_6 (- metrics->descender), unscaled->y_scale);
+ fs_metrics.height = SCALE (DOUBLE_FROM_26_6 (metrics->height), unscaled->y_scale);
if (!_cairo_ft_scaled_font_is_vertical (&scaled_font->base)) {
- fs_metrics.max_x_advance = DOUBLE_FROM_26_6(metrics->max_advance) * x_factor;
+ fs_metrics.max_x_advance = SCALE (DOUBLE_FROM_26_6 (metrics->max_advance), unscaled->x_scale);
fs_metrics.max_y_advance = 0;
} else {
fs_metrics.max_x_advance = 0;
- fs_metrics.max_y_advance = DOUBLE_FROM_26_6(metrics->max_advance) * y_factor;
+ fs_metrics.max_y_advance = SCALE (DOUBLE_FROM_26_6 (metrics->max_advance), unscaled->y_scale);
}
} else {
double scale = face->units_per_EM;
@@ -3137,7 +3151,6 @@ _cairo_ft_scaled_glyph_get_metrics (cairo_ft_scaled_font_t *scaled_font,
cairo_text_extents_t *fs_metrics)
{
FT_Glyph_Metrics *metrics;
- double x_factor, y_factor;
cairo_ft_unscaled_font_t *unscaled = scaled_font->unscaled;
cairo_bool_t hint_metrics = scaled_font->base.options.hint_metrics != CAIRO_HINT_METRICS_OFF;
FT_GlyphSlot glyph = face->glyph;
@@ -3147,16 +3160,6 @@ _cairo_ft_scaled_glyph_get_metrics (cairo_ft_scaled_font_t *scaled_font,
*/
metrics = &glyph->metrics;
- if (unscaled->x_scale == 0)
- x_factor = 0;
- else
- x_factor = 1 / unscaled->x_scale;
-
- if (unscaled->y_scale == 0)
- y_factor = 0;
- else
- y_factor = 1 / unscaled->y_scale;
-
/*
* Note: Y coordinates of the horizontal bearing need to be negated.
*
@@ -3181,13 +3184,13 @@ _cairo_ft_scaled_glyph_get_metrics (cairo_ft_scaled_font_t *scaled_font,
advance = ((metrics->horiAdvance + 32) & -64);
- fs_metrics->x_bearing = DOUBLE_FROM_26_6 (x1) * x_factor;
- fs_metrics->y_bearing = DOUBLE_FROM_26_6 (y1) * y_factor;
+ fs_metrics->x_bearing = SCALE (DOUBLE_FROM_26_6 (x1), unscaled->x_scale);
+ fs_metrics->y_bearing = SCALE (DOUBLE_FROM_26_6 (y1), unscaled->y_scale);
- fs_metrics->width = DOUBLE_FROM_26_6 (x2 - x1) * x_factor;
- fs_metrics->height = DOUBLE_FROM_26_6 (y2 - y1) * y_factor;
+ fs_metrics->width = SCALE (DOUBLE_FROM_26_6 (x2 - x1), unscaled->x_scale);
+ fs_metrics->height = SCALE (DOUBLE_FROM_26_6 (y2 - y1), unscaled->y_scale);
- fs_metrics->x_advance = DOUBLE_FROM_26_6 (advance) * x_factor;
+ fs_metrics->x_advance = SCALE (DOUBLE_FROM_26_6 (advance), unscaled->x_scale);
fs_metrics->y_advance = 0;
} else {
x1 = (metrics->vertBearingX) & -64;
@@ -3197,37 +3200,37 @@ _cairo_ft_scaled_glyph_get_metrics (cairo_ft_scaled_font_t *scaled_font,
advance = ((metrics->vertAdvance + 32) & -64);
- fs_metrics->x_bearing = DOUBLE_FROM_26_6 (x1) * x_factor;
- fs_metrics->y_bearing = DOUBLE_FROM_26_6 (y1) * y_factor;
+ fs_metrics->x_bearing = SCALE (DOUBLE_FROM_26_6 (x1), unscaled->x_scale);
+ fs_metrics->y_bearing = SCALE (DOUBLE_FROM_26_6 (y1), unscaled->y_scale);
- fs_metrics->width = DOUBLE_FROM_26_6 (x2 - x1) * x_factor;
- fs_metrics->height = DOUBLE_FROM_26_6 (y2 - y1) * y_factor;
+ fs_metrics->width = SCALE (DOUBLE_FROM_26_6 (x2 - x1), unscaled->x_scale);
+ fs_metrics->height = SCALE (DOUBLE_FROM_26_6 (y2 - y1), unscaled->y_scale);
fs_metrics->x_advance = 0;
- fs_metrics->y_advance = DOUBLE_FROM_26_6 (advance) * y_factor;
+ fs_metrics->y_advance = SCALE (DOUBLE_FROM_26_6 (advance), unscaled->y_scale);
}
} else {
- fs_metrics->width = DOUBLE_FROM_26_6 (metrics->width) * x_factor;
- fs_metrics->height = DOUBLE_FROM_26_6 (metrics->height) * y_factor;
+ fs_metrics->width = SCALE (DOUBLE_FROM_26_6 (metrics->width), unscaled->x_scale);
+ fs_metrics->height = SCALE (DOUBLE_FROM_26_6 (metrics->height), unscaled->y_scale);
if (!vertical_layout) {
- fs_metrics->x_bearing = DOUBLE_FROM_26_6 (metrics->horiBearingX) * x_factor;
- fs_metrics->y_bearing = DOUBLE_FROM_26_6 (-metrics->horiBearingY) * y_factor;
+ fs_metrics->x_bearing = SCALE (DOUBLE_FROM_26_6 (metrics->horiBearingX), unscaled->x_scale);
+ fs_metrics->y_bearing = SCALE (DOUBLE_FROM_26_6 (-metrics->horiBearingY), unscaled->y_scale);
if (hint_metrics || glyph->format != FT_GLYPH_FORMAT_OUTLINE)
- fs_metrics->x_advance = DOUBLE_FROM_26_6 (metrics->horiAdvance) * x_factor;
+ fs_metrics->x_advance = SCALE (DOUBLE_FROM_26_6 (metrics->horiAdvance), unscaled->x_scale);
else
- fs_metrics->x_advance = DOUBLE_FROM_16_16 (glyph->linearHoriAdvance) * x_factor;
- fs_metrics->y_advance = 0 * y_factor;
+ fs_metrics->x_advance = SCALE (DOUBLE_FROM_16_16 (glyph->linearHoriAdvance), unscaled->x_scale);
+ fs_metrics->y_advance = 0;
} else {
- fs_metrics->x_bearing = DOUBLE_FROM_26_6 (metrics->vertBearingX) * x_factor;
- fs_metrics->y_bearing = DOUBLE_FROM_26_6 (metrics->vertBearingY) * y_factor;
+ fs_metrics->x_bearing = SCALE (DOUBLE_FROM_26_6 (metrics->vertBearingX), unscaled->x_scale);
+ fs_metrics->y_bearing = SCALE (DOUBLE_FROM_26_6 (metrics->vertBearingY), unscaled->y_scale);
- fs_metrics->x_advance = 0 * x_factor;
+ fs_metrics->x_advance = 0;
if (hint_metrics || glyph->format != FT_GLYPH_FORMAT_OUTLINE)
- fs_metrics->y_advance = DOUBLE_FROM_26_6 (metrics->vertAdvance) * y_factor;
+ fs_metrics->y_advance = SCALE (DOUBLE_FROM_26_6 (metrics->vertAdvance), unscaled->y_scale);
else
- fs_metrics->y_advance = DOUBLE_FROM_16_16 (glyph->linearVertAdvance) * y_factor;
+ fs_metrics->y_advance = SCALE (DOUBLE_FROM_16_16 (glyph->linearVertAdvance), unscaled->y_scale);
}
}
}
More information about the cairo-commit
mailing list