pruiz / WkHtmlToXSharp

C# wrapper wrapper (using P/Invoke) for the excelent Html to PDF conversion library wkhtmltopdf library.
239 stars 84 forks source link

Cleaned up the html => image conversion #17

Closed deganii closed 8 years ago

deganii commented 11 years ago

Hi Pablo,

Here are some changes which should make Dan's image classes ready for primetime.

1) Added a base interface IHtmlToXConverter (the specific image/pdf interfaces derive from it) 2) Created an abstract converter which contains most of the functionality of the old WkPdfToHtmlConverter 3) Made the MultiplexingConverter generic (renamed to GenericMultiplexingConverter) so it handles both PDF's and images.

This eliminates most of the code duplication. I was thinking to generate the child classes WkPdfToHtmlConverter and WkPdfToImageConverter from a T4 template. (This is because the PInvokes are very similar). But I decided not to make things too complicated :)

Please let me know if you need anything further to merge this into the main branch.

Best, Ismail

pruiz commented 11 years ago

Hi deganii,

Looks great, however I will need to merge it manually as there are some white space changes I would prefer to get rid of, and also the previous patch included an updated wkhtmltox.dll binary for unknown reasons which I don't know if I should exclude or not.. I'll have to take a look at it.

Thanks for a great work. Pablo

dambrisco commented 11 years ago

Hey Pablo,

If you're referring to my change to wkhtmltox.dll, it's simply a higher compression ratio. If you unzip it and compare the binaries, they should be identical. (If they're not, it may be a version issue, in which case feel free to exclude it - it should function with either.)

I'll see if I can merge this into mine and do some cleanup work since I'm on vacation right now anyway.

Ismail,

Thanks for taking care of those changes! Have you tested the compiled library outside of the project just to make sure everything still works?

Cheers,

On Wed, Mar 20, 2013 at 3:34 PM, Pablo Ruiz García <notifications@github.com

wrote:

Hi deganii,

Looks great, however I will need to merge it manually as there are some white space changes I would prefer to te rid off, and also the previous patch included an updated wkhtmltox.dll binary for unknown reasons which I don't know if I should exclude or not.. I'll have to take a look at it.

Thanks for a great work. Pablo

— Reply to this email directly or view it on GitHubhttps://github.com/pruiz/WkHtmlToXSharp/pull/17#issuecomment-15205108 .

pruiz commented 11 years ago

Hi Dan,

That's great news as I am a bit busy right now finishing some sutff.. so I'll be really glad if you could work on it. ;)

Pablo

deganii commented 11 years ago

Hi Dan/Pablo,

Yes, I've added a test case to confirm the image routines work, and I've also used it in my application successfully. (The Win64 stuff doesn't work but that's a known issue).

As an aside, I get weird issues with corrupted PDF's when I run in an IIS app pool. Using the MultiplexingConverter doesn't address this. It happens after the pool is recycled or if I publish a new update without restarting. The dll remains locked by IIS and I have to restart in order to fix the problem. Are there any workarounds for this?

Best, Ismail

dambrisco commented 11 years ago

Hey Ismail,

I'm not sure how recycling and all that works. It sounds like we're not releasing a resource properly in a destructor somewhere. I'll see if I can spot anything while I'm doing this cleanup. You might consider setting up a WCF call to a windows service or console app anyway due to both the relative slowness of wkhtmltox and the giant pile of bugs it has, including some that won't be fixed without deep upgrades in Qt. (If you don't like WCF, you can use a simple localhost socket trigger or a database queue.)

Additionally, SetErrorMode or SetThreadErrorMode (choose appropriately) should always be used when dealing with wkhtmltox with flag (0x8000 | 0x0001 | 0x0002) if you don't want error message boxes potentially popping up. It's not relevant to your current issue, but it's something to keep in mind.

Cheers,

Hi Dan/Pablo,

Yes, I've added a test case to confirm the image routines work, and I've also used it in my application successfully. (The Win64 stuff doesn't work but that's a known issue).

As an aside, I get weird issues with corrupted PDF's when I run in an IIS app pool. Using the MultiplexingConverter doesn't address this. It happens after the pool is recycled or if I publish a new update without restarting. The dll remains locked by IIS and I have to restart in order to fix the problem. Are there any workarounds for this?

Best, Ismail

— Reply to this email directly or view it on GitHubhttps://github.com/pruiz/WkHtmlToXSharp/pull/17#issuecomment-15212169 .

drgrieve commented 10 years ago

What is the chance of this pull request being done? Or should I used the forked project which is two commits behind?

Thanks.

pruiz commented 9 years ago

Hi,

I am willing to merge it, but some cleanup is still needed, as well as rebasing it to be based on curren master will help too.

pruiz commented 8 years ago

Closing due to inactivity.