Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support text runs. #251

Merged
merged 52 commits into from
Mar 26, 2022
Merged

Support text runs. #251

merged 52 commits into from
Mar 26, 2022

Conversation

JimBobSquarePants
Copy link
Member

@JimBobSquarePants JimBobSquarePants commented Mar 2, 2022

Prerequisites

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

Description

Adds initial support for individual text runs within a text shaping run. Fixes #10

TODO:

  • Fix Font size application to individual metrics.
  • Capture subscript/superscript/strikethrough metrics for attributes from OS2 table so we can render.
  • Capture underline metrics for attributes from Post table so we can render.
  • Implement rendering in GlyphMetrics

Example API for setting TextRun properties. Start and End properties are measured in graphemes to avoid slicing up emoji and such like.

var textRuns = new List<TextRun>
{
    new TextRun { Start = 4, End = 9, Font = uiFont.CreateFont(32, FontStyle.Bold), TextAttributes = TextAttribute.Underline | TextAttribute.Strikethrough },
    new TextRun { Start = 26, End = 30, Font = uiFont.CreateFont(32, FontStyle.Italic) }
};

Example Output.

The quick brown fox jumps over the lazy dog

@codecov
Copy link

codecov bot commented Mar 2, 2022

Codecov Report

Merging #251 (7b3ec08) into main (388a24f) will decrease coverage by 0%.
The diff coverage is 84%.

@@         Coverage Diff          @@
##           main   #251    +/-   ##
====================================
- Coverage    83%    83%    -1%     
====================================
  Files       183    186     +3     
  Lines      8985   9430   +445     
  Branches   1425   1478    +53     
====================================
+ Hits       7542   7902   +360     
- Misses     1118   1187    +69     
- Partials    325    341    +16     
Flag Coverage Δ
unittests 83% <84%> (-1%) ⬇️

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

Impacted Files Coverage Δ
src/SixLabors.Fonts/FontMetrics.cs 100% <ø> (ø)
src/SixLabors.Fonts/IColorGlyphRenderer.cs 57% <ø> (ø)
...nts/Tables/AdvancedTypographic/GPos/AnchorTable.cs 38% <0%> (ø)
...xLabors.Fonts/Tables/General/Glyphs/GlyphVector.cs 96% <0%> (-4%) ⬇️
src/SixLabors.Fonts/GlyphLayout.cs 76% <56%> (+29%) ⬆️
src/SixLabors.Fonts/GlyphMetrics.cs 61% <60%> (-20%) ⬇️
src/SixLabors.Fonts/GlyphRendererParameters.cs 62% <66%> (+<1%) ⬆️
...Labors.Fonts/Tables/General/Post/PostNameRecord.cs 66% <66%> (ø)
src/SixLabors.Fonts/GlyphShapingData.cs 85% <75%> (+<1%) ⬆️
src/SixLabors.Fonts/Tables/General/PostTable.cs 82% <82%> (ø)
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 388a24f...7b3ec08. Read the comment docs.

Copy link
Member

@tocsoft tocsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good, couple of comments.

src/SixLabors.Fonts/TextLayout.cs Outdated Show resolved Hide resolved
src/SixLabors.Fonts/TextOptions.cs Show resolved Hide resolved
@JimBobSquarePants
Copy link
Member Author

JimBobSquarePants commented Mar 3, 2022

@tocsoft My reasoning re TextOptions was that the new TextRun override in Drawing would be TextDrawingRun

so the following naming looked correct.

  • TextOptions - TextRun
  • TextDrawingOptions - TextDrawingRun

It also made development a little chicken and eggy since the referenced Drawing version in the Samples project would not build if I change the name.

Changing the name isn't great for upgrades but I'm sure we'll do worse before we ship an RC 😁

@JimBobSquarePants
Copy link
Member Author

Font size now works for each run.

I am big and I am small

@JimBobSquarePants
Copy link
Member Author

JimBobSquarePants commented Mar 4, 2022

@tocsoft I've captured metrics for subscript, superscript, underline, and strikethrough that can be used for rendering. My assumption would be that this is performed in GlyphMetrics.RenderTo().

I'm gonna need some help with the actual render implementation though, drawing is still not a strong point of mine.

Redth and others added 4 commits March 3, 2022 21:19
Roughly correct and ported from another project I had this in before, but not exactly tested after porting, so may be some rough edges
First pass at POST table
@tocsoft
Copy link
Member

tocsoft commented Mar 4, 2022

@tocsoft I've captured metrics for subscript, superscript, underline, and strikethrough that can be used for rendering. My assumption would be that this is performed in GlyphMetrics.RenderTo().

I'm gonna need some help with the actual render implementation though, drawing is still not a strong point of mine.

Yeah the rendering side I should be able to tackle... ultimately for all this we will need to pass long a TextRun (either synthetic or user created) long to the IGlyphRenderer specifically we should be able to add a new TestRun field to GlyphRendererParameters with the applicable TextRun for the glyph, we can then utilise that on the Drawing side to handle things like changes to the Brush/Pen, and knowing to draw the strike throughs/underlines etc.

GlyphRendererParameters is also where we can access the metrics for underlining etc.

@JimBobSquarePants
Copy link
Member Author

@tocsoft I'm pinches fingers "this close" to cracking superscript and subscript rendering. I've got scaling and positioning working, I just need to fix an indexing issue with the runs (exclusive end turned out to be a bad idea.) so hold out please.

@JimBobSquarePants
Copy link
Member Author

I'll push the updates soon

superscript and subscript

@tocsoft
Copy link
Member

tocsoft commented Mar 6, 2022

I've split out TextAttributes and TextDecorations into 2 enums as they target different parts of the stack, one effects the layout side and the other is just additional decorations on top.

But we can now override the pen used while drawing decorations.

DrawRichTextRainbow_Solid500x300_(0,0,0,255)_RichText-ArabicRainbow-F(40)
DrawRichText_Solid500x300_(0,0,0,255)_RichText-F(40)

@JimBobSquarePants
Copy link
Member Author

JimBobSquarePants commented Mar 11, 2022

@tocsoft Just did a BIG update to the layout engine to ensure we generate the correct line-height + offsets for mixed font sizes.

Here's how we compare to Chromium based browsers rendering mixed font sizes. It's now exact.

Six Labors - Left : Chromium - Right

Horizontal

layout-horizontal

Vertical

layout-vertical

Found and fixed a line-break issue in the process and also vertical and horizontal centering was a shade off. Test updates have also been verified visually.

@JimBobSquarePants
Copy link
Member Author

Too tired to fix the .NET 472 rounding silliness just now. Will fix tomorrow.

@JimBobSquarePants
Copy link
Member Author

@tocsoft I'm pretty happy with the shape of this now and with your blessing will get it merged. The last things I wanted to have a look at was font hinting and It looks like I've solved the horizontal alignment problem of the l and i glyphs

Tahoma 11.25pt @ 72 dpi

Unhinted
Unhinted

Hinted
Hinted

Tahoma 11.25pt @ 96 dpi

Unhinted
Unhinted

Hinted
Hinted

@JimBobSquarePants JimBobSquarePants requested a review from a team March 26, 2022 12:36
@JimBobSquarePants JimBobSquarePants merged commit 3c2e86d into main Mar 26, 2022
@JimBobSquarePants JimBobSquarePants deleted the js/text-runs branch March 26, 2022 14:01
This was referenced Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rendering engine should be able to support spans of characters with different formats
3 participants