libgdiplus icon indicating copy to clipboard operation
libgdiplus copied to clipboard

Implement CachedBitmap functionality

Open reflectronic opened this issue 4 years ago • 6 comments

This PR implements the CachedBitmap functionality from GDI+. This is in preparation for implementing https://github.com/dotnet/winforms/issues/8822, which adds CachedBitmap to System.Drawing.

I would like to get CachedBitmap implemented for .NET 5. There are only 19 or so days until the window closes. @filipnavara, hopefully I can get this quickly reviewed, ironed out, and merged, so that I can finalize the .NET implementation.

reflectronic avatar Jun 26 '20 07:06 reflectronic

Ah, I forgot about license headers. I'm not really sure what to do with them, since the ones in other files are ancient. Should I use the .NET Foundation license headers found in other projects?

reflectronic avatar Jun 26 '20 22:06 reflectronic

@akoeplinger if you could take a look and merge this soon, that would be great. I hope to get this in soon, so that .NET 5 can depend on it. Is there a time frame for when 6.1 will be releasing?

Sorry for rushing here, I just hope to get this in ASAP before dotnet/runtime closes off new features.

reflectronic avatar Jul 01 '20 18:07 reflectronic

@reflectronic Really great to see new features being implemented in libgdiplus!

I went through the process of getting .NET Core code to depend on a new version of libgdiplus.

  • On Linux, the current policy of the .NET Core team is to test against the version of libgdiplus which ships with the various Linux distributions (as opposed to the latest version of libgdiplus from the Mono repositories). That means you'll need to get a release in libgdiplus, get someone to upstream those releases to the distros, and wait for a new distro release.
  • On macOS, that same policy implies .NET Core relies on whatever version ships with Homebrew. All it takes for Homebrew to ship a new version of libgdiplus is a tag in this repo. You can then open a PR against Homebrew and have it merged fairly quickly. However, as a next step, you'll need someone to run brew update libgdiplus the .NET Core build machines. Last time, that process took 5 months: dotnet/corefx#41707 dotnet/runtime#335

Let's hope the process has been ironed out a bit by now :).

On the other hand, System.Drawing.Common ships as a NuGet package, so it's a bit easier to consume newer versions of System.Drawing.Common.

qmfrederik avatar Jul 01 '20 20:07 qmfrederik

Yeah, surely the process of getting a new version shipped is going to take a while. Since this feature introduces new API surface (possibly for the first time in a long while), I implemented version detection so that the managed API can fail gracefully on an older version.

As for running tests in dotnet/runtime, currently my plan was to just skip the tests for now; when 6.1 eventually makes its way to the build machines, the tests would just light up. When I submit my PR to the runtime repository, then I could probably get some guidance from the area owners on what to do. Hopefully something can be figured out.

reflectronic avatar Jul 01 '20 21:07 reflectronic

For the record, I don't think it's critical to get this shipped everywhere within the next 5 months. For example, one of the biggest scenarios for System.Drawing is Windows Forms; for those who weren't going to rely on libgdiplus anyway, adding CachedBitmap to .NET 5 is going to work flawlessly. Whenever 6.1 does come out, then CachedBitmap can light up on non-Windows platforms from the managed side. However, at the least, we do need to merge this first, so that I can ensure that whatever does ship in the future will be compatible with the way it's implemented in .NET 5.

reflectronic avatar Jul 01 '20 21:07 reflectronic

@akoeplinger, @reflectronic has a matching PR for this open against dotnet/runtime. Do you expect to merge this PR? I guess even when you do, any code in System.Drawing would have to be "light up" for some time.

danmoseley avatar Oct 16 '20 16:10 danmoseley