Add drawing centric path building api. #206
Conversation
-migrate GlyphRenderer to use new helper class directly
|
we probably have some missing api/cases as those are just the ones we can utilise for GlyphRendering but we can expand as required. That's the class I would hand the SVG path parsing logic off too (at least as an extension method). |
Codecov Report
@@ Coverage Diff @@
## main #206 +/- ##
===================================
Coverage 70% 71%
===================================
Files 87 87
Lines 5119 5176 +57
Branches 1062 1069 +7
===================================
+ Hits 3630 3694 +64
+ Misses 1279 1273 -6
+ Partials 210 209 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| /// <summary> | ||
| /// Allow you to derivatively draw shapes and paths. | ||
| /// </summary> | ||
| public class PathDrawer |
There was a problem hiding this comment.
Is there anyway we can simplify things by combining these two types? It seems, to me, that the overlap is confusing.
There was a problem hiding this comment.
Initially I had assumed it wouldn't work as a single class but it seems I was wrong, I've merged the ***To() APIs into the PathBuilder itself.
|
Currently missing the |
| /// <param name="point">The point.</param> | ||
| /// <returns>The <see cref="PathBuilder"/></returns> | ||
| public PathBuilder QuadraticBezierTo(Vector2 secondControlPoint, Vector2 point) | ||
| => this.AddBezier(this.currentPoint, secondControlPoint, point); |
There was a problem hiding this comment.
I wonder whether the two AddBezier methods should be renamed? AddQuadraticBezier AddCubicBezier
| /// <param name="point">The point.</param> | ||
| /// <returns>The <see cref="PathBuilder"/></returns> | ||
| public PathBuilder QuadraticBezierTo(Vector2 secondControlPoint, Vector2 point) | ||
| => this.AddBezier(this.currentPoint, secondControlPoint, point); |
There was a problem hiding this comment.
I wonder whether the two AddBezier methods should be renamed? AddQuadraticBezier AddCubicBezier
|
@tocsoft How should I wanna get the improvements to path text drawing in before #211 so we can have the same rich text functionality in both scenarios. |
|
Ah.... I see you already worked out ArcTo d3ae3c0#diff-ef4d23a8c344c2392f8ac64a5b6bca8d1a8d3fb3eca4d8fb7ed17a28b346bfbdR229 Shall I clean that up and copy it across? |
|
please do... I had forgotten I had gotten that far with it... I think the code still need validating as right. Also it was in need of optimising it looks to me like there should be some quick Vector2 wins possible in there. |
|
Cool. Will have a look tonight |
|
I've made some progress using this as an additional reference. SkMatrix is whack! What a horrible API! |
|
I've almost got this working. I had to almost completely refactor your implementation as It was giving me NANs for any input. I based mine on the reference above https://github.com/google/skia/blob/1f193df9b393d50da39570dab77a0bb5d28ec8ef/src/core/SkPathBuilder.cpp#L437-L505 I'm confident about the radius and sweep and calculations as the basic example I've added to the unit tests works well however when I mix radii things start to go wrong. I'm fairly certain it's to do with the startAngle parameter I'm passing. I've been using the following SkiaSharp code as a reference. var info = new SKImageInfo(300, 300);
using (var surface = SKSurface.Create(info))
{
SKCanvas canvas = surface.Canvas;
canvas.Clear(SKColors.White);
using (SKPath path = new SKPath())
using (SKPaint fillPaint = new SKPaint())
{
fillPaint.Color = SKColors.Black;
path.MoveTo(new(50, 50));
//path.ArcTo(20, 20, -72, SKPathArcSize.Large, SKPathDirection.CounterClockwise, 200, 200);
path.ArcTo(20, 50, -72, SKPathArcSize.Small, SKPathDirection.Clockwise, 200, 200);
canvas.DrawPath(path, fillPaint);
canvas.Save();
}
using FileStream fs = File.Create(@"C:\development\arc2.png");
surface.Snapshot().Encode(SKEncodedImageFormat.Png, 100).SaveTo(fs);
}I also have an SVG <svg height="300" width="300" xmlns="http://www.w3.org/2000/svg">
<path d="M 50 50 A 20 50 -72 0 1 200 2000" stroke="black" stroke-width="1" fill="black" />
</svg>I don't have the geometry chops to figure out this last part I'm afraid. 馃様 |
|
@tocsoft I've made progress! There's something wrong with the point generation code in Black on the left represents our working solution, red the solution derived from
|
|
Hey @tocsoft Got this all working now and also fixed a heap of issues with the old I renamed a few things in I think it's ready to review. I'm sure there's some stuff that could be using Here's some examples of the output. (These are also in the tests)
|
|
Looking good, only thing I can see is the left over class. I can't actually approve the PR cause I initially, created but apart from the class I am happy with it. |
Add drawing centric path building api and migrate GlyphRenderer to use new helper class directly
Prerequisites
Description
As we replatformed glyph rendering onto of this API its should be covered by font tests.