PdfSharpCore icon indicating copy to clipboard operation
PdfSharpCore copied to clipboard

Charge target to netstandard2.0 and remove dependency to ImageSharp

Open PhenX opened this issue 6 years ago • 7 comments

PhenX avatar Mar 31 '19 15:03 PhenX

As far as I know, your change appears to go against the purpose of this fork. (But I can't speak for @ststeiger so I don't know what he intended) There is already another .NET Standard fork of PdfSharp that uses System.Drawing. System.Drawing requires additional libraries to be installed to use outside of Windows, (libgdiplus on Linux for example) and is not officially supported in headless (services, web apps) environments despite the fact that most people used it that way on Windows. It is also not thread safe.

ImageSharp was designed to solve these problems - It's multi-platform .NET managed code with no native dependencies, works headless, and is thread safe. By getting rid of ImageSharp in your PR you re-introduce those limitations.

I started using ImageSharp and this fork a few weeks ago and found that it (ImageSharp) supported all of the drawing features we needed despite being slightly slower to draw than GDI+. Given that my team was incorrectly using System.Drawing in a server environment for years when we should not have been, we agreed that switching to ImageSharp was a good idea. I would recommend keeping ImageSharp unless Microsoft intends to make System.Drawing officially supported in headless environments.

Thoughts?

Sappharad avatar Apr 03 '19 14:04 Sappharad

@Sappharad: Yea, not entirely. If I guess correctly, he wanted to add it as interface, so you don't need to have ImageSharp as dependency. He then added a default-source back that uses GDI+ (mono/Windows). But it misses an implemented ImageSource for ImageSharp. We just need to re-add a class that implements ImageSource. I already merged another pull-request that boost the framework to NetStandard 2.0 I have little time at the moment, so I haven't merged this, just yet. Another reason I didn't merge it, is that this breaks backwards compatiblity, which is bad.

ststeiger avatar Apr 03 '19 15:04 ststeiger

@Sappharad @ststeiger As far as I understand, System.Drawing.Common is now cross platform : https://www.hanselman.com/blog/HowDoYouUseSystemDrawingInNETCore.aspx

Microsoft has released System.Drawing.Common to provide access to GDI+ graphics functionality cross-platform.

However, while I agree about the backwards compatibility break, I recommend using official packages provided by Microsoft.

The ImageSource implementation should be retrieved via dependeny injection, in my opinion, if you want to keep it customizable.

PhenX avatar Apr 03 '19 16:04 PhenX

@PhenX The same article you linked does mention the native dependencies that must be installed on the server to use it on Linux

sudo apt install libc6-dev 
sudo apt install libgdiplus

The article you linked also mentions that you should not be using System.Drawing for new code.

There's lots of great options for image processing on .NET Core now! It's important to understand that this System.Drawing layer is great for existing System.Drawing code, but you probably shouldn't write NEW image management code with it. Instead, consider one of the great other open source options.

Scott's first recommendation is ImageSharp, which is exactly what you removed in this PR.

Sappharad avatar Apr 03 '19 17:04 Sappharad

@PhenX: Be careful about everything you read on hanselman's blog. It's usually not really well researched, obsolete the day it is published, doesn't go into depth, and he doesn't tell you the bad parts, because he's a developer evangelist, not an actual user.

So for one, you need superuser-rights to install libgdiplus. You might not have or even get them. Then, it doesn't really work on Android/iOS (NetStandard is NetCore + Android + iOS).

Besides, I ported the full PdfSharp 1.5 to NetStandard, here. So if you want to use GDI+, use that. There are some issues, however. See issues 5 and 6

Also, in some settings on Azure, you can't use GDI+. And you shouldn't use GDI+, because the way it is built, it leaks memory, for no good reason. GDI+ is an antique overcomplicated system that wasn't designed to be run on servers. If your application has a few hundred users and should be always available, you can't just restart the server every now and then (apart from the fact that you then need to monitor memory in order to know when to restart). Also, memory isn't free, so if you run on AWS/Google/Azure, you'll pay for every byte you use, so you don't want to use more than necessary. Then, there still is that issue of security, buffer overflows etc. that you don't have with a fully managed library.

ststeiger avatar Apr 04 '19 06:04 ststeiger

@PhenX: Be careful about everything you read on hanselman's blog. It's usually not really well researched, obsolete the day it is published, doesn't go into depth, and he doesn't tell you the bad parts, because he's a developer evangelist, not an actual user.

haha, I had to chuckle at this...

Besides, I ported the full PdfSharp 1.5 to NetStandard, here.

Which of your ports would you recomment to use on Linux then? Core or NetStandard?

aggsol avatar Aug 08 '19 08:08 aggsol

System.Drawing.Common is no longer crossplatform on net6.0

https://docs.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/6.0/system-drawing-common-windows-only

viceice avatar Jun 09 '22 12:06 viceice