external/harfbuzz/0001-graphite-Fix-shaping-with-varying-font-size.patch | 130 ---------- external/harfbuzz/UnpackedTarball_harfbuzz.mk | 1 vcl/inc/CommonSalLayout.hxx | 2 vcl/source/gdi/CommonSalLayout.cxx | 67 ++--- 4 files changed, 33 insertions(+), 167 deletions(-)
New commits: commit e31f7f4c87d5501599daa8d11d08b6e4a8725356 Author: Khaled Hosny <khaledho...@eglug.org> Date: Tue Nov 1 02:15:23 2016 +0200 tdf#103403: Wrong glyph advances with Graphite Always create HarfBuzz font at the UPEM size and scale HarfBuzz output with the desired size instead. This theoretically means we loss any size-specific adjustments in the font but in practice very few fonts do this and in general modern APIs prefer stable glyph positioning across font sizes. Change-Id: Idf396eec5e241cc5fb9d0db698f2c081b7de29e3 diff --git a/vcl/inc/CommonSalLayout.hxx b/vcl/inc/CommonSalLayout.hxx index eb2e3cd..c42f520 100644 --- a/vcl/inc/CommonSalLayout.hxx +++ b/vcl/inc/CommonSalLayout.hxx @@ -55,7 +55,7 @@ class CommonSalLayout : public GenericSalLayout OString msLanguage; std::vector<hb_feature_t> maFeatures; - hb_font_t* getHbFont(); + void getScale(double* nXScale, double* nYScale); public: #if defined(_WIN32) diff --git a/vcl/source/gdi/CommonSalLayout.cxx b/vcl/source/gdi/CommonSalLayout.cxx index e7f2826..6ba8cb6 100644 --- a/vcl/source/gdi/CommonSalLayout.cxx +++ b/vcl/source/gdi/CommonSalLayout.cxx @@ -80,31 +80,28 @@ static hb_blob_t* getFontTable(hb_face_t* /*face*/, hb_tag_t nTableTag, void* pU static hb_font_t* createHbFont(hb_face_t* pHbFace) { hb_font_t* pHbFont = hb_font_create(pHbFace); + unsigned int nUPEM = hb_face_get_upem(pHbFace); + hb_font_set_scale(pHbFont, nUPEM, nUPEM); hb_ot_font_set_funcs(pHbFont); + hb_face_destroy(pHbFace); + return pHbFont; } -// We cache and re-use the HarfBuzz font for different layout instances, so we -// need sure to set the correct scale (font size) before using the font. -hb_font_t* CommonSalLayout::getHbFont() +void CommonSalLayout::getScale(double* nXScale, double* nYScale) { - unsigned int nXScale = mrFontSelData.mnWidth << 6; - unsigned int nYScale = mrFontSelData.mnHeight << 6; - -#if defined(_WIN32) - // HACK to get stretched/shrunken text. TODO: Get rid of HACK - if (nXScale) - nXScale = double(nXScale) * 1.812; -#endif + hb_face_t* pHbFace = hb_font_get_face(mpHbFont); + unsigned int nUPEM = hb_face_get_upem(pHbFace); - if (!nXScale) - nXScale = nYScale; + double nHeight(mrFontSelData.mnHeight); + double nWidth(mrFontSelData.mnWidth ? mrFontSelData.mnWidth : nHeight); - hb_font_set_ppem(mpHbFont, nXScale, nYScale); - hb_font_set_scale(mpHbFont, nXScale, nYScale); + if (nYScale) + *nYScale = nHeight / nUPEM; - return mpHbFont; + if (nXScale) + *nXScale = nWidth / nUPEM; } #if !HB_VERSION_ATLEAST(1, 1, 0) @@ -182,8 +179,6 @@ CommonSalLayout::CommonSalLayout(HDC hDC, WinFontInstance& rWinFontInstance, con mpHbFont = createHbFont(pHbFace); rWinFontFace.SetHbFont(mpHbFont); - - hb_face_destroy(pHbFace); } } @@ -206,8 +201,6 @@ CommonSalLayout::CommonSalLayout(const CoreTextStyle& rCoreTextStyle) mpHbFont = createHbFont(pHbFace); rCoreTextStyle.SetHbFont(mpHbFont); - - hb_face_destroy(pHbFace); } } @@ -223,8 +216,6 @@ CommonSalLayout::CommonSalLayout(FreetypeFont& rFreetypeFont) mpHbFont = createHbFont(pHbFace); mrFreetypeFont.SetHbFont(mpHbFont); - - hb_face_destroy(pHbFace); } } #endif @@ -370,8 +361,7 @@ static int GetVerticalFlagsForScript(UScriptCode aScript) bool CommonSalLayout::LayoutText(ImplLayoutArgs& rArgs) { - hb_font_t* pHbFont = getHbFont(); - hb_face_t* pHbFace = hb_font_get_face(pHbFont); + hb_face_t* pHbFace = hb_font_get_face(mpHbFont); hb_script_t aHbScript = HB_SCRIPT_INVALID; int nGlyphCapacity = 2 * (rArgs.mnEndCharPos - rArgs.mnMinCharPos); @@ -401,6 +391,10 @@ bool CommonSalLayout::LayoutText(ImplLayoutArgs& rArgs) ParseFeatures(mrFontSelData.maTargetName); + double nXScale = 0; + double nYScale = 0; + getScale(&nXScale, &nYScale); + Point aCurrPos(0, 0); while (true) { @@ -485,7 +479,7 @@ bool CommonSalLayout::LayoutText(ImplLayoutArgs& rArgs) hb_segment_properties_t aHbProps; hb_buffer_get_segment_properties(pHbBuffer, &aHbProps); hb_shape_plan_t* pHbPlan = hb_shape_plan_create_cached(pHbFace, &aHbProps, maFeatures.data(), maFeatures.size(), pHbShapers); - bool ok = hb_shape_plan_execute(pHbPlan, pHbFont, pHbBuffer, maFeatures.data(), maFeatures.size()); + bool ok = hb_shape_plan_execute(pHbPlan, mpHbFont, pHbBuffer, maFeatures.data(), maFeatures.size()); assert(ok); (void) ok; hb_buffer_set_content_type(pHbBuffer, HB_BUFFER_CONTENT_TYPE_GLYPHS); @@ -543,7 +537,7 @@ bool CommonSalLayout::LayoutText(ImplLayoutArgs& rArgs) if (bDiacritic) nGlyphFlags |= GlyphItem::IS_DIACRITIC; - int32_t nAdvance, nXOffset, nYOffset; + DeviceCoordinate nAdvance, nXOffset, nYOffset; if (bVertical) { int nVertFlag; @@ -556,15 +550,15 @@ bool CommonSalLayout::LayoutText(ImplLayoutArgs& rArgs) nVertFlag = GF_ROTL; #endif nGlyphIndex |= nVertFlag; - nAdvance = -pHbPositions[i].y_advance >> 6; - nXOffset = pHbPositions[i].y_offset >> 6; - nYOffset = -pHbPositions[i].x_offset >> 6; + nAdvance = -pHbPositions[i].y_advance * nYScale; + nXOffset = pHbPositions[i].y_offset * nYScale; + nYOffset = -pHbPositions[i].x_offset * nXScale; } else { - nAdvance = pHbPositions[i].x_advance >> 6; - nXOffset = pHbPositions[i].x_offset >> 6; - nYOffset = pHbPositions[i].y_offset >> 6; + nAdvance = pHbPositions[i].x_advance * nXScale; + nXOffset = pHbPositions[i].x_offset * nXScale; + nYOffset = pHbPositions[i].y_offset * nYScale; } Point aNewPos = Point(aCurrPos.X() + nXOffset, -(aCurrPos.Y() + nYOffset)); @@ -649,9 +643,12 @@ void CommonSalLayout::ApplyDXArray(ImplLayoutArgs& rArgs) if (rArgs.mnFlags & SalLayoutFlags::KashidaJustification) { // Find Kashida glyph width and index. - hb_font_t* pHbFont = getHbFont(); - if (hb_font_get_glyph(pHbFont, 0x0640, 0, &nKashidaIndex)) - nKashidaWidth = hb_font_get_glyph_h_advance(pHbFont, nKashidaIndex) / 64; + if (hb_font_get_glyph(mpHbFont, 0x0640, 0, &nKashidaIndex)) + { + double nXScale = 0; + getScale(&nXScale, nullptr); + nKashidaWidth = hb_font_get_glyph_h_advance(mpHbFont, nKashidaIndex) * nXScale; + } bKashidaJustify = nKashidaWidth != 0; } commit 86abe3cb3d4b06ffecced691829c15ba90d00937 Author: Khaled Hosny <khaledho...@eglug.org> Date: Tue Nov 1 02:14:17 2016 +0200 Revert "tdf#103403: Wrong glyph advances with Graphite" This reverts commit 3d83c42008ab51202c0577f493e8ed3fde0310b7. A simpler fix in the next commit. diff --git a/external/harfbuzz/0001-graphite-Fix-shaping-with-varying-font-size.patch b/external/harfbuzz/0001-graphite-Fix-shaping-with-varying-font-size.patch deleted file mode 100644 index f9e6afc..0000000 --- a/external/harfbuzz/0001-graphite-Fix-shaping-with-varying-font-size.patch +++ /dev/null @@ -1,130 +0,0 @@ -From 82f7be388090f19333876877366907fec09f0312 Mon Sep 17 00:00:00 2001 -From: Khaled Hosny <khaledho...@eglug.org> -Date: Sun, 30 Oct 2016 20:16:41 +0200 -Subject: [PATCH] [graphite] Fix shaping with varying font size - -See https://bugs.documentfoundation.org/show_bug.cgi?id=103403#c7 ---- - src/hb-graphite2.cc | 40 ++++++++++++++++++++++++++++------------ - 1 file changed, 28 insertions(+), 12 deletions(-) - -diff --git a/src/hb-graphite2.cc b/src/hb-graphite2.cc -index c32318d..138a5ae 100644 ---- src/hb-graphite2.cc -+++ src/hb-graphite2.cc -@@ -27,7 +27,6 @@ - */ - - #define HB_SHAPER graphite2 --#define hb_graphite2_shaper_font_data_t gr_font - #include "hb-shaper-impl-private.hh" - - #include "hb-graphite2.h" -@@ -55,6 +54,12 @@ struct hb_graphite2_shaper_face_data_t { - hb_graphite2_tablelist_t *tlist; - }; - -+struct hb_graphite2_shaper_font_data_t { -+ gr_font *grfont; -+ int xscale; -+ int yscale; -+}; -+ - static const void *hb_graphite2_get_table (const void *data, unsigned int tag, size_t *len) - { - hb_graphite2_shaper_face_data_t *face_data = (hb_graphite2_shaper_face_data_t *) data; -@@ -166,13 +171,20 @@ _hb_graphite2_shaper_font_data_create (hb_font_t *font) - hb_face_t *face = font->face; - hb_graphite2_shaper_face_data_t *face_data = HB_SHAPER_DATA_GET (face); - -- return gr_make_font_with_advance_fn (font->x_scale, font, &hb_graphite2_get_advance, face_data->grface); -+ hb_graphite2_shaper_font_data_t *data = (hb_graphite2_shaper_font_data_t *) calloc (1, sizeof (hb_graphite2_shaper_font_data_t)); -+ data->grfont = gr_make_font_with_advance_fn (font->x_scale, font, &hb_graphite2_get_advance, face_data->grface); -+ data->xscale = font->x_scale; -+ data->yscale = font->y_scale; -+ -+ return data; - } - - void - _hb_graphite2_shaper_font_data_destroy (hb_graphite2_shaper_font_data_t *data) - { -- gr_font_destroy (data); -+ gr_font_destroy (data->grfont); -+ -+ free (data); - } - - /* -@@ -182,7 +194,7 @@ gr_font * - hb_graphite2_font_get_gr_font (hb_font_t *font) - { - if (unlikely (!hb_graphite2_shaper_font_data_ensure (font))) return NULL; -- return HB_SHAPER_DATA_GET (font); -+ return HB_SHAPER_DATA_GET (font)->grfont; - } - - -@@ -228,7 +240,7 @@ _hb_graphite2_shape (hb_shape_plan_t *shape_plan, - { - hb_face_t *face = font->face; - gr_face *grface = HB_SHAPER_DATA_GET (face)->grface; -- gr_font *grfont = HB_SHAPER_DATA_GET (font); -+ gr_font *grfont = HB_SHAPER_DATA_GET (font)->grfont; - - const char *lang = hb_language_to_string (hb_buffer_get_language (buffer)); - const char *lang_end = lang ? strchr (lang, '-') : NULL; -@@ -371,7 +383,11 @@ _hb_graphite2_shape (hb_shape_plan_t *shape_plan, - } - buffer->len = glyph_count; - -- float yscale = font->y_scale / font->x_scale; -+ hb_graphite2_shaper_font_data_t* font_data = HB_SHAPER_DATA_GET (font); -+ -+ float xscale = (float) font->x_scale / (float) font_data->xscale; -+ float yscale = (float) font->y_scale / (float) font_data->yscale; -+ yscale *= yscale / xscale; - /* Positioning. */ - if (!HB_DIRECTION_IS_BACKWARD(buffer->props.direction)) - { -@@ -381,10 +397,10 @@ _hb_graphite2_shape (hb_shape_plan_t *shape_plan, - curradvx = 0; - for (is = gr_seg_first_slot (seg); is; pPos++, ++info, is = gr_slot_next_in_segment (is)) - { -- pPos->x_offset = gr_slot_origin_X (is) - curradvx; -+ pPos->x_offset = gr_slot_origin_X (is) * xscale - curradvx; - pPos->y_offset = gr_slot_origin_Y (is) * yscale - curradvy; - if (info->cluster != currclus) { -- pPos->x_advance = info->var1.i32; -+ pPos->x_advance = info->var1.i32 * xscale; - curradvx += pPos->x_advance; - currclus = info->cluster; - } else -@@ -399,20 +415,20 @@ _hb_graphite2_shape (hb_shape_plan_t *shape_plan, - int currclus = -1; - const hb_glyph_info_t *info = buffer->info; - hb_glyph_position_t *pPos = hb_buffer_get_glyph_positions (buffer, NULL); -- curradvx = gr_seg_advance_X(seg); -+ curradvx = gr_seg_advance_X(seg) * xscale; - for (is = gr_seg_first_slot (seg); is; pPos++, info++, is = gr_slot_next_in_segment (is)) - { - if (info->cluster != currclus) - { -- pPos->x_advance = info->var1.i32; -- if (currclus != -1) curradvx -= info[-1].var1.i32; -+ pPos->x_advance = info->var1.i32 * xscale; -+ if (currclus != -1) curradvx -= info[-1].var1.i32 * xscale; - currclus = info->cluster; - } else - pPos->x_advance = 0.; - - pPos->y_advance = gr_slot_advance_Y (is, grface, grfont) * yscale; - curradvy -= pPos->y_advance; -- pPos->x_offset = gr_slot_origin_X (is) - curradvx + pPos->x_advance; -+ pPos->x_offset = gr_slot_origin_X (is) * xscale - curradvx + pPos->x_advance; - pPos->y_offset = gr_slot_origin_Y (is) * yscale - curradvy; - } - hb_buffer_reverse_clusters (buffer); --- -2.10.1 - diff --git a/external/harfbuzz/UnpackedTarball_harfbuzz.mk b/external/harfbuzz/UnpackedTarball_harfbuzz.mk index ed4a12c..7d408d2 100644 --- a/external/harfbuzz/UnpackedTarball_harfbuzz.mk +++ b/external/harfbuzz/UnpackedTarball_harfbuzz.mk @@ -16,7 +16,6 @@ $(eval $(call gb_UnpackedTarball_set_patchlevel,harfbuzz,0)) $(eval $(call gb_UnpackedTarball_add_patches,harfbuzz, \ external/harfbuzz/ubsan.patch \ external/harfbuzz/clang-cl.patch \ - external/harfbuzz/0001-graphite-Fix-shaping-with-varying-font-size.patch \ )) ifneq ($(ENABLE_RUNTIME_OPTIMIZATIONS),TRUE) _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits