Add Unix support for System.Drawing.Common

[mellinoe]

The version of System.Drawing.Common ported from the .NET Framework cannot be used outside of Windows. In order to support the library elsewhere, we are going to port the implementation from mono, which is built around libgdiplus, another mono project. We should do the following:

• Move the code for System.Drawing into corefx, perhaps initially separated from the Windows files. Mono's code is here: https://github.com/mono/mono/tree/master/mcs/class/System.Drawing
• Update build configurations in corefx to create a second build for System.Drawing.Common for Unix platforms. It should reference the code files above.
• In the mono repository: delete the original source files, update the corefx submodule link, and redirect references into the submodule for all of the deleted files.
• [Follow-Up] Try to rationalize the Windows and Unix codebases. There is bound to be a moderately-sized chunk of code that can be shared between the two configurations. Enums and simple structures should be identical between the two.

@qmfrederik Do you have any pointers for this? I know you have worked extensively with mono's System.Drawing recently.

[qmfrederik]

@mellinoe I would try to rationalize the code as first, like this:

• Update the corefx submodule in Mono (and the mono/corefx fork)
• Redirect all "duplicate" files in Mono's System.Drawing to the netfx implementation. The following types should be easy to port (and there are a fair amount of them in Drawing):
  • structs
  • enums
  • interfaces
  • attributes
  • EventArgs
  • exceptions

Mono is likely to rationalize System.Drawing as well; by doing that work in the Mono repository and then porting it to CoreFX I think you're going to avoid some work.

Plus, you can use the Mono build system which is in place to make sure you're not breaking any compatibility on the Mono side - if you do all the work on the CoreFX side of things, you would only detect Mono breaks when you bump the corefx dependency.

Once this is done, it'll give us a better understanding of how much difference there really is between the Mono System.Drawing and the NetFX. I think it will be relatively little.

It looks like Mono bumps CoreFX frequently, and that they are tracking corefx/master, so that shouldn't be too much of a hassle.

I'd be happy to help with that.

@marek-safar How realistic is it to have an updated version of corefx as a submodule of Mono (one that includes e15746e)? Once that is in place, I guess we can start submitting PRs to Mono to consume as much of corefx as possible.

[fanoI]

Having System.Drawing on Unix does not means that WinForms are needed too? I've tried to use System.Drawing on Cosmos but when I've seen that any method draw in a form I've done my separated implementation.

There is a chance in any case to have an official WinForm port on Unix?

[qmfrederik]

@fanoI System.Drawing does not require WinForms (WinForms does require System.Drawing).

So you can have System.Drawing on Unix without WinForms - see Mono or CoreCompat.System.Drawing as an example of how that could work.

I don't think anyone is working on porting WinForms to netfx on Unix; Mono does have a WinForms on Unix implementation if that's what you're after.

[marek-safar]

@mellinoe done the mono update

[mellinoe]

@qmfrederik Thanks for jumping on this. I think reducing any duplication in the mono sources is a good first step. It should make the later steps much easier.

[qmfrederik]

@mellinoe Sure, I think there's still a lot of low hanging fruit in the .Imaging and .Drawing2D namespaces. I'll submit a new PR to Mono once the names in corefx are OK.

Once that's done, I think there are a couple of fairly easy changes still to be made (for example, I omitted exceptions because they use embedded resources, or some of the enums because they reference constants declared in NativeMethods).

After that, we should have a pretty good understanding of the delta between Mono's S.D and the one in CoreFX.

[fanoI]

@qmfrederik it does exist a try to port Winforms to Net Core: https://github.com/akoeplinger/mono-winforms-netcore but its last update is 2 years ago!
As System.Drawing is the building block of Winforms will be possible in future to have Winforms as an official part of Net Core?

Not having a portable way to do a GUI on .Net is its only drawback, there are alternative yes (Avalonia, Eto.Forms, XWT...) but neither of them offer a simply to use designer to create the forms!

[mellinoe]

@fanoI Please file a separate issue if you would like to discuss Winforms. I would like to keep the discussion here focused on the work needed for System.Drawing.

[mellinoe]

I have a WIP version of the mono code compiling for netcoreapp-Unix in this branch:

https://github.com/mellinoe/corefx/tree/system.drawing.unix

I'll try to note some issues as I encounter them.

• Mono source code has [MonoTODO], etc. littered around it; we probably don't want to persist this into our assemblies, but we should come to a consensus on what is appropriate here.
• Mono ships some system icons in the System.Drawing assembly. The "application" icon is the mono logo, which we do not want to use for .NET Core. We should think about what we want to do for system icons.
• With only the minimal set of changes, the implementation fails about 1/4 of the tests that @hughbe is adding in Add System.Drawing.Icon tests corefx#20811. Some of these look like genuine bugs, like icon's being loaded with the wrong Width and Height. There will most likely be a long bug tail as we try to achieve behavior compatibility between the two implementations.

[hughbe]

We're planning to import libgdiplus into corefx, right? Most of the compat bugs will be in the native implementation (including missing argument validation or segfaults - see mono/libgdiplus#54) rather than in the managed implementation and this would make it easier to fix things

[qmfrederik]

@hughbe Cool, looks like libgdiplus is getting additional test coverage as part of this!

Just one nit - libgdiplus does not ship with .NET Core, but is acquired as part of the Linux distributions.
This means that any change you make to libgdiplus will only be picked up by the distro's at a future point in time.

When porting the CoreFX code to Mono, I also hit some cases where, for example, argument validation was done in the managed code rather than the native code.

In those cases, I think it makes sense to add these checks to the managed code as well.

For example, regarding mono/libgdiplus#54, the checks you've added on the native side could be duplicated on the managed side as well. Plus, this would give you the opportunity to let the caller know which argument was invalid (information that is lost if you just check the error code on the native side).

[mellinoe]

@hughbe We would very much like to avoid building libgdiplus for .NET Core. It will add a very large amount of complexity for us if we aren't able to rely on the package manager's version of libgdiplus, even though it means we can't directly fix bugs there. When we have tests in place along with the libgdiplus wrapper in place, we will understand how bad the behavior problems are. If they are limited to exception handling, error cases, etc. then I'm not sure it will be worth it to take on that burden.

To expand: it's not just about moving the code into corefx and building it -- we would also need to re-configure the library such that it only depends on glibc (like our other "portable Linux" shims). We might not even be able to build it in such a way that is ABI-compatible with all of the distros we support. We would also need to track and document all of the new dependencies that we use (instead of just "libgdiplus").

Depending on how much work that all is, it may even make more sense to just re-implement the "faulty" parts in C#, and avoid all of the auxiliary complexity with configuring and building a new native shim.

[hughbe]

@qmfredrik I was wondering if you could share any documentation or instructions you have for building libgdiplus on Windows.
So far I've managed to get the solution opened after also installing the vs2015 c++ tools, but am getting lots of errors for glib.h. Obviously this is a missing dependency. I can fix this myself but I was wondering how you do it so this can be documented and I do this right.
Cheers

[qmfrederik]

@hughbe When I got libgdiplus to compile on Windows, I documented the steps in the appveyor.yml file.

The before_build section contains the section where the dependencies are downloaded, you'll need https://dl.hexchat.net/gtk-win32/vc14/x86/gtk-Win32.7z or https://dl.hexchat.net/gtk-win32/vc14/x64/gtk-x64.7z and extract it to the gtk folder.

Some gotchas are described here, for now, your best bet is to compile for 64-bit Windows and disable fontconfig initialization.

PS: I believe that libgdiplus on Windows, once running properly, should unblock System.Drawing on Nano server

[mellinoe]

PS: I believe that libgdiplus on Windows, once running properly, should unblock System.Drawing on Nano server

I'm not sure it will be a viable option. We currently don't ship any native shims for Windows, and doing so for this would introduce a lot of extra dependencies, just for Nano. It's likely that we just won't support the library there. After all, System.Drawing has never been supported for use on servers, and this is well-documented.

[mellinoe]

I updated my branch with all of the changes in mono:

https://github.com/mellinoe/corefx/tree/system.drawing.unix

Currently, 1460 / 2459 tests pass on Ubuntu 16.04. Many of the failures seem to be fixable without making any changes to libgdiplus.

@qmfrederik Are you planning on doing any more consolidation to the mono library?

[qmfrederik]

@mellinoe There's definitively room to do more consolidation in the Mono library, although I can't commit to much the coming weeks.

• There's still a lot of classes that can be migrated fairly easily, perhaps with tweaking some of Mono's unit test. @akoeplinger would it be possible to get an updated external/corefx submodule in Mono, as the System.Drawing sources have changed over the past weeks?
• I could also help with getting the unit tests to pass in corefx on Linux (see what we can do on the managed side). Do we have libgdiplus on the corefx build servers?

[akoeplinger]

@qmfrederik yep, I'll work on updating corefx in Mono. edit done!

[mellinoe]

Do we have libgdiplus on the corefx build servers?

Not yet -- I will work on getting that done.

[mellinoe]

I've discovered a slight wrench in the plans for using the existing mono code on CoreCLR. Some platforms (like Fedora) use the name "libgdiplus.so.0", rather than just "libgdiplus.so". The Ubuntu/Debian packages include both of those names (or at least symlinks for them), so CoreCLR is able to resolve DllImport's which just specify "gdiplus" as the library name. Mono is able to work on all of its platforms because it has a global config file which specifies a DllMap from "gdiplus" to "libgdiplus.so.0".

Realistically, the best option here is to rewrite the native imports using manual library and function loading. This can be shared across all platforms, including mono.

@yizhang82 @stephentoub Thoughts on this?

[stephentoub]

Given the runtime support (or lack thereof) that exists for this, doing it manually seems like the best option.

Seems like even a small runtime feature would be helpful here, e.g. if DllImport could be augmented to support taking the name of a static field to read from or a static property/method to invoke to get the name of the library that should be targeted. Then this would be fairly easily solved by using a static cctor to get the name of the library to use for the given platform, either hardcoded based on looking up platform information, or doing a search for the library, or whatever. Rewriting however many hundreds of DllImports this code has to do it manually, even with a helper I assume we'd employ, is a lot of work.

[qmfrederik]

@mellinoe For CentOS, I think there are two additional options that could serve as temporary workarounds:

• Use libgdiplus0 from the Mono repositories (http://www.mono-project.com/download/#download-lin-centos); that does include libgdiplus.so
• If you don't find libgdiplus.so in the libgdiplus packages, can you check the libgdiplus-devel package? That sometimes includes the .so-files without a version suffix.

[mellinoe]

@qmfrederik Realistically, it will probably be more work to go through and update all of the CI machines with a different package (and test them, etc.) as a temporary workaround than it will be to just fix the imports in the code. We will need to fix the code at some point, anyways. I should be able to write something that will automate the translation.