Bidi + Font Shaping Support #198
Conversation
Codecov Report
@@ Coverage Diff @@
## master #198 +/- ##
==========================================
+ Coverage 87.29% 87.34% +0.05%
==========================================
Files 99 177 +78
Lines 4651 7897 +3246
Branches 764 1240 +476
==========================================
+ Hits 4060 6898 +2838
- Misses 473 722 +249
- Partials 118 277 +159
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Hey @tocsoft !! I got combined emojis working
|
src/SixLabors.Fonts/Tables/AdvancedTypographic/Gsub/LookupType5SubTable.cs
Outdated
Show resolved
Hide resolved
|
There's gaps in some of the GSUB table support which I could do with help with completing. Lookups 5,6, & 8 which I'm sure someone with an understanding of C++ or Rust would be able to flesh out. Complete implementations can be found here. |
tests/SixLabors.Fonts.Tests/Tables/AdvancedTypographic/Gsub/GSubTableTests.cs
Outdated
Show resolved
Hide resolved
tests/SixLabors.Fonts.Tests/Tables/AdvancedTypographic/Gsub/GSubTableTests.cs
Outdated
Show resolved
Hide resolved
|
@mhmd-azeez has kindly volunteered to help keep us right with the Arabic shaping output so if you have any questions just ping him. |
|
Here are some first rendering results for text written from right to left:
@mhmd-azeez does this look correct to you? |
Text needs to be itemized so each script run can be shaped separately. That also comes into play when you enable certain font features for a portion of the text. |
JimBobSquarePants
changed the title
|
@SixLabors/core I forgot to change the title but this is very much "ready to review" |
|
|
|
Thanks @Gillibald just a bad copy/paste will fix. We’re definitely doing the right thing |
|
You did a really great job here. So impressive. Hopefully, this will be adopted by many projects so this can improve even further. .Net is used everywhere so there should be first-class text processing without any extra native dependencies. Your work is the first starting point to make that possible. |
| return; | ||
| // TODO: Test and fix tomorrow. Gsub Lookup 4. |
There was a problem hiding this comment.
Ignore this file. It's become a bit of a dumping ground for experiments.
Shouldn't we redeclare samples/DrawWithImageSharp as tests/SandboxDrawWithImageSharp then?
There was a problem hiding this comment.
Yeah. I'll likely delete most of it during a cleanup though.
| @@ -3,7 +3,6 @@ | |||
|
|
|||
| using System; | |||
| using System.Collections.Generic; | |||
| using System.Linq; | |||
There was a problem hiding this comment.
Not related to the PR, but this type is confusing in context of our other libraries. Opened #212 and SixLabors/ImageSharp.Drawing#174
| /// <param name="text">The text.</param> | ||
| /// <param name="options">The style.</param> | ||
| /// <returns>A collection of layout that describe all that's needed to measure or render a series of glyphs.</returns> | ||
| internal static TextLayout Default { get; set; } = new(); |
There was a problem hiding this comment.
Do we expect this type to be inheritable and have other implementations than Default?
There was a problem hiding this comment.
It's an odd one. TextMeasurer accepts the instance as a parameter and stores it as a private field. Not sure whether @tocsoft had plans to allow replacing parts.
src/SixLabors.Fonts/Tables/AdvancedTypographic/CoverageTable.cs
Outdated
Show resolved
Hide resolved
| return anchorFormat switch | ||
| { | ||
| 1 => AnchorFormat1.Load(reader), | ||
| _ => throw new NotSupportedException($"anchorFormat {anchorFormat} not supported. Should be '1'.") |
There was a problem hiding this comment.
Do we really need the abstraction and the type hierarchy here? Can't AnchorTable be a struct instead to make things faster?
| /// The order of the records also corresponds to the writing direction — that is, the logical direction — of the text. | ||
| /// For text written left to right, the first component is on the left; for text written right to left, the first component is on the right. | ||
| /// </summary> | ||
| internal class LigatureAttachTable |
There was a problem hiding this comment.
I see a bunch of simple ***Table, and similar classes:
- Are not subclassed
- Are small
- Stored in arrays after loading.
Most of them seem to be good candidates to be turned into structs.
LigatureAttachTable, ComponentRecord, LookupTable, NameRecord ...
There was a problem hiding this comment.
Yeah, I'll have a look.
There was a problem hiding this comment.
Gonna leave for now. I'd like to get this in and focus on potential performance issues after.
| { | ||
| internal struct AttachPoint | ||
| { | ||
| public ushort[] PointIndices; |
There was a problem hiding this comment.
Oooh! I'll check this out!
There was a problem hiding this comment.
Ah yeah, this has something to do with font variations which we don't support yet.
| /// Kerning is the contextual adjustment of inter-glyph spacing. | ||
| /// This property controls metric kerning, kerning that utilizes adjustment data contained in the font. | ||
| /// </summary> | ||
| public enum KerningMode |
Prerequisites
Description
Implementation of bidi layout support + glyph shaping. Includes a complete rewrite of the layout engine.
Information and reference code adapted from
https://docs.microsoft.com/en-us/typography/opentype/spec/
https://github.com/harfbuzz/harfbuzz
https://github.com/foliojs/fontkit
https://github.com/LayoutFarm/Typography
Fixes #150, Fixes #66
Also provides the basic for supporting #8, and #9
Implemented features.
Example Output
Chinese Horizontal
Chinese Vertical (columns are read right to left; the text direction is top to bottom)
Combined emoji (4 scalar values and 7 chars)
Arabic with complex ligatures
Mixed English and Arabic.
Text alignment
Word wrapping (normal)
Word wrapping (break all)
Word wrapping (keep all)