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

Fix #878 #907

Merged
merged 1 commit into from
Sep 29, 2024
Merged

Fix #878 #907

merged 1 commit into from
Sep 29, 2024

Conversation

BobLd
Copy link
Collaborator

@BobLd BobLd commented Sep 20, 2024

This is in order to fix #878

Unfortunately some tests are now failing because we are now to able to assess to proper precision to use. We either can fix the test to make it pass, or we need to review the logic

 UglyToad.PdfPig.Tests.Parser.PageContentParserTests.CorrectlyWritesOperations
   Source: PageContentParserTests.cs line 34

Expected: ···"0 0 0 rg BT /F0 21.33 Tf 1 0 0 -1 0 140 T"···
Actual:     ···"0 0 0 rg BT /F0 21.32999999999999829 Tf 1"···

Leaving it here as a draft for the moment. @cremor feel free to have a look

@BobLd BobLd marked this pull request as draft September 20, 2024 18:14
@cremor
Copy link

cremor commented Sep 21, 2024

Do you know how the expected value of 21.33 was produced back then? Which data type and format specifier was used? What is the exact value that is formatted, and is is read somewhere or calculated?

If we would change the F17 format specifier to F14 then this specific case would again be formatted as 21.33. Maybe even less precision is enough? How much precision does the PDF standard even support?

@BobLd
Copy link
Collaborator Author

BobLd commented Sep 24, 2024

Using F14 sounds good to me. For background:

Note: To represent real numbers, Acrobat 6 uses IEEE single-precision floating-point numbers,

image

@cremor
Copy link

cremor commented Sep 24, 2024

If I read this correctly then even F5 would be enough.

@BobLd
Copy link
Collaborator Author

BobLd commented Sep 25, 2024

Yes you are right, that's the theoretical precision. I'm going for 9 (Microsoft recommends F7 for single precision.

I still have an issue with the change as the behaviour is not the same between Framework (some tests are failing) and dotnet8 (all tests are good)

@cremor
Copy link

cremor commented Sep 27, 2024

Yeah, there seems to be a difference in number formatting between .NET Framework and .NET 8:
.NET Framework: https://dotnetfiddle.net/DyH0RY
.NET 8: https://dotnetfiddle.net/1YjwJ8

But this only affects numbers with more than 15 digits. Is this actually a problem? How big would a PDF page need to be so that this could be a real coordinate/size?

edit: I found this about the problem: https://stackoverflow.com/a/1658420/631802

Btw, the WriteDouble3 test fails (in both runtimes) if the current culture isn't using a dot as a decimal separator. You should add CultureInfo.InvariantCulture to that double.Parse call in the test.

@BobLd
Copy link
Collaborator Author

BobLd commented Sep 27, 2024

@cremor thanks for the help here, now updated with your feedback

@BobLd BobLd marked this pull request as ready for review September 27, 2024 13:00
@BobLd BobLd merged commit f6566d6 into UglyToad:master Sep 29, 2024
1 check passed
@BobLd BobLd deleted the issue-878 branch September 29, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PdfDocumentBuilder creates broken annotation elements when copying pages from specific source PDFs
3 participants