Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.ArgumentOutOfRangeException: Non-negative number required on PDFs larger than two gigabyte #72

Closed
erxbout opened this issue Jan 11, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@erxbout
Copy link

erxbout commented Jan 11, 2024

Hello!

While doing some research on my error I stumbled opon this in the repository https://github.com/ststeiger/PdfSharpCore/
This issue is going to be an exact clone of ststeiger/PdfSharpCore#193

Like mentioned in the linked issue, I would also need to process files that are larger than 2GB..
Yea its unusual I agree but that should not be the main discussion point please..

Currently I have the package PDFsharpNetStandard2 installed, that is quite old and its needed to be replaced..
I want to replace it with PdfSharpCore or with this library here PdfSharp..

As stated in already in ststeiger/PdfSharpCore#403 (comment)
I am standing in front of the decision of the package it should be for the future in production code..

It also seems like the code is more or less a fork of each other but I am confused about it.. like do both parties know the other one exists? can there be a cooperation between the packages? Or even a merge?

After looking through this repository I found out that the fix for large files is not yet applied here.. To me thats a blocker to use PdfSharp in production code..

There is a pr where this is fixed in the other library: ststeiger/PdfSharpCore#227

It would be nice to know if this is a possible implementation here, if so I would also be able to open this pr here..

Maybe even adjust it a little because why even bother with negative values in the position value anyway? why not use ulong because the position should never be negative or am I wrong there?

Hoping for help here, I would also work on the pr if thats helpful!
Have a nice day folks!

@ThomasHoevel ThomasHoevel added the enhancement New feature or request label Jan 11, 2024
@ThomasHoevel
Copy link
Member

In the beginning there was PDFsharp. Then there was a partial port to Xamarin which was partially ported to .NET Core giving PdfSharpCore.

Sometimes it is difficult to take bug fixes or improvements from PdfSharpCore back into PDFsharp.

The PR looks quite simple and if you create a PR against the current PDFsharp 6.1.0 Preview 1 it will be quite simple for us to make the changes. I cannot promise they will be included, but if they have no impact on speed then I think they will be used.

@erxbout
Copy link
Author

erxbout commented Jan 11, 2024

Thank you very much for your fast response!

And PdfSharpCore is still maintained by empira or did ststeiger this on his own by copy not by fork?

As for the change PR: That should be created in this repository here or what do you mean by "PR against the current PDFsharp 6.1.0 Preview 1"?
Should this change just convert the int position to long position or should it use ulong position as I proposed? (because a Position should not get negative anyway?)

I would prepare a PR in the following days then :D

@erxbout
Copy link
Author

erxbout commented Feb 1, 2024

Hi

May I ask for feedback on my PR and some help maybe?

@ThomasHoevel
Copy link
Member

PdfSharpCore is not and was never maintained by empira.

Support for files larger 2 GiB will be included with the next preview of PDFsharp 6.1.0 coming this month or so.
Thanks for the inspirational PR. The current implementation is slightly different though.

@erxbout
Copy link
Author

erxbout commented Mar 21, 2024

          The currently released 6.1 preview 2 now supports PDF files larger than 2 GB.

Originally posted by @StLange in #76 (comment)

I will test this in the following weeks and close the issue if everything is working as expected.
Thank you for your time, information and effort! :D

@erxbout
Copy link
Author

erxbout commented Apr 7, 2024

Hi

Thank you again for your time and effort! The new package is now implemented and it seems to work now as expected!

Also nice to be mentioned indirectly in the docs, had a laugh reading that xD

Only thing that I am not sure about at the moment is the TODO comment on this line:

I tried opening the file with this mode and I at least got the metadata out of it..
Later I had to use PdfDocumentOpenMode.Import anyway as the other library we use still has the problem of big sized pdf..
PDFSharp returned an exception when trying to read the page into a new cached document with PdfDocumentOpenMode.InformationOnly..
That confuses me as it was not a "NotImplementedException" so I assume by now there is at least some implementation and the comment does not reflect that?
Would be nice to stream pages but I do not know how complicated that is and its not a necessity for me as I have enough RAM at hand.. But anyway thats another seperate issue for itself ;)

using var pdf = PdfReader.Open(_pathConverter.ConvertForLocalUsage(fileInfo.FullPath), PdfDocumentOpenMode.Import);
var pdfVersion = $"{pdf.Version / 10}.{pdf.Version % 10}";
var pdfSize = pdf.FileSize;
.
.
foreach (var page in pdf.Pages)
{
  using var cachePdfMemoryStream = new MemoryStream();
  using var cachePdf = new PdfDocument();
  cachePdf.AddPage(page);
  cachePdf.Save(cachePdfMemoryStream);
  .
  .

@erxbout
Copy link
Author

erxbout commented Jun 7, 2024

My implementation works now with a little workaround

Thx for the help!

@erxbout erxbout closed this as completed Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants