Add parsing anchor table format 2 #238
Conversation
Codecov Report
@@ Coverage Diff @@
## main #238 +/- ##
===================================
- Coverage 84% 83% -1%
===================================
Files 182 183 +1
Lines 8938 8985 +47
Branches 1420 1425 +5
===================================
+ Hits 7531 7542 +11
- Misses 1083 1118 +35
- Partials 324 325 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
We actually do, it's hidden behind a property in the I think the provided index represents the index of the point in the I had a quick look at |
This looks about right. To really get this correct, I think we need a real world example where this feature is actually used. The one example from microsoft docs does not do it for me. I wish they would provide fonts along those examples. |
Completely agree, the savings in development time would be incredible! I think we should go ahead and grab the point from the contour points when possible. I'd have to double check to see if we should be scaling the value like Harfbuzz does. I'm surprised they don't have a test font squirreled away somewhere. |
| @@ -45,7 +45,8 @@ public static AnchorTable Load(BigEndianBinaryReader reader, long offset) | |||
| return anchorFormat switch | |||
| { | |||
| 1 => AnchorFormat1.Load(reader), | |||
| _ => throw new NotSupportedException($"anchorFormat {anchorFormat} not supported. Should be '1'.") | |||
| 2 => AnchorFormat2.Load(reader), | |||
There was a problem hiding this comment.
As somebody who knows nothing about fonts, making these error messages more descriptive might be helpful too. If I knew that my font format wasn't supported, I'd just pick a different one.
3 => throw new NotSupportedException($"Variable fonts (format 3) are not supported. Read: https://docs.sixlabors.com/articles/fonts/anchorTableFormats.html")
_ => throw new NotSupportedException($"Fonts using anchor table format {anchorFormat} are not supported. Read: https://docs.sixlabors.com/articles/fonts/anchorTableFormats.html")
There was a problem hiding this comment.
I wouldn't link to docs from exception messages. The maintenance burden of tracking URL changes would be too much.
There was a problem hiding this comment.
that's fair enough, was just a thought
|
@brianpopow I decided to actually go ahead and implement anchor format 2 table as I'm fairly confident of the approach. I've also added the font to the solution so we can test it. I've been trying various character combinations to see if I can trigger the table but have so far failed. |
I tried to look through the harfbuzz tests for examples, but so far also without luck. I am pretty sure there must be an example there, but dont know how to find one. |
@behdad @khaledhosny Would you possibly be able to help here? We're looking for example fonts and tests that allow us to test features using AnchorTable Formats 2 and 3. |
I'm not aware of any fonts using those currently. |
|
Thanks @behdad at least this means if we make a mistake, no one will see it 😂 |
|
@brianpopow I'm gonna merge this as-is since we can't provide a test font. I'll add an issue to flesh out |
Prerequisites
Description
This adds parsing Anchor table format 2.
The difference to format 1 is, that there is a additional Index to glyph contour point, which should be used for hinting.
We dont support hinting yet, right? The output looks correct from what I can tell without it.
closes #237