Expose final internals to allow decoupling Drawing.

[JimBobSquarePants]

Prerequisites

• I have written a descriptive pull-request title
• I have verified that there are no overlapping pull-requests open
• I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
• I have provided test coverage for my change (where applicable)

Description

Fixes accessibility for any remaining blockers that are stopping us decoupling ImageSharp.Drawing

TODO:

• 'ImageFrame.Configuration' is inaccessible due to its protection level.
• 'float' does not contain a definition for 'Clamp' and no accessible extension method 'Clamp' accepting a first argument of type 'float'.
• 'Image<TPixel>' does not contain a definition for 'GetMemoryAllocator' and no accessible extension method 'GetMemoryAllocator' accepting a first argument of type 'Image<TPixel>' could be found
• 'MemoryAllocator' does not contain a definition for 'Allocate2D' and no accessible extension method 'Allocate2D' accepting a first argument of type 'MemoryAllocator' could be found.
• 'GraphicsOptions.IsOpaqueColorWithoutBlending(Color)' is inaccessible due to its protection level

I consider this issue the highest priority for now and would really appreciate any assistance that can be given determining the best fix for each outstanding issue.

[JimBobSquarePants]

Ok. I'm managed workarounds for everything but the MemoryAllocator limitations. @antonfirsov I'll need your help there.

[JimBobSquarePants]

I've temporarily enabled internals to test the build cross platform. To test and develop locally comment out the following line in the ImageSharp.csproj

<InternalsVisibleTo Include="SixLabors.ImageSharp.Drawing" />

[codecov]

Codecov Report

Merging #1046 into master will increase coverage by 0.03%.
The diff coverage is 97.09%.

@@            Coverage Diff             @@
##           master    #1046      +/-   ##
==========================================
+ Coverage   89.86%   89.89%   +0.03%     
==========================================
  Files        1104     1107       +3     
  Lines       48948    49156     +208     
  Branches     3452     3447       -5     
==========================================
+ Hits        43987    44191     +204     
- Misses       4250     4253       +3     
- Partials      711      712       +1

Impacted Files	Coverage Δ
src/ImageSharp.Drawing/Utils/QuickSort.cs	100% <ø> (ø)	⬆️
src/ImageSharp/Memory/MemoryAllocatorExtensions.cs	100% <ø> (ø)	⬆️
...Processing/Extensions/FillPathBuilderExtensions.cs	0% <0%> (ø)	⬆️
...g/Processing/Extensions/DrawRectangleExtensions.cs	0% <0%> (ø)	⬆️
...ing/Processing/Extensions/DrawPolygonExtensions.cs	33.33% <0%> (ø)	⬆️
...ts/ImageSharp.Tests/Drawing/Paths/FillRectangle.cs	100% <100%> (ø)	⬆️
...ageSharp.Tests/Drawing/Paths/DrawPathCollection.cs	100% <100%> (ø)	⬆️
...ts/ImageSharp.Tests/Drawing/FillSolidBrushTests.cs	100% <100%> (ø)	⬆️
...ng/Processors/Overlays/BackgroundColorProcessor.cs	100% <100%> (ø)	⬆️
...ing/Common/Extensions/GraphicsOptionsExtensions.cs	100% <100%> (ø)
... and 58 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41c7651...1f28f0c. Read the comment docs.

[JimBobSquarePants]

@antonfirsov Is there any reason you can think of why we shouldn't expose the first two extension methods here?

ImageSharp/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs

Line 14 in 41c7651

internal static class MemoryAllocatorExtensions

EDIT
I'm exposing it. I can't see any reason not to.

[antonfirsov]

Mostly good, however the API surface of GraphicsOptions and TextGraphicsOptions has several issues. The current API review/cleanup process is our last chance to address them:

1. GraphicsOptions.Default and TextGraphicsOptions.Default are exposing an arbitrary shared mutable state. It was not the case when those were struct-s, and I think we should avoid this, and replace the singletons with default constructor calls.
2. The classes need a deep copy logic, just like Configuration
3. The classes are showing the symptoms of the Telescoping Constructor anti-pattern. These constructors are unnecessary, since the properties are settable.

Additionally, we should remove <InternalsVisibleTo Include="SixLabors.ImageSharp.Drawing" /> from ImageSharp.csproj to ensure this is indeed the final step to close #1002.

[JimBobSquarePants]

Thanks for the fast review @antonfirsov I'm keen to move forward with this.

GraphicsOptions.Default and TextGraphicsOptions.Default are exposing an arbitrary shared mutable state. It was not the case when those were struct-s, and I think we should avoid this, and replace the singletons with default constructor calls.

1. The classes need a deep copy logic, just like Configuration
2. The classes are showing the symptoms of the Telescoping Constructor anti-pattern. These constructors are unnecessary, since the properties are settable.

Agree with all of the above. That won't take me long.

Additionally, we should remove from ImageSharp.csproj to ensure this is indeed the final step to close #1002.

Yeah, good point, will do.

[JimBobSquarePants]

@antonfirsov I have updated everything except the csproj InternalsVisibleTo declaration as the shared infrastructor classes used in Drawing rely on that call until we can move the library to a separate repository.

[antonfirsov]

The shared default instances are still there.

except the csproj InternalsVisibleTo declaration as the shared infrastructor classes used in Drawing rely on that call

I still think we shall get rid of those build errors in this step, or a separate PR before actually moving the whole thing out. The Drawing project shall include the shared infrastructure independently.

[JimBobSquarePants]

I still think we shall get rid of those build errors in this step, or a separate PR before actually moving the whole thing out. The Drawing project shall include the shared infrastructure independently.

Feel free to try. I got an ambiguity errors when I tried.

[antonfirsov]

Ok, changed my mind 😄
It would be easier to fix those errors as part of the migration process.