Fix out of range exception in typographic utils.

[JimBobSquarePants]

Prerequisites

• [x] I have written a descriptive pull-request title
• [x] I have verified that there are no overlapping pull-requests open
• [x] I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:.
• [ ] I have provided test coverage for my change (where applicable)

Description

Fixes #298 Fixes #302

Also fixes a layout issue for marks I spotted when rendering the test text where we were moving them vertically in the wrong direction. Line rendering was broken also.

@tocsoft I noticed something a bit weird with the first sample where the top of the text is slightly cut off. This is due to the min Y bounds having a negative value (read from the glyph entry and not altered).

I haven't attempted a workaround as I'm not sure what the best approach would be. I could maybe capture the min Y when breaking lines and offset accordingly.

[codecov[bot]]

Codecov Report

Merging #303 (478574b) into main (f42190c) will increase coverage by 0%. The diff coverage is 15%.

@@          Coverage Diff          @@
##            main    #303   +/-   ##
=====================================
  Coverage     83%     83%           
=====================================
  Files        222     222           
  Lines      12276   12281    +5     
  Branches    1781    1784    +3     
=====================================
+ Hits       10253   10272   +19     
+ Misses      1591    1585    -6     
+ Partials     432     424    -8     

Flag	Coverage Δ
unittests	83% <15%> (+<1%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/SixLabors.Fonts/GlyphMetrics.cs	50% <0%> (-2%)	:arrow_down:
...es/AdvancedTypographic/AdvancedTypographicUtils.cs	92% <100%> (+3%)	:arrow_up:
...bors.Fonts/Tables/TrueType/TrueTypeGlyphMetrics.cs	89% <100%> (ø)
src/SixLabors.Fonts/TextMeasurer.cs	84% <0%> (+1%)	:arrow_up:
...es/AdvancedTypographic/GSub/LookupType6SubTable.cs	90% <0%> (+2%)	:arrow_up:
...es/AdvancedTypographic/GSub/LookupType4SubTable.cs	78% <0%> (+3%)	:arrow_up:
...nts/Tables/TrueType/Glyphs/CompositeGlyphLoader.cs	100% <0%> (+10%)	:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[JimBobSquarePants]

System.Drawing clips that particular text also.

Note the use of StringFormat.GenericTypographic to prevent the addition of leading and end spacing of 1/6em.

using Font font = new Font("Arial", 72F);
using Graphics graphics = Graphics.FromHwnd(IntPtr.Zero);
graphics.PageUnit = GraphicsUnit.Point;
SizeF size = graphics.MeasureString(input, font, Point.Empty, StringFormat.GenericTypographic).Dump();

using Bitmap image = new Bitmap((int)Math.Ceiling(size.Width), (int)Math.Ceiling(size.Height));
image.SetResolution(72F, 72F);
using Graphics graphics2 = Graphics.FromImage(image);
graphics2.PageUnit = GraphicsUnit.Point;
graphics2.Clear(Color.Black);

using Brush brush = new SolidBrush(Color.White);
graphics2.DrawString(input, font, brush, Point.Empty, StringFormat.GenericTypographic);
image.Save(@"C:\Users\james\Desktop\sd3.png", ImageFormat.Png);

I'm not going to waste time attempting to handle it.

[mhmd-azeez]

Hello @JimBobSquarePants, if you don't mind giving me a rough outline of how to test these changes, I will try to test it on a couple of Kurdish fonts.

[JimBobSquarePants]

Thanks @mhmd-azeez The best thing to do is to use the sample project as a scratchpad to test out a few things. I'm just writing a test at the moment that will at least ensure coverage of these changes.

[mhmd-azeez]

I ran the sample on main and got a System.ArgumentOutOfRangeException in GlyphSubstitutionCollection.GetGlyphShapingData for "فِيلْمٌ" with Arial and "بِسْمِ ٱللَّهِ ٱلرَّحْمَٟنِ ٱلرَّحِيمِ" using Segoe UI (and Tahoma). I ran the code again on this branch and didn't get any exceptions. I also looked at the output folder and it seems to be correct:

But my arial is different from your Arial, here is how it's rendered in notepad:

I also tried out a couple of other fonts:

FontFamily arabic2 = SystemFonts.Get("Noto Naskh Arabic");
FontFamily arabic3 = SystemFonts.Get("Vazirmatn");

RenderText(arabic2, "دۆ دێرین چیا گڤ ڕەنگین", pointSize: 20);
RenderText(arabic3, "دۆ دێرین چیا گڤ ڕەنگین", pointSize: 20);

Which are also properly rendered: Vazirmatn:

Noto Naskh Arabic

However, Noto Naskh's rendering of "بِسْمِ ٱللَّهِ ٱلرَّحْمَٟنِ ٱلرَّحِيمِ" is not ideal:

This is how it's rendered in notepad:

[JimBobSquarePants]

However, Noto Naskh's rendering of "بِسْمِ ٱللَّهِ ٱلرَّحْمَٟنِ ٱلرَّحِيمِ" is not ideal:

I'll definitely have to investigate that. Thanks!

[JimBobSquarePants]

@mhmd-azeez I got a copy of Noto Naskh Arabic from Google Fonts and rendered it using this branch. Here's my output. Are you sure you had the right branch running in your final test?

[mhmd-azeez]

@JimBobSquarePants I just ran the samples again and you're right. Yesterday I went back and forth between main and js/issue-302 a lot and ran the sample to confirm the fix. That image I posted was from main.

main:

js/issue-302:

It seems like this PR solves that problem was well. Sorry for sending you on a wild goose chase.

[JimBobSquarePants]

Sorry for sending you on a wild goose chase.

No worries at all! I'm gonna merge this then. Thanks for your help!