Adding primitive types to improve drawing

[CoryCharlton]

Description

• Restructure Graphics.Core so folders correspond to namespace
• Added System.Drawing.Point and marked nanoFramework.UI.Point as obsolete
• Added System.Drawing.Size
• Moved Mathematics to Graphics.Core
• Added System.Drawing.Rectangle
• Added overloads to nanoFramework.UI.Bitmap to use rectangle/point
• Added System.Drawing.ContentAlignment

Motivation and Context

This improves the experience of drawing to a Bitmap

How Has This Been Tested?

• Unit tests
• On device

Screenshots

Types of changes

• Improvement (non-breaking change that improves a feature, code or algorithm)
• Bug fix (non-breaking change which fixes an issue with code or algorithm)
• New feature (non-breaking change which adds functionality to code)
• Breaking change (fix or feature that would cause existing functionality to change)
• Config and build (change in the configuration and build system, has no impact on code or features)
• Dependencies (update dependencies and changes associated, has no impact on code or features)
• Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
• Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

• My code follows the code style of this project (only if there are changes in source code).
• My changes require an update to the documentation (there are changes that require the docs website to be updated).
• I have updated the documentation accordingly (the changes require an update on the docs in this repo).
• I have read the CONTRIBUTING document.
• I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
• I have added new tests to cover my changes.

[Ellerbach]

Is there a way to bring couple of unit tests with the additions?
Also thanks for the comments for the commented code, that's perfect!
And thanks for the addition! There are few bindings using those Point, Size, Rectangle. So some migration to expect as well.

[CoryCharlton]

Is there a way to bring couple of unit tests with the additions? Also thanks for the comments for the commented code, that's perfect! And thanks for the addition! There are few bindings using those Point, Size, Rectangle. So some migration to expect as well.

Sure, I'll add some unit tests today.

Any feedback/comments on my other review comments?

[Ellerbach]

Thanks a lot for the addition and the work here. It looks good all up. Kudos points for the tests, for the clarity of the comments.

[Ellerbach]

Any feedback/comments on my other review comments?

Yes, few more things:

• Bump the version in both files
• This will require to update the native side as well. Can you make sure to do this and raise a PR as well? So we can merge both accordingly

[CoryCharlton]

@Ellerbach I updated the minor version in the version.json

Still getting my head around the native side of things. I assume you are asking for changes in https://github.com/nanoframework/nf-interpreter

Couple questions:

• Why do these changes require updates on the native side?
• What do I need to update on the native side? I see this but it doesn't seem relevant for my changes.

[Ellerbach]

Why do these changes require updates on the native side?

Because by changing the API, you are changing the signature. So you need to update this on the native side. The stub are generated for you, then it's a matter of adding those back in their place. So here:

• https://github.com/nanoframework/nf-interpreter/blob/main/src/nanoFramework.Graphics/Graphics/Native/nanoFramework_Graphics.h (you should adjust anything that has been changed, be careful, it's not just a copy/paste of the generated file)
• https://github.com/nanoframework/nf-interpreter/blob/main/src/nanoFramework.Graphics/Graphics/Native/nanoFramework_Graphics.cpp (here, you basically just copy/paste the generated file and as you'll the bottom signature is going to change)

And as all changes, you have to bump as well the versions!

And on the managed side (this PR), you also have to bump this file: https://github.com/nanoframework/nanoFramework.Graphics/blob/main/version.json with something like 1.2

Let me know if you have any question!

[CoryCharlton]

Ok that makes sense. It wasn't clear why entirely managed changes would require updates to the native side but now I see that the method lookup array has NULL for all the managed functions (I wonder if this could be simplified, will open discussion in Discord).

In any case here's the PR for nf-interpreter. I think it's good but I'm not setup to validate yet. nanoframework/nf-interpreter#2802

[josesimoes]

@CoryCharlton no way to change those NULLs on the declaration. That's part of the mechanism there and it has to happen.

[CoryCharlton]

@CoryCharlton no way to change those NULLs on the declaration. That's part of the mechanism there and it has to happen.

Understood. I threw out a naive though regarding an alternative in #interpreter

[CoryCharlton]

Not sure why nanoFramework / NuGets using latest version and nanoFramework / package.lock exists workflows aren't completing but I believe this should have everything needed. Please let me know if there is anything else required.

[josesimoes]

Not sure why nanoFramework / NuGets using latest version and nanoFramework / package.lock exists workflows aren't completing

They need permission to run when originating from external developer.

[josesimoes]

Just minor stuff.
Suggest that this added to mscorlib friends so that after the next release of mscorlib the Mathematics class can be removed.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
51 Code Smells

No Coverage information
0.0% Duplication

[CoryCharlton]

I just replaced Mathematics with MathInternal so I think that covers all feedback in this PR.

[josesimoes]

Merging as it is. The refs to mscorlib will be updated by automation after the release.