Investigate integration with Microsoft.Toolkit.HighPerformance

[Sergio0694]

Hey @JimBobSquarePants and team, wanted to run an idea by you guys 😄
I noticed there's quite some overlap between lots of code in ImageSharp and two packages in the toolkit, specifically the base Microsoft.Toolkit and the new Microsoft.Toolkit.HighPerformance package. In particular:

• Virtually the entire Guard class is present in Microsoft.Toolkit package, possibly with even more APIs there. Related docs here. The codegen in these APIs should even be slightly better than that of ImageSharp, in fact I was currently backporting some of the tweaks there to ImageSharp in Improved Guard APIs codegen SharedInfrastructure#12.
• I've added a very simple API to wrap Memory<byte> to arbitrary pixel types in Image.WrapMemory<TPixel> APIs wrapping Memory<byte> #1314, to close this feature requestCreating a new Image<TPixel> by Wrapping Memory<byte> #1097) in ImageSharp. I'm now adding a much better version of this API to the Microsoft.Toolkit.HighPerformance package (here), which is more flexible but also more efficient, as it skips a number of indirections that are present in the API currently in ImageSharp.
• I've also added a comprehensive set of ThrowHelper APIs to the Microsoft.Toolkit package (here and here) that we could use throughout ImageSharp to improve the codegen (I've noticed some inline throw new expressions all over the codebase).
• There's also a number of utility methods that I've added in the whole Microsoft.Toolkit.HighPerformance that might be useful to have as well in various ImageSharp internal methods, without having to reinvent the wheel every time and test all that.

I'm fully aware that ImageSharp is meant to be completely self-contained and with no dependencies, so I know that just referencing the two NuGet packages mentioned above would probably not be feasible (though note that both those packages mentioned above are fully self-contained as well and only use standard System.* assemblies and nothing else).

I was wondering, would this be something you might be interested in if it came with no extra package dependencies, for instance via a git submodule that directly points to the source of the two packages in the Toolkit repo (here)? I'd be more than happy to help out with the integration and with some refactoring in ImageSharp to switch the APIs to those from the toolkit, of course 😊

---

Other examples

Was going through some bits of code in ImageSharp to find other examples of types from the HighPerformance package that could be useful, and I find some instances where I think the new set of 2D memory types I've been working on would be pretty useful to make the code less error prone and much easier to read. The types are Memory2D<T>, Span2D<T> and others (full list of APIs and related PR here).

I was these conversion methods from System.Drawing to ImageSharp images (source) that could be rewritten using Span2D<T> to be clearer and less verbose. Here's an example of that method linked above, rewritten using these new APIs:

internal static unsafe Image<TPixel> From24bppRgbSystemDrawingBitmap2<TPixel>(Bitmap bmp)
    where TPixel : unmanaged, IPixel<TPixel>
{
    int w = bmp.Width;
    int h = bmp.Height;

    var fullRect = new Rectangle(0, 0, w, h);

    BitmapData data = bmp.LockBits(fullRect, ImageLockMode.ReadWrite, bmp.PixelFormat);
    
    try
    {
        var image = new Image<TPixel>(w, h);

        Configuration configuration = image.GetConfiguration();

        Span2D<Bgr24> source = new Span2D<Bgr24>((void*)data.Scan0, h, w, 0);

        for (int y = 0; y < h; y++)
        {
            PixelOperations<TPixel>.Instance.FromBgr24(configuration, source.GetRowSpan(y), image.GetPixelRowSpan(y));
        }

        return image;
    }
    finally
    {
        bmp.UnlockBits(data);
    }
}

You can see how the Span2D<T> type automatically takes care of calculating the right indexing into the source memory area. The same type can also track an internal pitch, which means it can support discontiguous memory regions as well, as in, memory regions that are cropped within a larger memory area (the source area is contiguous). I think there could be other use cases for these APIs as well in some places within the codebase, just like using the type here made the code much less verbose due to not having to manually calculate the right offset into each row manually. It basically acts as an abstraction API in the same way that Memory<T> and Span<T> work, but in 2D.

Let me know what you think! 🚀

[antonfirsov]

You probably won't like my answer.

We have our own versions of Memory2D (Buffer2D) and Span2D (Buffer2DRegion) which are already integrated, and work efficiently with other ImageSharp memory management primitives. In theory we could unify these middleware components, which would be an interesting technical challenge, but doesn't bring visible customer benefits for ImageSharp.

Regarding Guard, ThrowHelper and similar: I believe these are very thin primitives, where a potential benefit of a dependency doesn't justify the cost of taking a dependency. I believe we get the most productivity and flexibility if the source of these primitives is self-contained & maintained by SixLabors.

In short, I'd see such an experiment a distraction from our top goals, with little or no benefits. Our time is super limited these days, and we need it to be 100% focused on our backlog:
https://github.com/SixLabors/ImageSharp/issues
https://github.com/SixLabors/ImageSharp.Drawing/issues

[antonfirsov]

There is no safe way to guarantee backwards compatibility here. A future .NET 13.0 app can take a reference to a netcoreapp3.1 dll which is compiled against this stuff, and then probably crash.

In my previous job, I've seen a case where a poorly supported internal library based a critical feature on .NET Framework runtime internals in a similar manner. After a .NET Framework upgrade, one our components started to crash in a magical way. My collogues spent a weekend with desperate debugging + disassembling to figure out what went wrong.

[Sergio0694]

Ok, well that's pretty terrifying 🤣

I mean, good thing those methods are all called Dangerous___, and I for one I'm eager to eventually remove all these hacks relying on internals. In fact the current version of the HighPerformance package has a different implementation on .NET 5 there which is fully portable and doesn't rely on internals at all (did so as the .NET 5 target could run on Mono LLVM as well). Hopefully people won't have to go through all that debugging work in the future, or rather, these things should hopefully not happen 😄

[JimBobSquarePants]

That’s one of only two places I’m using it. The other place is an array of integers so can’t use ROS

[JimBobSquarePants]

For NET 5 we can use the new MemoryMarshal APIs to avoid all the hacking as mentioned above.

[JimBobSquarePants]

Actually, just realized I was looking at the wrong table! Can use MM and ROS for both. Hurrah!