search

manuelbl / SwissQRBill.NET

.NET library for Swiss QR bill payment slips (aka QR-Rechnung)
MIT License
87 stars 33 forks source link

Custom ICanvas implementation for PDFsharp #4

Closed rhegner closed 4 years ago

rhegner commented 4 years ago

This pull request is related to https://github.com/manuelbl/SwissQRBill.NET/issues/3

Here is an initial ICanvas implementation using PDFsharp.

This is not production ready yet. These are the issues:

One challenge was that PDFsharp uses a coordinate system with its origin in the top left corner and the y axis pointing downwards. So I needed to apply a transformation to mirror the y axis. But this caused to mirror the actual text output as well. So as a workaround I had to apply additional transformations in PutText. Text looks ok with this workaround, but I'm not sure if the erroneous scissor symbol is related to this coordinate system mess.

I tried a couple of things, but I didn't find a way to resolve any of the issues 1-3 above. @manuelbl maybe you have some ideas?

rhegner commented 4 years ago

I tried solving issue 2 above by playing around with CurrentPath.FillMode, but no luck so far.

Issue 3 above can probably be solved by calling CurrentPath.StartFigure() and CurrentPath.CloseFigure() at the appropriate places. But I didn't find the solution yet.

rhegner commented 4 years ago

Ok I realized that the cross is drawn in white color, so it is related to issue 4 above. Issues 2 and 4 are now fixed.

rhegner commented 4 years ago

Issue 1 above was a rad vs. degree problem. Fixed.

rhegner commented 4 years ago

I was able to fix issue 3 above as well by calling CurrentPath.StartFigure() right before AddLine, and CurrentPath.CloseFigure() right after AddLine in the LineTo implementation. It feels wrong to wrap every individual line segment in Start-/CloseFigure. I thought that dealing with Start-/CloseFigure in CloseSubpath would be more appropriate (@manuelbl do you agree?), but I couldn't get it to work that way.

Anyway, the result looks fine now :)

I notice that the scissor symbol looks slightly different when comparing the output of my new implementation with the output of the existing implementation, and I'm not sure why. But that doesn't bother me too much.

manuelbl commented 4 years ago

Thanks a lot. I'll into it later today...

manuelbl commented 4 years ago

The code looks good but it brings back bad memories. I've wasted time with PDFsharp's XGraphicsPath before because it's broken and I couldn't find a workaround. It's basically impossible to implement this sequence of drawing commands, which would perfectly fit the graphics model of PDF:

With XGraphicsPath (and System.Drawing's GraphicsPath), it would translate to:

path.AddLine(x1, y1, x2, y2);
path.StartFigure();
path.AddLine(x3, y3, x4, y4);
graphics.DrawPath(pen, path);

But as it turns out, StartFigure() is broken. The implementation is (see XGraphicsPath.cs):

        // TODO: ???

In the context of QR bill, your workaround mostly works except line joins are incorrect as each line segment is a separate subpath. It's visible in the corner marks:

image

The implementation would certainly break for more complex paths. They would be incorrectly filled. It seems like pure luck already that the scissors aren't affected. They look identical to other implementations.

I wish I could recommend a better implementation. But as mentioned above, I failed to find a solution. If you can find a better workaround, that would be great.

Regarding the top-down coordinate system: This also affects the PNG and SVG canvas implementation. And I also found that a transformation matrix alone is not sufficient as the text will be mirrored. I've solved it by mirroring the y coordinates in each method. Your solution seems to work just fine. So no need to change.

The only thing that should be changed is to add a license in the header of the new files, preferably the MIT License. As an example see https://github.com/manuelbl/SwissQRBill.NET/blob/master/Generator/QRBill.cs. I am aware that not all my files have a license. I will add them soon.

Once you have added the license, the PR can be merged.

rhegner commented 4 years ago

Thanks for your feedback. I will add MIT license information and update the PR tomorrow.

Regarding the path related problems: Would it be possible to draw the QR bills without using subpaths/moveto, but just using connected paths? So for example to create the "amount box" you would create two separate paths instead of a single path with a gap?

If this is possible from a design perspective, we could maybe change the interface in the following way, to enforce that limitation on the consumer side of the interface:

This interface would only allow to draw connected paths, and it should probably be possible to implement such a simplified interface using PDFsharp without hitting its limitations.

On the one hand it is unfortunate to adjust the interface because of a bug in some other library. But on the other hand a simplification of the interface could make other future implementations easier as well. Furthermore, I wouldn't hold my breath for PDFsharp to fix this bug anytime soon, as it does not seem to be very actively maintained. Also there are many forks of PDFsharp out there, and I'm forced to use one of these forks because the official build does not yet support .NET Core.

manuelbl commented 4 years ago

I guess I've finally found a workaround for the PDFsharp bug. Basically, all path operations need to be recorded. Once it is know if the path will be stroked or filled, they are replayed. For filled path, each subpath is closed. For stroked paths, each subpath is drawn separately.

BTW: Paths with subpaths are required for the scissors with the hole in the handle.

rhegner commented 4 years ago

Perfect! Now the output looks nice even with zoom level 600% :) Thank you!