search

SixLabors / ImageSharp.Drawing

:pen: Extensions to ImageSharp containing a cross-platform 2D polygon manipulation API and drawing operations.
https://sixlabors.com/products/imagesharp-drawing/
Other
282 stars 38 forks source link

Rich text rendering #211

Closed tocsoft closed 1 year ago

tocsoft commented 2 years ago

Prerequisites

Description

This build on the SixLabors/Fonts#251 code to extend the capability to also altering the color / outlines. DrawRichTextWithMixOfPensAndBrushes_Solid500x200_(255,255,255,255)_RichText-F(40)

TODO

UPDATE I've (@JimBobSquarePants ) ran with this and completed the PR replacing the existing CachedGlypRender with a new RichTextGlyphRender in the process removing some old hacks that were negatively affecting the output. Output is more accurate and crisp now. This is obvious for lower font sizes.

Here's a comparison (new right, note the improved rendering on several glyphs (i, j, h, e and others) ) image

Rich text along a path is also supported

Some example output.

Rich text following a path with red strikethrough. CanDrawRichTextAlongPathHorizontal_Solid100x100_(0,0,0,255)_RichText-Path-(spiral)

Rich text using a different pen to underline each glyph to draw a rainbow. DrawRichTextRainbow_Solid500x300_(0,0,0,255)_RichText-Rainbow-F(40)

tocsoft commented 2 years ago

Figure out what's causing the w/v shifting. This looks like it was due to the version of Open Sans we where using in this repo, using the version from the Fonts repo resolved that issue (didn't investigate too much on how/why they where different).

Maybe a hinting issue changing the version of the font did break a lot of tests however they all look like relative mine pixel shifting things but they all looked good to me

JimBobSquarePants commented 2 years ago

This looks like it was due to the version of Open Sans we where using in this repo, using the version from the Fonts repo resolved that issue (didn't investigate too much on how/why they where different).

Ah yes, there were definitely issues with the one we were using and our layout engine was suffering for it. That's why we switched. I must have forgotten to update the one in drawing to match.

JimBobSquarePants commented 2 years ago

@tocsoft I've just merged the latest main into this branch which allows us to update to the version of Fonts supporting this feature. I've also done a little cleanup and added comments.

One thing I'd really like to investigate with this is consolidation of drawing text with or without a path so that we can allow drawing of rich text along a path.

tocsoft commented 2 years ago

I'm am quite confident it can be done, I've spent a couple of hours on it and got close but then failed... so i'm confident it'll be possible...the difficulty is caused by our internal/custom GlyphRenderer that render/caches the individual glyphs prior to compositing them together onto to image. (this is to speed up processing, at the cost of working memory, as we can avoid rasterizing the same glyph multiple times for a large body of text) This custom renderer has a pipeline that causes the rendering to be translated towards (0,0) first for the rasterization (why sometimes we had chopped glyphs) before being rendered to the correct pixel position during composition.

Something in that pipeline in introducing compounded translations for me at the moment ... so I'm thinking on going back to first principle and rebuild up the process ensuring the additional rotation and translation can be applied on the way through.

On in conclusion Bad news i've not get it working, good news I am confident it is possible to enable path rendering with rich text.

JimBobSquarePants commented 2 years ago

Thanks for giving it an initial crack. (I’m wondering whether the compounding transforms are a result of Clear()?)

First principal sounds wise though. I did that a while back with convolution and ended up with a far more efficient and simple solution than I had previously.

JimBobSquarePants commented 1 year ago

Is there anything I can do to help here? I've just updated to the latest Fonts build but didn't want to touch anything you were still tinkering with.

JimBobSquarePants commented 1 year ago

@tocsoft I've cleaned up all the pens and brushes to use the inheritance pattern you designed in conjunction with the changes brought via #257

I can have a crack at the processor if you like but I'm not sure how successful I'll be. If you have any guidance, it would be much appreciated.

P.S I managed to corrupt LFS files with previous updates from main as long paths do not seem to be supported despite the config set to support them. I've fixed those files using the ones from main.

JimBobSquarePants commented 1 year ago

I've gotten everything working. Just need to update the test reference images and add new ones. The new renderer is more accurate than the old one.

JimBobSquarePants commented 1 year ago

@tocsoft When you have time can you have a look over this? I can't add you as a reviewer since you originally opened the PR.

codecov[bot] commented 1 year ago

Codecov Report

Merging #211 (f6dbef1) into main (dc7b553) will decrease coverage by 1%. The diff coverage is 77%.

:exclamation: Current head f6dbef1 differs from pull request most recent head 5d116ce. Consider uploading reports for the commit 5d116ce to get more accurate results

@@         Coverage Diff          @@
##           main   #211    +/-   ##
====================================
- Coverage    71%    71%    -1%     
====================================
  Files        89     95     +6     
  Lines      5395   5628   +233     
  Branches   1098   1153    +55     
====================================
+ Hits       3872   4005   +133     
- Misses     1304   1387    +83     
- Partials    219    236    +17
Flag Coverage Δ
unittests 71% <77%> (-1%) :arrow_down:

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

Impacted Files Coverage Δ
src/ImageSharp.Drawing/Processing/ImageBrush.cs 82% <0%> (-6%) :arrow_down:
...ageSharp.Drawing/Processing/LinearGradientBrush.cs 86% <0%> (-14%) :arrow_down:
src/ImageSharp.Drawing/Processing/PatternBrush.cs 77% <0%> (-6%) :arrow_down:
...ageSharp.Drawing/Processing/RadialGradientBrush.cs 80% <0%> (-20%) :arrow_down:
src/ImageSharp.Drawing/Processing/RecolorBrush.cs 88% <0%> (-8%) :arrow_down:
...ImageSharp.Drawing/Processing/PathGradientBrush.cs 82% <10%> (-5%) :arrow_down:
src/ImageSharp.Drawing/Processing/GradientBrush.cs 80% <20%> (-6%) :arrow_down:
src/ImageSharp.Drawing/Processing/SolidBrush.cs 89% <33%> (-5%) :arrow_down:
...ImageSharp.Drawing/Shapes/Text/BaseGlyphBuilder.cs 57% <40%> (-31%) :arrow_down:
src/ImageSharp.Drawing/Shapes/Text/TextBuilder.cs 62% <40%> (-38%) :arrow_down:
... and 23 more

... and 1 file with indirect coverage changes

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

JimBobSquarePants commented 1 year ago

@tocsoft I've fixed everything based upon feedback. I've also done additional work on decoration positioning to exactly match the CSS approach.

tocsoft commented 1 year ago

@JimBobSquarePants 👍 everything looks good to me...not spotting any other issues. :shipit:

JimBobSquarePants commented 1 year ago

@JimBobSquarePants 👍 everything looks good to me...not spotting any other issues. :shipit:

I had one more trick up my sleeve. I'd spotted that rendering was a bit off vertically with smaller fonts and while fixing it realized we could only cache glyphs along a single line. With the last commit any glyph with matching bounds following the default transform can use a previous render. This makes a huge difference and brings us much closer to System.Drawing.

Given we don't have access to the fast blenders here I'm pretty happy! 

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.22621
11th Gen Intel Core i7-11370H 3.30GHz, 1 CPU, 8 logical and 4 physical cores
.NET SDK=7.0.201
  [Host]     : .NET Core 3.1.32 (CoreCLR 4.700.22.55902, CoreFX 4.700.22.56512), X64 RyuJIT
  DefaultJob : .NET Core 3.1.32 (CoreCLR 4.700.22.55902, CoreFX 4.700.22.56512), X64 RyuJIT

|                                         Method | TextIterations |       Mean |     Error |    StdDev | Ratio | RatioSD |      Gen 0 |     Gen 1 |    Gen 2 |     Allocated |
|----------------------------------------------- |--------------- |-----------:|----------:|----------:|------:|--------:|-----------:|----------:|---------:|--------------:|
|             'System.Drawing Draw Text Outline' |             10 |   2.918 ms | 0.0114 ms | 0.0101 ms |  1.00 |    0.00 |          - |         - |        - |         622 B |
| 'ImageSharp Draw Text Outline - Cached Glyphs' |             10 |   4.901 ms | 0.0267 ms | 0.0237 ms |  1.68 |    0.01 |   421.8750 |  140.6250 |        - |   2,669,828 B |
|         'ImageSharp Draw Text Outline - Naive' |             10 |  25.740 ms | 0.4913 ms | 0.5045 ms |  8.85 |    0.17 |  1000.0000 |         - |        - |  11,499,112 B |
|                                                |                |            |           |           |       |         |            |           |          |               |
|             'System.Drawing Draw Text Outline' |            100 |  26.539 ms | 0.3611 ms | 0.3016 ms |  1.00 |    0.00 |          - |         - |        - |       7,432 B |
| 'ImageSharp Draw Text Outline - Cached Glyphs' |            100 |  69.846 ms | 1.3791 ms | 2.1061 ms |  2.64 |    0.08 |  4714.2857 | 1857.1429 | 571.4286 |  26,412,187 B |
|         'ImageSharp Draw Text Outline - Naive' |            100 | 259.370 ms | 4.9783 ms | 6.9789 ms |  9.85 |    0.34 | 18500.0000 | 3000.0000 | 500.0000 | 115,457,552 B |