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

Fix accuracy of drawing text along a path #178

Merged
merged 18 commits into from
Apr 16, 2022
Merged

Conversation

JimBobSquarePants
Copy link
Member

@JimBobSquarePants JimBobSquarePants commented Nov 22, 2021

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

My attempt to fix the accuracy of drawing text along a given path. Fixes #159

  • Fix horizontal text. (Rotates along bottom center and handles vertical/horizontal layout options)
  • Fix vertical layout.
  • Create tests

Layout rules are different for vertical text. https://svgwg.org/svg2-draft/text.html#TextpathLayoutRules

For vertical text layout flows, the default orientation for a given glyph is along the vector that starts at the intersection point on the path to which the glyph is attached and which points in the direction 180 degrees from the angle of the curve at the intersection point.

image

Merging #177 already improved the output for horizontal layout but more work was required to fix the over rotation of letters.

Old
old

New
new-hor

I can't tell whether the glyph offsetting in the outer row is expected.

@JimBobSquarePants JimBobSquarePants added bug Something isn't working text help wanted Extra attention is needed labels Nov 22, 2021
@JimBobSquarePants JimBobSquarePants added this to the 1.0.0 milestone Nov 22, 2021
@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #178 (287987a) into main (1b15fe9) will increase coverage by 0%.
The diff coverage is 76%.

@@         Coverage Diff         @@
##           main   #178   +/-   ##
===================================
  Coverage    71%    71%           
===================================
  Files        87     87           
  Lines      5307   5349   +42     
  Branches   1089   1094    +5     
===================================
+ Hits       3797   3835   +38     
+ Misses     1297   1295    -2     
- Partials    213    219    +6     
Flag Coverage Δ
unittests 71% <76%> (+<1%) ⬆️

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

Impacted Files Coverage Δ
src/ImageSharp.Drawing/Shapes/PathBuilder.cs 89% <ø> (ø)
...ImageSharp.Drawing/Shapes/Text/PathGlyphBuilder.cs 65% <66%> (+65%) ⬆️
src/ImageSharp.Drawing/Shapes/Path.cs 75% <76%> (-2%) ⬇️
...ImageSharp.Drawing/Shapes/Text/BaseGlyphBuilder.cs 88% <84%> (-6%) ⬇️
src/ImageSharp.Drawing/Shapes/Text/GlyphBuilder.cs 100% <100%> (ø)
src/ImageSharp.Drawing/Shapes/Text/TextBuilder.cs 100% <100%> (+50%) ⬆️

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 1b15fe9...287987a. Read the comment docs.

@JimBobSquarePants
Copy link
Member Author

JimBobSquarePants commented Feb 21, 2022

OK. So here's the current layout state for both horizontal and vertical text in this PR.

I'm rendering multiline text which is wraps at the length of the path. It looks to me like our calculations start off accurate for both text directions but become less so. I'll need help figuring out why.

Horizontal
horizontal

Vertical
vertical

@JimBobSquarePants
Copy link
Member Author

JimBobSquarePants commented Feb 25, 2022

@SixLabors/core

Question. How, from a maths perspective would you adjust the position and rotation to compensate for the vertical distance of each glyph away from the path? Ideally the letter spacing should never change. EDIT. Nothing else works that way so we can keep as-is.

THEQUI~1

@JimBobSquarePants
Copy link
Member Author

I'm actually very happy with this now however there is an issue with TextBuilder_Bounds_AreCorrect since we now don't skip empty glyphs in the new Fonts version. @tocsoft @antonfirsov any tips here on how to fix that?
CanDrawTextAlongPathHorizontal_Rgba32_Blank100x100_type-spiral
CanDrawTextAlongPathHorizontal_Rgba32_Blank120x120_type-triangle
CanDrawTextAlongPathHorizontal_Rgba32_Blank350x350_type-circle
CanDrawTextAlongPathVertical_Rgba32_Blank250x250_type-triangle
CanDrawTextAlongPathVertical_Rgba32_Blank350x350_type-circle

@tocsoft
Copy link
Member

tocsoft commented Apr 15, 2022

To be honest I'm not exactly convinced how great a test that is in the first place as they are measuring different things at different points in the processing pipeline of rendering, one don prior to hinting and everything the other is a final vector once all that stuff is applied etc

to unblock yourself in this test case however you can do

-FontRectangle measuredBounds = TextMeasurer.MeasureBounds(text, options);
+FontRectangle direcectMesured = TextMeasurer.MeasureBounds(text, options);
+var measuredBounds = new FontRectangle(new(0, 0), direcectMesured.Size + direcectMesured.Location);

and that test passes again

@JimBobSquarePants
Copy link
Member Author

I have no idea what is going on here with r. It only happens with ellipses using vertical mode and multiline text. You can see things realign with o go out of sync with u then every following glyph is correct. All glyphs are center aligned when using vertical mode as shown in the second image.

image

image

@JimBobSquarePants JimBobSquarePants marked this pull request as ready for review April 16, 2022 08:07
@JimBobSquarePants JimBobSquarePants changed the title WIP Fix accuracy of drawing text along a path Fix accuracy of drawing text along a path Apr 16, 2022

svgPath = FindPoint(svgPath, out PointF point, relative, c);
if (svgPath.Length > 0)
if (TryFindScaler(ref svgPath, out float radiiX)
Copy link
Member Author

Choose a reason for hiding this comment

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

Skia was comparing each pointer here whereas we would only draw an arc if the span processing wasn't complete. Wile fixing to allow checking each step I change the order of params for consistency.


Vector2 targetPoint = point.Point + new PointF(0, rect.Top - this.yOffset);
// Now offset our target point since we're aligning the bottom-left location of our glyph against the path.
Copy link
Member Author

Choose a reason for hiding this comment

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

The SVG docs say vertical layout should use the centre point but I found left/bottom still yielded better results.

@@ -17,8 +17,8 @@
<None Include="..\..\shared-infrastructure\branding\icons\imagesharp.drawing\sixlabors.imagesharp.drawing.128.png" Pack="true" PackagePath="" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="SixLabors.Fonts" Version="1.0.0-beta16" />
<PackageReference Include="SixLabors.ImageSharp" Version="2.1.1-alpha.0.1" />
<PackageReference Include="SixLabors.Fonts" Version="1.0.0-beta16.13" />
Copy link
Member Author

Choose a reason for hiding this comment

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

All the text drawing reference updates are a result of updating Fonts which contains layout fixes.

@JimBobSquarePants JimBobSquarePants requested a review from a team April 16, 2022 08:23
@JimBobSquarePants JimBobSquarePants removed the help wanted Extra attention is needed label Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve accuracy of drawing text along a path
2 participants