Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.Drawing.Common only supported on Windows #354

Closed
mathiasnohall opened this issue Dec 3, 2021 · 7 comments
Closed

System.Drawing.Common only supported on Windows #354

mathiasnohall opened this issue Dec 3, 2021 · 7 comments

Comments

@mathiasnohall
Copy link

mathiasnohall commented Dec 3, 2021

Type of issue

Request for new feature/improvement

Expected Behavior

QRCoder can be run in a linux docker container

Current Behavior

QRCoder can only be run in a windows docker container

  • Version used: 1.4.2

As specified here:
https://docs.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/6.0/system-drawing-common-windows-only
System.Drawing.Common is only supported on Windows which is referenced by QRCoder.
We want to be able to run our service using QRCoder in our Kubernetes Cluster using linux containers. This cannot be done with this depencency.

@codebude
Copy link
Owner

codebude commented Dec 4, 2021

Hi @mathiasnohall ,

thanks for pointing me there. Up to .NET 5.0 System.Drawing could be used via NuGet package. That's why the System.Drawing-dependent parts of QRCoder are available in the binaries that target net50. Since we don't use an explicit target net60 in our NuGet package, NuGet picks the "nearest" match (net50) when using QRCoder in a .NET6.0 environment. This may result in the problems you describe.

Short-term:
I will add net60 and net60-windows as target platforms in the next QRCoder release (and remove the System.Drawing-related parts from the net60 target). That should hopefully solve your trouble.

Mid-term:
We will move away from System.Drawing as discussed in #315

@codebude
Copy link
Owner

codebude commented Dec 9, 2021

Hi @mathiasnohall ,

I added net6.0 and net6.0-windows as targets and removed all System.Drawing dependencies from the net6.0 target.

Can you please pick the latest CI-build and check if it's working for you? CI-Build Packages
(I would like to get some feedback, before I publicly release it on nuget.org)

If you need help on how to use Github Packages as NuGet package source, let me know (or check this article).

@mathiasnohall
Copy link
Author

mathiasnohall commented Dec 10, 2021

Hi @codebude, thanks for trying to solve this!

tried it out and I can download the package from github nuget repo locally. However our code doesnt compile with the new version. We are creating a QRcode image from string input that is converted into a byte array.

        public byte[] GenerateQRCode(string input)
        {
            using var qrGenerator = new QRCodeGenerator();
            using var qrCodeData = qrGenerator.CreateQrCode(input, QRCodeGenerator.ECCLevel.Q);
            using var qrCode = new QRCode(qrCodeData);
            using var qrCodeImage = qrCode.GetGraphic(20);
            using var qrCodeStream = new MemoryStream();
            qrCodeImage.Save(qrCodeStream, ImageFormat.Png);
            qrCodeStream.Position = 0;
            return qrCodeStream.ToArray();
        }

maybe there is a better way?

@codebude
Copy link
Owner

codebude commented Dec 10, 2021

Hi @mathiasnohall ,

However our code doesnt compile with the new version.

That's "clear" and expected. Since you target net6.0 (non Windows) all System.Drawing references were removed. Thus not all renderers (read more about renderer here) are available. (All renderes that are based on System.Drawing - and QRCode is one of these - aren't part of thenet6.0 targeted binary.) So you have to use a rendering class that isn't based on System.Drawing (=all remaining/avilable classes in QRCoder net6.0 binary.)

maybe there is a better way?

Yes, there is. Just replace QRCode with PngByteQRCode renderer class. It doesn't depend on System.Drawing and returns a byte-array, so you don't have to do the conversion yourself.

So for your example from your comment it would look like this:

public byte[] GenerateQRCode(string input)
{
    using var qrGenerator = new QRCodeGenerator();
    using var qrCodeData = qrGenerator.CreateQrCode(input, QRCodeGenerator.ECCLevel.Q);
    using var qrCode = new PngByteQRCode(qrCodeData);
    return qrCode.GetGraphic(20);
}

@mathiasnohall
Copy link
Author

ok thanks! cool our CI pipe works now with jenkins and docker linux containers :)

Just going to verify that the QR-code image turns out OK in our QA testings, that will take a couple of hours.

the memory footprint of the final QR code byte array reduced from 16263 bytes to 726 bytes using the new code.. :)

Thanks!

@codebude
Copy link
Owner

Hi @mathiasnohall ,

thanks for your help and patience. I released QRCoder 1.4.3 right now: https://www.nuget.org/packages/QRCoder/
It contains the changes from the pre-release and should solve this issue/your painpoints.

@TheConstructor
Copy link

Just a quick question: have you tried if adding [SupportedOSPlatform("windows")] was enough? As far as my net5.0/net6.0 experience went, it was enough to get rid of all System.Drawing-related compiler warnings for me, to annotate the code-paths ending to use it. It would still cause compile-time-warnings for non -windows TFM/non-annotated code to use e.g. QRCode, but would make 1.4.3 less of a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants