From ec118b8ba1c6bc5f639e22d115793d877b777228 Mon Sep 17 00:00:00 2001 From: Karl Date: Wed, 2 Sep 2020 09:12:57 +1200 Subject: [PATCH 1/3] Fix text baseline position for some fonts Some fonts (particularly Calibri or the metric-equivalent Carlito) were not aligned to the same baseline as on Windows. This is because fonts have three sets of roughly equivalent metrics, and different platforms use different ones. Pango uses one set, and Windows uses a different set. The one Windows uses has the line spacing included in the ascent and descent. The other two allow the gap to be specified separately, but the one Pango seems to use has normally got only a minimal gap. The problem is that Pango is aligning the top of the font according to just the ascent, and on fonts like the above where this is considerably different to the Windows ascent the text is positioned equivalently higher in the bounding box. --- src/text-pango.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/text-pango.c b/src/text-pango.c index 938a0d238..ff065328f 100644 --- a/src/text-pango.c +++ b/src/text-pango.c @@ -470,8 +470,22 @@ gdip_pango_setup_layout (cairo_t *cr, GDIPCONST WCHAR *stringUnicode, int length box_offset->Y = 0; switch (fmt->lineAlignment) { - case StringAlignmentNear: + case StringAlignmentNear: { + // Some fonts have interesting metrics, and cause Pango to position them differently to Windows. + // For instance, Calibri and the free metric-equivalent Carlito. It is positioned higher, because Pango + // uses a different ascent value than Windows. We can handle that by drawing it slightly lower. + uint16_t ascent; + uint16_t em_size; + if (GdipGetCellAscent(font->family, (INT) font->style, &ascent) == Ok && + GdipGetEmHeight(font->family, (INT) font->style, &em_size) == Ok) { + + int baseline = pango_layout_get_baseline(layout) / PANGO_SCALE; + int correct_baseline = (int) ceil(font->sizeInPixels / em_size * ascent); + if (baseline < correct_baseline) + box_offset->Y = correct_baseline - baseline; + } break; + } case StringAlignmentCenter: box_offset->Y += (FrameHeight - box->Height) * 0.5; break; From 428fda636b350b6fc02a72b43a3fc9631b6ee917 Mon Sep 17 00:00:00 2001 From: Karl Date: Thu, 3 Sep 2020 16:09:30 +1200 Subject: [PATCH 2/3] Fixes Don't adjust baseline on vertical text. It needs to be handled differently to horizontal, and has enough other problems that I don't want to start. Fixes test failure. Fixes the compile error that happened for Windows. Report the baseline offset separately to the box offset, because it needs to be handled differently for measurement etc. Basically when drawing it needs to make it draw lower (affecting position), but when measuring it needs to make the rectangle taller (affecting size but leaving the position unchanged). --- src/graphics-path.c | 5 +++-- src/text-pango-private.h | 2 +- src/text-pango.c | 45 +++++++++++++++++++++++----------------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/graphics-path.c b/src/graphics-path.c index b9b7b12e7..8a1b51f6d 100644 --- a/src/graphics-path.c +++ b/src/graphics-path.c @@ -1273,6 +1273,7 @@ GdipAddPathString (GpPath *path, GDIPCONST WCHAR *string, int length, { GpRectF box; GpPointF box_offset; + float baseline_offset; PangoLayout* layout; GpStringFormat *string_format; @@ -1296,8 +1297,8 @@ GdipAddPathString (GpPath *path, GDIPCONST WCHAR *string, int length, return status; } - layout = gdip_pango_setup_layout (cr, string, length, font, layoutRect, &box, &box_offset, string_format, NULL); - cairo_move_to (cr, layoutRect->X + box_offset.X, layoutRect->Y + box_offset.Y); + layout = gdip_pango_setup_layout (cr, string, length, font, layoutRect, &box, &box_offset, &baseline_offset, string_format, NULL); + cairo_move_to (cr, layoutRect->X + box_offset.X, layoutRect->Y + box_offset.Y + baseline_offset); pango_cairo_layout_path (cr, layout); g_object_unref (layout); diff --git a/src/text-pango-private.h b/src/text-pango-private.h index 1d8115d27..357af1c73 100644 --- a/src/text-pango-private.h +++ b/src/text-pango-private.h @@ -47,7 +47,7 @@ PangoLayout* gdip_pango_setup_layout (cairo_t *cr, GDIPCONST WCHAR *stringUnicode, INT length, GDIPCONST GpFont *font, - GDIPCONST RectF *rc, RectF *box, PointF *box_offset, GDIPCONST GpStringFormat *format, INT **charsRemoved); + GDIPCONST RectF *rc, RectF *box, PointF *box_offset, float *baseline_offset, GDIPCONST GpStringFormat *format, INT **charsRemoved); GpStatus pango_DrawString (GpGraphics *graphics, GDIPCONST WCHAR *stringUnicode, INT length, GDIPCONST GpFont *font, GDIPCONST RectF *rc, GDIPCONST GpStringFormat *format, GpBrush *brush) GDIP_INTERNAL; diff --git a/src/text-pango.c b/src/text-pango.c index ff065328f..b7be59fff 100644 --- a/src/text-pango.c +++ b/src/text-pango.c @@ -177,7 +177,7 @@ gdip_process_string (gchar *text, int length, int removeAccelerators, int trimSp PangoLayout* gdip_pango_setup_layout (cairo_t *cr, GDIPCONST WCHAR *stringUnicode, int length, GDIPCONST GpFont *font, - GDIPCONST RectF *rc, RectF *box, PointF *box_offset, GDIPCONST GpStringFormat *format, int **charsRemoved) + GDIPCONST RectF *rc, RectF *box, PointF *box_offset, float *baseline_offset, GDIPCONST GpStringFormat *format, int **charsRemoved) { GpStringFormat *fmt; PangoLayout *layout; @@ -468,24 +468,26 @@ gdip_pango_setup_layout (cairo_t *cr, GDIPCONST WCHAR *stringUnicode, int length box_offset->X = 0; box_offset->Y = 0; + *baseline_offset = 0; switch (fmt->lineAlignment) { - case StringAlignmentNear: { - // Some fonts have interesting metrics, and cause Pango to position them differently to Windows. - // For instance, Calibri and the free metric-equivalent Carlito. It is positioned higher, because Pango - // uses a different ascent value than Windows. We can handle that by drawing it slightly lower. - uint16_t ascent; - uint16_t em_size; - if (GdipGetCellAscent(font->family, (INT) font->style, &ascent) == Ok && - GdipGetEmHeight(font->family, (INT) font->style, &em_size) == Ok) { - - int baseline = pango_layout_get_baseline(layout) / PANGO_SCALE; - int correct_baseline = (int) ceil(font->sizeInPixels / em_size * ascent); - if (baseline < correct_baseline) - box_offset->Y = correct_baseline - baseline; + case StringAlignmentNear: + if (!(fmt->formatFlags & StringFormatFlagsDirectionVertical)) { + // Some fonts have interesting metrics, and cause Pango to position them differently to Windows. + // For instance, Calibri and the free metric-equivalent Carlito. It is positioned higher, because Pango + // uses a different ascent value than Windows. We can handle that by drawing it slightly lower. + USHORT ascent; + USHORT em_size; + if (GdipGetCellAscent(font->family, (INT) font->style, &ascent) == Ok && + GdipGetEmHeight(font->family, (INT) font->style, &em_size) == Ok) { + + int baseline = pango_layout_get_baseline(layout) / PANGO_SCALE; + int correct_baseline = (int) ceil(font->sizeInPixels / em_size * ascent); + if (baseline < correct_baseline) + *baseline_offset = correct_baseline - baseline; + } } break; - } case StringAlignmentCenter: box_offset->Y += (FrameHeight - box->Height) * 0.5; break; @@ -542,6 +544,7 @@ pango_DrawString (GpGraphics *graphics, GDIPCONST WCHAR *stringUnicode, INT leng PangoLayout *layout; RectF box; PointF box_offset; + float baseline_offset; /* Setup cairo */ if (brush) { @@ -552,7 +555,7 @@ pango_DrawString (GpGraphics *graphics, GDIPCONST WCHAR *stringUnicode, INT leng cairo_save (graphics->ct); - layout = gdip_pango_setup_layout (graphics->ct, stringUnicode, length, font, rc, &box, &box_offset, format, NULL); + layout = gdip_pango_setup_layout (graphics->ct, stringUnicode, length, font, rc, &box, &box_offset, &baseline_offset, format, NULL); if (!layout) { cairo_restore (graphics->ct); return OutOfMemory; @@ -566,7 +569,7 @@ pango_DrawString (GpGraphics *graphics, GDIPCONST WCHAR *stringUnicode, INT leng cairo_clip (graphics->ct); } - gdip_cairo_move_to (graphics, rc->X + box_offset.X, rc->Y + box_offset.Y, FALSE, TRUE); + gdip_cairo_move_to (graphics, rc->X + box_offset.X, rc->Y + box_offset.Y + baseline_offset, FALSE, TRUE); pango_cairo_show_layout (graphics->ct, layout); g_object_unref (layout); @@ -583,15 +586,17 @@ pango_MeasureString (GpGraphics *graphics, GDIPCONST WCHAR *stringUnicode, INT l PangoRectangle logical; PangoLayoutIter *iter; PointF box_offset; + float baseline_offset; int *charsRemoved = NULL; cairo_save (graphics->ct); - layout = gdip_pango_setup_layout (graphics->ct, stringUnicode, length, font, rc, boundingBox, &box_offset, format, &charsRemoved); + layout = gdip_pango_setup_layout (graphics->ct, stringUnicode, length, font, rc, boundingBox, &box_offset, &baseline_offset, format, &charsRemoved); if (!layout) { cairo_restore (graphics->ct); return OutOfMemory; } + boundingBox->Height += baseline_offset; if (codepointsFitted || linesFilled) { int charsFitted; @@ -692,6 +697,7 @@ pango_MeasureCharacterRanges (GpGraphics *graphics, GDIPCONST WCHAR *stringUnico int i, j; GpRectF boundingBox; GpPointF box_offset; + float baseline_offset; if (layoutRect->Width <= 0.0 && layoutRect->Height < 0.0) { /* special case only if BOTH values are negative */ @@ -702,7 +708,7 @@ pango_MeasureCharacterRanges (GpGraphics *graphics, GDIPCONST WCHAR *stringUnico cairo_save (graphics->ct); - layout = gdip_pango_setup_layout (graphics->ct, stringUnicode, length, font, layoutRect, &boundingBox, &box_offset, format, NULL); + layout = gdip_pango_setup_layout (graphics->ct, stringUnicode, length, font, layoutRect, &boundingBox, &box_offset, &baseline_offset, format, NULL); if (!layout) { cairo_restore (graphics->ct); return OutOfMemory; @@ -764,6 +770,7 @@ pango_MeasureCharacterRanges (GpGraphics *graphics, GDIPCONST WCHAR *stringUnico } charRect.X += box_offset.X + layoutRect->X; charRect.Y += box_offset.Y + layoutRect->Y; + charRect.Height += baseline_offset; // g_warning ("[%d] [%d : %d-%d] %c [x %g y %g w %g h %g]", i, j, start, end, (char)stringUnicode[j], charRect.X, charRect.Y, charRect.Width, charRect.Height); status = GdipCombineRegionRect (regions [i], &charRect, CombineModeUnion); if (status != Ok) From 6b5f2265322aa7a46870e8d66e3521935efc3cb0 Mon Sep 17 00:00:00 2001 From: Karl Date: Thu, 3 Sep 2020 16:18:23 +1200 Subject: [PATCH 3/3] Add test --- tests/testtext.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/testtext.c b/tests/testtext.c index 32853d3de..1a749e7f1 100644 --- a/tests/testtext.c +++ b/tests/testtext.c @@ -50,6 +50,12 @@ static void test_measure_string(void) int glyphs; int lines; const SHORT fontSize = 10; + GpFontCollection *collection; + WCHAR *ttfFile; + INT count; + GpFontFamily *families[1]; + GpFont *font2; + REAL fontHeight; status = GdipCreateStringFormat (0, 0, &format); expect (Ok, status); @@ -179,6 +185,34 @@ static void test_measure_string(void) expect (1, lines); expectf ((double)fontSize, bounds.Width); // An em-space should be the same width as the font size. + // Check measuring a string drawn with a font that has metrics which cause Pango to draw differently to GDI+ + GdipNewPrivateFontCollection (&collection); + ttfFile = createWchar ("test.ttf"); + status = GdipPrivateAddFontFile (collection, ttfFile); + expect (Ok, status); + status = GdipGetFontCollectionFamilyList (collection, 2, families, &count); + expect (Ok, status); + expect (1, count); + status = GdipCreateFont (families[0], 36, 0, UnitPixel, &font2); + expect (Ok, status); + status = GdipGetFontHeight (font2, graphics, &fontHeight); + expect (Ok, status); + set_rect_empty (&bounds); + status = GdipMeasureString (graphics, teststring1, 1, font2, &rect, format, &bounds, &glyphs, &lines); + expect (Ok, status); + expectf (5.0, bounds.X); + expectf (5.0, bounds.Y); + expectf_ (20.0, bounds.Width, 3); + expectf_ (fontHeight, bounds.Height, 3); + expect (1, glyphs); + expect (1, lines); + GdipDeleteFont(font2); + // This causes a crash in GDI+. +#if !defined(USE_WINDOWS_GDIPLUS) + GdipDeleteFontFamily (families[0]); +#endif + GdipDeletePrivateFontCollection (&collection); + // MonoTests.System.Drawing.GraphicsTest.MeasureString_Wrapping_Dots GdipDeleteStringFormat (format); status = GdipCreateStringFormat (0, 0, &format);