dotnet / iot

This repo includes .NET Core implementations for various IoT boards, chips, displays and PCBs.
MIT License
2.18k stars 585 forks source link

Library requirements to replace System.Drawing #1767

Closed raffaeler closed 1 year ago

raffaeler commented 2 years ago

This discussion was opened here. I now opened this thread to collect the pros/cons for any proposal coming from the community members.

As stated by @krwq, the main things to consider for the libraries being proposed here are the following:

I also stressed about the importance of these other following points:

Any discussion beyond the requisited can done "in-person" on the discord channel (displays area): https://discord.gg/GyEfttux (my handle is raf)

Things that need to be changed

Here is the list of references to System.Drawing in the repo:

Those only using Color are quite straight forward to migrate. They require always the same pattern to transform the color space to the right one. A limited hardware can be needed to test but the pattern will always be the same. So that can limit the errors when the hardware is not available.

The problematic ones will be VideoDevice, RGBLedMAtrix and Ssd1351. They will for sure require the hardware to test and make sure all is working perfectly.

/cc @A-J-Bauer @krwq

raffaeler commented 2 years ago

I found this (quite old but still good) comparison among the most popular 2D graphics libraries: https://devblogs.microsoft.com/dotnet/net-core-image-processing/

A-J-Bauer commented 2 years ago

@raffaeler put a lot of effort into the question which of the remainig libraries can handle small fonts better and made a really nice comparison tool here: https://github.com/raffaeler/CompareGraphics. Clear winner is ImageSharp,
After having a long discussion on discord, building skia, using it natively, looking at the experimental native c-api, looking at how SkiaSharp is built on top of that, taking into account that the underlying google implementation frequently changes, looking at the number of persons actually maintaining SkiaSharp, taking into account that Microsoft Maui (prerelease) uses it (as a plus), the number of open issues, the different results produced by the skia C++ code compared to SkiaSharp and performance tradeoff considerations leads me to the conclusion that if System.Drawing has to be replaced at this time:

ImageSharp is probably the better choice for donet/iot (as proposed by @krwq from the start).

Ellerbach commented 2 years ago

+1 on the efforts from @raffaeler and @A-J-Bauer for all the code samples, the deep dive, digging into nasty questions, positively challenging what is next. True community and engineering spirit!

Thank you both really for this engagement an effort. I guess that the summary is a very fair one. So now, we can go for ImageSharp knowing it's been proven to be the best choice for .NET IoT. And knowing as well that if someone wants to use SkiaSharp it's possible in complement of .NET IoT and nothing is blocking to use all the sensor bindings, Gpio, etc etc.

raffaeler commented 2 years ago

To be honest and very clear, the comparison shows that:

  1. the rendering done with the native support in SkiaSharp provides worse results. From my investigation this is clearly a bug in SkiaSharp as it "stretches" the fonts instead of rendering them with the (specified) alias setting.
  2. the system fonts rendering is slightly different. IMO the SkiaSharp rendering is slightly more readable, but it is debatable.
  3. the rendering done using the BDF code in this repo is pixel perfect for SkiaSharp and ImageSharp. There are no differences at all.
  4. the ImageSharp packages SixLabors.Fonts and SixLabors.ImageSharp.Drawing only exists in pre-release on nuget. This is disappointing.
  5. SixLabors provides two distribution: one is free and the other is for payment. It is not the first time a free library changes its license to migrate to a paid one. I am not an expert here, but if @Ellerbach or @joperezr know someone in SixLabors, it would be nice to be reassured that the free one stays free.

As agreed, I limited the comparison to just the fonts and disregarded the performance. In my repository https://github.com/raffaeler/CompareGraphics (where I published the comparison code) I excluded some other libraries because they don't either run on Linux or on ARM. I had to ask to the maintainers as it was not clearly specified.

As ImageSharp has already been used in this repo, it is clearly a 'plus' to adopt this but the point 4 and 5 are the "hottest" and I really would like to have a clear statement from SixLabors before proceeding.

Thanks again to @A-J-Bauer for the help with SkiaSharp code.

raffaeler commented 2 years ago

@raffaeler One last comment from me on the subject (to not confuse people), please change "skia" to "SkiaSharp" in your comment since using skia's core functions natively with C++ provide the right results. It's SkiaSharp (the wrapping of skia's C-API) that breaks things.

You are right, done.

raffaeler commented 2 years ago

Also, please consider that I did not finish the tests. As I wrote this code in the weekend, I could only test it on Windows. I plan to run the code on a real RPi this week.

JimBobSquarePants commented 2 years ago

FYI. I'm working VERY hard on getting both SixLabors Fonts and ImageSharp.Drawing completed and out of pre-release. For example, in I've just pushed major improvements to hinting that can be used to improve output to our nightly feed. Any assistance, of course is most welcome.

Also I'm not planning on dropping the Apache 2.0 license at any time and any changes to the Commercial license will never affect the IOT libraries since they are open source. .

raffaeler commented 2 years ago

@JimBobSquarePants thanks for the great work Jim. Fonts. The code that is currently in this library manually draw the pixels and this of course guarantees the best possible results as the BDF fonts are already of the exact size. My guess is that hinting is good for standard fonts, right?

License. I raised that point because other libraries (Identity Server for example) caused a lot of troubles to a lot of companies, for various reasons, not just for the license fee. I would love to see a clear decoupling API between this library and any graphics primitive. This would ensure that anyone could easily add support for other graphics libraries, if needed.

JimBobSquarePants commented 2 years ago

My guess is that hinting is good for standard fonts, right?

Yeah. Exactly right. That was in response to your point about the rendering of system fonts. Hinting can make a huge different at low resolution.

License. I raised that point because other libraries (Identity Server for example) caused a lot of troubles to a lot of companies

I don't think that's really fair - The people behind Identity server made the correct choice in my opinion. They only charge for companies with 1M USD annual gross revenue. I would even go as far as to say the only trouble involved was companies expecting support under the Apache 2.0 license.

Anyway, I'm not precious about direct integration with ImageSharp and would probably recommend an abstraction also if possible. I just wanted to ensure you had a complete picture of the Drawing/Fonts library progress.

raffaeler commented 2 years ago

Yeah. Exactly right. That was in response to your point about the rendering of system fonts. Hinting can make a huge different at low resolution.

got it, great.

I don't think that's really fair - The people behind Identity server made the correct choice in my opinion. They only charge for companies with 1M USD annual gross revenue. I would even go as far as to say the only trouble involved was companies expecting support under the Apache 2.0 license.

Companies felt to be "hooked" by the promise of the initial license. I had long talks about that (FYI I doesn't change anything for me). They didn't expect support BTW. Futhermore some of them told me the decisions to buy software is made from a different division of the company and they got stuck with a lot of code they could not use anymore in a very short time. While I know very well the efforts required to develop IS, I totally understand their perspective and I can't blame them at all.

Anyway, I'm not precious about direct integration with ImageSharp and would probably recommend an abstraction also if possible. I just wanted to ensure you had a complete picture of the Drawing/Fonts library progress.

Thank you

krwq commented 2 years ago

[Triage] We've discussed this already and we've decided to go ahead with ImageSharp or abstraction layer. Any conversations about how this will be done will be continued here: https://github.com/dotnet/iot/issues/1403

joperezr commented 2 years ago

[Triage] Reopening since we are in talks as well as we are now thinking of potentially using a different library due to licensing issues.