Angle for axonometric grid is now configurable

[Lehonti]

Closes #1438

The logic for calculating the line paths is in PintaCanvas.BuildCanvasAxonometricGridPath, and that's where we should review the changes most closely. The rest of the changes are just plumbing.

[pedropaulosuzuki]

Very good! Tested and works well.

Is there a way to limit the range on the angle widget? Or maybe we could mirror the angle all around and converting to 0-90deg in the backend, if limiting the range in the widget isn't possible.

[pedropaulosuzuki]

I believe defaulting to 30deg would be the best option, as it's the standard for isometric projection. It's also the default in Inkscape.

[cameronwhite]

Also looks good to me! 🎉 No comments beyond what was already mentioned

[Lehonti]

Default angle changed to 30 degrees :)

@pedropaulosuzuki I thought about limiting the range of the angle widget. My thoughts were:

• Adding a NumberRange<int>? property to it, with all necessary checks, allowing us to limit the range.
• Fill the unavailable portion of the 'pie' with a different color

I didn't do it in this pull request because I think it's already long enough and it might be better left to a separate pull request

[pedropaulosuzuki]

Reading this again, I have two small questions:

1. Why create this extra xEnd variable that's only used once, instead of just inserting s / tanAngle directly?
2. Having this variable, why not cast it to (float) when declaring, instead of doing it on the LineTo call?

[Lehonti]

@pedropaulosuzuki

The idea behind having a separate named variable is to have the code be self-documenting (comments get outdated, code does not). I don't follow this rule perfectly, but in general I think the name of the variable helps one understand what it does. And the compiler optimizes it away, so there is no performance cost.

And as for the float variable, that's a good point. I just changed it. And I am sure there are still many more places like this in the code, because finding and fixing all of them is very time consuming. But in fact many of my pull requests have been code cleanups, and I think they have improved the code. If you want, you could compare the code then vs. now https://github.com/PintaProject/Pinta/tree/release-2.1

[pedropaulosuzuki]

The idea behind having a separate named variable is to have the code be self-documenting (comments get outdated, code does not).

Got it. I'm not the biggest fan of single use variables, but I understand some people prefer to have them for expressing intent without risking having outdated comments.

And as for the float variable, that's a good point. I just changed it. And I am sure there are still many more places like this in the code, because finding and fixing all of them is very time consuming.

No worries, I just pointed it out because this wasn't merged yet, and with the casting, I'm not sure if the compiler would optimize it away, as I'm not deeply familiar with the C# compiler, though it's probably not too hard to check the compiled bytecode to find if it inlines it in this case or not.