Conversation
Added matrix transformation to path operators.
|
Thanks very much for putting this together, I'll try to get round to reviewing it as soon as I can. |
There was a problem hiding this comment.
Sorry it's taken me so long to get round to this review, this week has been busy. Thanks for taking the time to contribute these changes.
There are just some changes to make the public API slightly neater but otherwise it seems good to me. It looks like some tests are failing, I'm going to have a look into the reason and add some further comments when I find out why.
src/UglyToad.PdfPig/Graphics/Operations/PathConstruction/AppendRectangle.cs
Outdated
Show resolved
Hide resolved
src/UglyToad.PdfPig/Graphics/Operations/PathConstruction/BeginNewSubpath.cs
Show resolved
Hide resolved
| private readonly List<IPathCommand> commands = new List<IPathCommand>(); | ||
| public readonly List<IPathCommand> Commands = new List<IPathCommand>(); | ||
| private PdfPoint? currentPosition; | ||
| internal TransformationMatrix CurrentTransformationMatrix; |
There was a problem hiding this comment.
This is a good addition 👍
I'd make this a constructor parameter rather than an internal settable field. As far as possible in the code I try to avoid publically mutable fields/properties. It makes the code easier to reason about if it can't be changed from elsewhere.
|
Thank you for the comments! I will review the code and resolve the issues. Sorry, been a bit busy lately :-( |
Co-Authored-By: Eliot Jones <elioty@hotmail.co.uk>
Co-Authored-By: Eliot Jones <elioty@hotmail.co.uk>
EliotJones
left a comment
EliotJones
left a comment
There was a problem hiding this comment.
Thanks for submitting these and sorry it's taken me so long to get round to reviewing!
2 participants
Hi, I added Paths collection to Page to be able to get the list of lines/curves on the page. Also added matrix transform to the path operators.