DanielVanNoord / System.Windows.Forms

A .Net Core implementation of System.Windows.Forms based on Mono's System.Windows.Forms for Linux and Windows
Other
43 stars 4 forks source link

Discussion: System.Drawing.Common #14

Open mfeemster opened 3 months ago

mfeemster commented 3 months ago

As mentioned in my post 2 weeks ago, MessageBox does not support the proper icons on linux: https://github.com/DanielVanNoord/System.Windows.Forms/issues/11

I couldn't see any way to make this work other than to obtain the source for System.Drawing.Common and try to build it for modern .NET, then incorporate mono's SystemIcons.cs code.

Finding the source was rather hard. First, it appears there is a version of it that is actively worked on here: https://github.com/dotnet/winforms/tree/main/src/System.Drawing.Common/src/System/Drawing

I assume that when you are on Windows and use a regular System.Drawing, this is what you are getting.

The problem is, this won't run on linux. It's been made to be very Windows specific.

The last version of System.Drawing.Common to work on linux was 6.0. This used to be in https://github.com/dotnet/runtime/ but has since been removed.

So I had to find the commit hash there of 6.0 and pull that: 4822e3c3aa77eb82b2fb33c9321f923cf11ddde6

I was able to get it to build with .NET 8. (I diffed it with the updated code mentioned above, and it seems to be mostly syntax sugar changes).

In doing so, I just deleted all Windows code. I figured there's no need to even make it cross platform because the nuget package works perfectly fine on Windows. The only reason I am doing this is for linux.

I was then able to get your System.Windows.Forms building and running on linux by linking to my custom built System.Drawing.Common.

I am now in the process of getting as many of these test projects to run on linux (Windows specific tests removed): https://github.com/dotnet/winforms/tree/main/src/System.Windows.Forms/tests/IntegrationTests/WinformsControlsTest

At this point, I figured I would take a break and make this post to get your input.

Do you think we should include this custom build of System.Drawing.Common in this project? Or should it be a separate repo/project/package?

Either way, you can switch what you reference in the project file based on the OS it's being built on like so:

  <ItemGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
     <PackageReference Include="System.Drawing.Common" Version="6.0.0" />
  </ItemGroup>

  <ItemGroup Condition="$([MSBuild]::IsOSPlatform('Linux'))">
    <ProjectReference Include="..\System.Drawing.Common\System.Drawing.Common.csproj" />
  </ItemGroup>

Please let me know your thoughts. I think we have something pretty cool going on here that is going to rescue Winforms from its stale state on linux.

mfeemster commented 2 months ago

Since this project appears dormant, I've just made my own project for personal use to be built and ran only on linux.

https://bitbucket.org/mfeemster/system.windows.forms/src/master/

I've got it successfully building this project along with the System.Drawing.Common code from version 6.0 (the last cross platform build MS released).

I've also included these tests:

https://github.com/dotnet/winforms/blob/main/src/System.Windows.Forms/tests/IntegrationTests/WinformsControlsTest

I had to fix them up a bit to run properly, but most of the tests without windows-specific code run fine on linux.

From now on, I'll just periodically check back here to see if any fixes are made and manually merge them into my repo.

sancheolz commented 2 months ago

@mfeemster It's nice to see that most of the tests from the original WinForms pass.

I see that you started the history of your fork from scratch and did not save the history of this repository, and there were many changes with spaces and line breaks. Because of this, it's hard to compare your fork, the wine-mono project and this repository. In addition, your fork is aimed at net8.0. Perhaps I will continue to upload my changes to this repository in the hope that your fork will be synchronized enough when I decide to switch to net8.0.

DanielVanNoord commented 2 months ago

Ok, I have had some time to look into options. Although I was hoping to avoid forking System.Drawing.Common that looks like it is the best bet going forward to maintain functionality. I'm going to see if I can pull it into this repository (or a pull request is welcome). Ideally we keep support for System.Drawing.Common on both Windows and Linux, I view testing on both operating systems (and comparing the behavior to the official version of Windows WinForms on .Net 6-8) as useful. Adding the tests to this project could also be helpful.

mfeemster commented 2 months ago

@sancheolz Thanks for taking a look. Sorry about the starting from scratch and changing the formatting. I have Visual Studio configured to format the file whenever I save it, so I didn't really pay attention. I didn't think anyone else would be looking at it. I've updated the readme to give this repo attribution.

@DanielVanNoord I'm glad to hear you've settled on that approach. Sadly, I've stripped the Windows-specific code out of mine so you're better off just getting the 6.0 commit from the hash I listed above and going through the process I did. It might be helpful to diff with mine at some point just to see if there are any needed changes.

One somewhat off topic item I found is that the chart/visualization control is left largely unimplemented. Might it be worth diffing with the latest Winforms to see how much of that can be pulled in? I haven't heard anyone clamoring for it, and my project doesn't use it, but it might be worth looking at after getting these current efforts done, for the sake of completeness.

As for targeting .net 8, that just happens to be the version I'm working with. I'm not sure how to tell the project file "target the latest version of .net on the system greater than 6.0".

If you guys are able to get this working within this repo, and publish it as a package, then I'll ditch mine and just use yours as a package reference. I'm not really married to either solution, I'm fine with whatever has the least friction and is most beneficial.

Thanks for all the work.

madewokherd commented 2 months ago

If it helps, I'm maintaining a fork of libgdiplus here, since upstream seems abandoned: https://gitlab.winehq.org/wine-mono/libgdiplus

sancheolz commented 2 months ago

@mfeemster You can extract changes from your repository if you made clean checkout of this repository, then apply same formatting changes and then compare by copying files from yours repository without .git folder to formatted checkout. Is there in your fork some useful changes?

mfeemster commented 2 months ago

No, there are no useful changes in my builds. I had to make a few changes that were specific to my project, such as making some things in Winforms public instead of internal. It wouldn't make sense for me to push any of those changes to this repo.

I am going in the other direction though: I watch what you commit here and manually pull the changes into my repo because obviously you've done some great bug fixing. I also got the refactoring of the clipboard functionality.