From 0a4399ca228d244e646abdb3487da0f13b228889 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 19 May 2010 15:45:06 -0400 Subject: [PATCH] Fix scale issues hb_font_set_scale() now sets the value to be used to represent a unit pixel. For example, if rendering a 10px font with a 26.6 representation, you would set scale to (10 << 6). For 10px in 16.16 you would set it to (10 << 16). This space should be the same space that the get_glyph_metrics and get_kerning callbacks work in. --- src/hb-common.h | 1 - src/hb-font-private.hh | 8 +++---- src/hb-font.cc | 12 +++++------ src/hb-font.h | 12 +++-------- src/hb-ft.cc | 4 ++-- src/hb-ot-layout-gdef-private.hh | 6 +++--- src/hb-ot-layout-gpos-private.hh | 37 +++++++++++++++----------------- src/hb-ot-layout-gsub-private.hh | 2 +- src/hb-ot-layout-private.hh | 20 +++++++++-------- src/hb-ot-layout.cc | 31 +++++++++++++------------- src/hb-private.h | 4 ---- 11 files changed, 61 insertions(+), 76 deletions(-) diff --git a/src/hb-common.h b/src/hb-common.h index f64d6cc0f..594d358b7 100644 --- a/src/hb-common.h +++ b/src/hb-common.h @@ -67,7 +67,6 @@ typedef uint32_t hb_tag_t; typedef uint32_t hb_codepoint_t; typedef int32_t hb_position_t; -typedef int32_t hb_16dot16_t; typedef uint32_t hb_mask_t; typedef void (*hb_destroy_func_t) (void *user_data); diff --git a/src/hb-font-private.hh b/src/hb-font-private.hh index 7cc632782..a58701e60 100644 --- a/src/hb-font-private.hh +++ b/src/hb-font-private.hh @@ -33,8 +33,6 @@ #include "hb-ot-head-private.hh" -#include "hb-ot-layout-private.hh" - HB_BEGIN_DECLS /* @@ -69,7 +67,7 @@ struct _hb_face_t { hb_blob_t *head_blob; const struct head *head_table; - hb_ot_layout_t ot_layout; + struct hb_ot_layout_t *ot_layout; }; @@ -80,8 +78,8 @@ struct _hb_face_t { struct _hb_font_t { hb_reference_count_t ref_count; - hb_16dot16_t x_scale; - hb_16dot16_t y_scale; + unsigned int x_scale; + unsigned int y_scale; unsigned int x_ppem; unsigned int y_ppem; diff --git a/src/hb-font.cc b/src/hb-font.cc index 3feddfc8e..c5355f653 100644 --- a/src/hb-font.cc +++ b/src/hb-font.cc @@ -234,7 +234,7 @@ static hb_face_t _hb_face_nil = { NULL, /* head_blob */ NULL, /* head_table */ - {} /* ot_layout */ + NULL /* ot_layout */ }; @@ -255,7 +255,7 @@ hb_face_create_for_tables (hb_get_table_func_t get_table, face->destroy = destroy; face->user_data = user_data; - _hb_ot_layout_init (face); + face->ot_layout = _hb_ot_layout_new (face); return face; } @@ -330,7 +330,7 @@ hb_face_create_for_data (hb_blob_t *blob, face->head_blob = Sanitizer::sanitize (hb_face_get_table (face, HB_OT_TAG_head)); face->head_table = Sanitizer::lock_instance (face->head_blob); - _hb_ot_layout_init (face); + face->ot_layout = _hb_ot_layout_new (face); return face; } @@ -353,7 +353,7 @@ hb_face_destroy (hb_face_t *face) { HB_OBJECT_DO_DESTROY (face); - _hb_ot_layout_fini (face); + _hb_ot_layout_free (face->ot_layout); hb_blob_unlock (face->head_blob); hb_blob_destroy (face->head_blob); @@ -458,8 +458,8 @@ hb_font_set_funcs (hb_font_t *font, void hb_font_set_scale (hb_font_t *font, - hb_16dot16_t x_scale, - hb_16dot16_t y_scale) + unsigned int x_scale, + unsigned int y_scale) { if (HB_OBJECT_IS_INERT (font)) return; diff --git a/src/hb-font.h b/src/hb-font.h index fee8ada22..433d31a3b 100644 --- a/src/hb-font.h +++ b/src/hb-font.h @@ -177,18 +177,12 @@ hb_font_get_funcs (hb_font_t *font); /* - * XXX - * should we decompose this to units_per_EM and font-size? - * units_per_EM setting then can go into the face, or better, - * read from the 'head' table. - * - * Then we either need size+shape like freetype does, or a full - * matrix. + * We should add support for full matrices. */ void hb_font_set_scale (hb_font_t *font, - hb_16dot16_t x_scale, - hb_16dot16_t y_scale); + unsigned int x_scale, + unsigned int y_scale); /* * A zero value means "no hinting in that direction" diff --git a/src/hb-ft.cc b/src/hb-ft.cc index 5b911bcf5..fdd87bc29 100644 --- a/src/hb-ft.cc +++ b/src/hb-ft.cc @@ -230,8 +230,8 @@ hb_ft_font_create (FT_Face ft_face, hb_ft_get_font_funcs (), destroy, ft_face); hb_font_set_scale (font, - ft_face->size->metrics.x_scale, - ft_face->size->metrics.y_scale); + ((uint64_t) ft_face->size->metrics.x_scale * (uint64_t) ft_face->units_per_EM) >> 16, + ((uint64_t) ft_face->size->metrics.y_scale * (uint64_t) ft_face->units_per_EM) >> 16); hb_font_set_ppem (font, ft_face->size->metrics.x_ppem, ft_face->size->metrics.y_ppem); diff --git a/src/hb-ot-layout-gdef-private.hh b/src/hb-ot-layout-gdef-private.hh index 63fbee7b2..4989363a0 100644 --- a/src/hb-ot-layout-gdef-private.hh +++ b/src/hb-ot-layout-gdef-private.hh @@ -95,7 +95,7 @@ struct CaretValueFormat1 inline int get_caret_value (hb_ot_layout_context_t *c, hb_codepoint_t glyph_id HB_UNUSED) const { /* TODO vertical */ - return _hb_16dot16_mul_round (c->font->x_scale, coordinate); + return c->scale_x (coordinate); } inline bool sanitize (hb_sanitize_context_t *c) { @@ -144,8 +144,8 @@ struct CaretValueFormat3 inline int get_caret_value (hb_ot_layout_context_t *c, hb_codepoint_t glyph_id HB_UNUSED) const { /* TODO vertical */ - return _hb_16dot16_mul_round (c->font->x_scale, coordinate) + - ((this+deviceTable).get_delta (c->font->x_ppem) << 16); + return c->scale_x (coordinate) + + ((this+deviceTable).get_delta (c->font->x_ppem) * c->font->x_scale); } inline bool sanitize (hb_sanitize_context_t *c) { diff --git a/src/hb-ot-layout-gpos-private.hh b/src/hb-ot-layout-gpos-private.hh index 35673b80e..81e18d485 100644 --- a/src/hb-ot-layout-gpos-private.hh +++ b/src/hb-ot-layout-gpos-private.hh @@ -93,18 +93,15 @@ struct ValueFormat : USHORT hb_internal_glyph_position_t &glyph_pos) const { unsigned int x_ppem, y_ppem; - hb_16dot16_t x_scale, y_scale; unsigned int format = *this; if (!format) return; - x_scale = layout->font->x_scale; - y_scale = layout->font->y_scale; /* design units -> fractional pixel */ - if (format & xPlacement) glyph_pos.x_offset += _hb_16dot16_mul_round (x_scale, get_short (values++)); - if (format & yPlacement) glyph_pos.y_offset += _hb_16dot16_mul_round (y_scale, get_short (values++)); - if (format & xAdvance) glyph_pos.x_advance += _hb_16dot16_mul_round (x_scale, get_short (values++)); - if (format & yAdvance) glyph_pos.y_advance += _hb_16dot16_mul_round (y_scale, get_short (values++)); + if (format & xPlacement) glyph_pos.x_offset += layout->scale_x (get_short (values++)); + if (format & yPlacement) glyph_pos.y_offset += layout->scale_y (get_short (values++)); + if (format & xAdvance) glyph_pos.x_advance += layout->scale_x (get_short (values++)); + if (format & yAdvance) glyph_pos.y_advance += layout->scale_y (get_short (values++)); if (!has_device ()) return; @@ -115,16 +112,16 @@ struct ValueFormat : USHORT /* pixel -> fractional pixel */ if (format & xPlaDevice) { - if (x_ppem) glyph_pos.x_offset += (base + get_device (values++)).get_delta (x_ppem) << 16; else values++; + if (x_ppem) glyph_pos.x_offset += (base + get_device (values++)).get_delta (x_ppem) * layout->font->x_scale; else values++; } if (format & yPlaDevice) { - if (y_ppem) glyph_pos.y_offset += (base + get_device (values++)).get_delta (y_ppem) << 16; else values++; + if (y_ppem) glyph_pos.y_offset += (base + get_device (values++)).get_delta (y_ppem) * layout->font->y_scale; else values++; } if (format & xAdvDevice) { - if (x_ppem) glyph_pos.x_advance += (base + get_device (values++)).get_delta (x_ppem) << 16; else values++; + if (x_ppem) glyph_pos.x_advance += (base + get_device (values++)).get_delta (x_ppem) * layout->font->x_scale; else values++; } if (format & yAdvDevice) { - if (y_ppem) glyph_pos.y_advance += (base + get_device (values++)).get_delta (y_ppem) << 16; else values++; + if (y_ppem) glyph_pos.y_advance += (base + get_device (values++)).get_delta (y_ppem) * layout->font->y_scale; else values++; } } @@ -208,8 +205,8 @@ struct AnchorFormat1 inline void get_anchor (hb_ot_layout_context_t *layout, hb_codepoint_t glyph_id HB_UNUSED, hb_position_t *x, hb_position_t *y) const { - *x = _hb_16dot16_mul_round (layout->font->x_scale, xCoordinate); - *y = _hb_16dot16_mul_round (layout->font->y_scale, yCoordinate); + *x = layout->scale_x (xCoordinate); + *y = layout->scale_y (yCoordinate); } inline bool sanitize (hb_sanitize_context_t *c) { @@ -240,8 +237,8 @@ struct AnchorFormat2 if (x_ppem || y_ppem) ret = hb_font_get_contour_point (layout->font, layout->face, anchorPoint, glyph_id, &cx, &cy); - *x = x_ppem && ret ? cx : _hb_16dot16_mul_round (layout->font->x_scale, xCoordinate); - *y = y_ppem && ret ? cy : _hb_16dot16_mul_round (layout->font->y_scale, yCoordinate); + *x = x_ppem && ret ? cx : layout->scale_x (xCoordinate); + *y = y_ppem && ret ? cy : layout->scale_y (yCoordinate); } inline bool sanitize (hb_sanitize_context_t *c) { @@ -266,14 +263,14 @@ struct AnchorFormat3 inline void get_anchor (hb_ot_layout_context_t *layout, hb_codepoint_t glyph_id HB_UNUSED, hb_position_t *x, hb_position_t *y) const { - *x = _hb_16dot16_mul_round (layout->font->x_scale, xCoordinate); - *y = _hb_16dot16_mul_round (layout->font->y_scale, yCoordinate); + *x = layout->scale_x (xCoordinate); + *y = layout->scale_y (yCoordinate); /* pixel -> fractional pixel */ if (layout->font->x_ppem) - *x += (this+xDeviceTable).get_delta (layout->font->x_ppem) << 16; + *x += (this+xDeviceTable).get_delta (layout->font->x_ppem) * layout->font->x_scale; if (layout->font->y_ppem) - *y += (this+yDeviceTable).get_delta (layout->font->y_ppem) << 16; + *y += (this+yDeviceTable).get_delta (layout->font->y_ppem) * layout->font->y_scale; } inline bool sanitize (hb_sanitize_context_t *c) { @@ -1615,7 +1612,7 @@ inline bool ExtensionPos::sanitize (hb_sanitize_context_t *c) static inline bool position_lookup (hb_apply_context_t *c, unsigned int lookup_index) { - const GPOS &gpos = *(c->layout->face->ot_layout.gpos); + const GPOS &gpos = *(c->layout->face->ot_layout->gpos); const PosLookup &l = gpos.get_lookup (lookup_index); if (unlikely (c->nesting_level_left == 0)) diff --git a/src/hb-ot-layout-gsub-private.hh b/src/hb-ot-layout-gsub-private.hh index f8ad1f8f9..828c45825 100644 --- a/src/hb-ot-layout-gsub-private.hh +++ b/src/hb-ot-layout-gsub-private.hh @@ -924,7 +924,7 @@ inline bool ExtensionSubst::is_reverse (void) const static inline bool substitute_lookup (hb_apply_context_t *c, unsigned int lookup_index) { - const GSUB &gsub = *(c->layout->face->ot_layout.gsub); + const GSUB &gsub = *(c->layout->face->ot_layout->gsub); const SubstLookup &l = gsub.get_lookup (lookup_index); if (unlikely (c->nesting_level_left == 0)) diff --git a/src/hb-ot-layout-private.hh b/src/hb-ot-layout-private.hh index e25f7ac87..723db4457 100644 --- a/src/hb-ot-layout-private.hh +++ b/src/hb-ot-layout-private.hh @@ -31,7 +31,7 @@ #include "hb-ot-layout.h" -#include "hb-font.h" +#include "hb-font-private.hh" #include "hb-buffer-private.hh" @@ -43,9 +43,7 @@ typedef unsigned int hb_ot_layout_class_t; * hb_ot_layout_t */ -typedef struct _hb_ot_layout_t hb_ot_layout_t; - -struct _hb_ot_layout_t +struct hb_ot_layout_t { hb_blob_t *gdef_blob; hb_blob_t *gsub_blob; @@ -62,8 +60,7 @@ struct _hb_ot_layout_t } new_gdef; }; -typedef struct _hb_ot_layout_context_t hb_ot_layout_context_t; -struct _hb_ot_layout_context_t +struct hb_ot_layout_context_t { hb_face_t *face; hb_font_t *font; @@ -77,14 +74,19 @@ struct _hb_ot_layout_context_t hb_position_t anchor_y; /* of the last valid glyph */ } gpos; } info; + + /* Convert from font-space to user-space */ + /* XXX div-by-zero */ + inline hb_position_t scale_x (int16_t v) { return (int64_t) this->font->x_scale * v / this->face->head_table->unitsPerEm; } + inline hb_position_t scale_y (int16_t v) { return (int64_t) this->font->y_scale * v / this->face->head_table->unitsPerEm; } }; -HB_INTERNAL void -_hb_ot_layout_init (hb_face_t *face); +HB_INTERNAL hb_ot_layout_t * +_hb_ot_layout_new (hb_face_t *face); HB_INTERNAL void -_hb_ot_layout_fini (hb_face_t *face); +_hb_ot_layout_free (hb_ot_layout_t *layout); /* diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 3ee031c21..82b56c8d6 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -39,12 +39,11 @@ #include -void -_hb_ot_layout_init (hb_face_t *face) +hb_ot_layout_t * +_hb_ot_layout_new (hb_face_t *face) { - hb_ot_layout_t *layout = &face->ot_layout; - - memset (layout, 0, sizeof (*layout)); + /* Remove this object altogether */ + hb_ot_layout_t *layout = (hb_ot_layout_t *) calloc (1, sizeof (hb_ot_layout_t)); layout->gdef_blob = Sanitizer::sanitize (hb_face_get_table (face, HB_OT_TAG_GDEF)); layout->gdef = Sanitizer::lock_instance (layout->gdef_blob); @@ -54,13 +53,13 @@ _hb_ot_layout_init (hb_face_t *face) layout->gpos_blob = Sanitizer::sanitize (hb_face_get_table (face, HB_OT_TAG_GPOS)); layout->gpos = Sanitizer::lock_instance (layout->gpos_blob); + + return layout; } void -_hb_ot_layout_fini (hb_face_t *face) +_hb_ot_layout_free (hb_ot_layout_t *layout) { - hb_ot_layout_t *layout = &face->ot_layout; - hb_blob_unlock (layout->gdef_blob); hb_blob_unlock (layout->gsub_blob); hb_blob_unlock (layout->gpos_blob); @@ -75,19 +74,19 @@ _hb_ot_layout_fini (hb_face_t *face) static const GDEF& _get_gdef (hb_face_t *face) { - return likely (face->ot_layout.gdef) ? *face->ot_layout.gdef : Null(GDEF); + return likely (face->ot_layout->gdef) ? *face->ot_layout->gdef : Null(GDEF); } static const GSUB& _get_gsub (hb_face_t *face) { - return likely (face->ot_layout.gsub) ? *face->ot_layout.gsub : Null(GSUB); + return likely (face->ot_layout->gsub) ? *face->ot_layout->gsub : Null(GSUB); } static const GPOS& _get_gpos (hb_face_t *face) { - return likely (face->ot_layout.gpos) ? *face->ot_layout.gpos : Null(GPOS); + return likely (face->ot_layout->gpos) ? *face->ot_layout->gpos : Null(GPOS); } @@ -106,7 +105,7 @@ hb_ot_layout_has_glyph_classes (hb_face_t *face) hb_bool_t _hb_ot_layout_has_new_glyph_classes (hb_face_t *face) { - return face->ot_layout.new_gdef.len > 0; + return face->ot_layout->new_gdef.len > 0; } static unsigned int @@ -118,8 +117,8 @@ _hb_ot_layout_get_glyph_property (hb_face_t *face, klass = gdef.get_glyph_class (glyph); - if (!klass && glyph < face->ot_layout.new_gdef.len) - klass = face->ot_layout.new_gdef.klasses[glyph]; + if (!klass && glyph < face->ot_layout->new_gdef.len) + klass = face->ot_layout->new_gdef.klasses[glyph]; switch (klass) { default: @@ -215,7 +214,7 @@ _hb_ot_layout_set_glyph_class (hb_face_t *face, /* TODO optimize this? similar to old harfbuzz code for example */ - hb_ot_layout_t *layout = &face->ot_layout; + hb_ot_layout_t *layout = face->ot_layout; hb_ot_layout_class_t gdef_klass; unsigned int len = layout->new_gdef.len; @@ -288,7 +287,7 @@ hb_ot_layout_build_glyph_classes (hb_face_t *face, if (HB_OBJECT_IS_INERT (face)) return; - hb_ot_layout_t *layout = &face->ot_layout; + hb_ot_layout_t *layout = face->ot_layout; if (unlikely (!count || !glyphs || !klasses)) return; diff --git a/src/hb-private.h b/src/hb-private.h index 6108d4056..f017bf9b5 100644 --- a/src/hb-private.h +++ b/src/hb-private.h @@ -158,10 +158,6 @@ _hb_popcount32 (uint32_t mask) } -/* Multiplies a 16dot16 value by another value, then truncates the result */ -#define _hb_16dot16_mul_round(A,B) (((int64_t) (A) * (B) + 0x8000) / 0x10000) - - /* We need external help for these */ #ifdef HAVE_GLIB