ImageSharp.Drawing
ImageSharp.Drawing copied to clipboard
Rich text rendering
Prerequisites
- [x] I have written a descriptive pull-request title
- [x] 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 :cop:.
- [ ] I have provided test coverage for my change (where applicable)
Description
This build on the SixLabors/Fonts#251 code to extend the capability to also altering the color / outlines.
TODO
- [x] Figure out what's causing the w/v shifting.
- [x] Change back to Fonts package once SixLabors/Fonts#251 is merged.
- [ ] Ensure we have enough test coverage
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
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.
@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.
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.
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.
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.
@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.
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.
@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 Report
Merging #211 (f6dbef1) into main (dc7b553) will decrease coverage by
1%
. The diff coverage is77%
.
: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
@tocsoft I've fixed everything based upon feedback. I've also done additional work on decoration positioning to exactly match the CSS approach.
@JimBobSquarePants 👍 everything looks good to me...not spotting any other issues. :shipit:
@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 |