Start refactoring pdfwriter #85
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This attacks the same problem as some crufty old code in pdfrw, plus the attempted fixes in PR #60 and PR #82. Those fixes worked on many PDFs, but attempted to do the impossible. If we're building a new page hierarchy, then we really do need a new hierarchy. This fix removes any annotations and beads (which could wend deep into an old page hierarchy) from any added page which is part of an old hierarchy. We detect the old page hierarchy by the presence of the /Parent entry in the page dictionary. If someone wants to preserve annotations, then we'll have to write code to do that properly. I am not 100% happy with making PdfWriter assume that /B and /Annots are the only dict entries that could cause cycles, but so far, the only PDFs I have seen where this is not true were completely broken to start with.
Like the code discussed in issue #52.
Also, stop using user_fmt in findobjs.
This will allow new serializer to check for the encoded attribute in the class, not the instance.
This code is designed to be able to do incremental writing from multiple PDFs. It is not all hooked up yet, but is passing the old basic tests (after double-checking results and updating checksums).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I tried to use the git tools to split and merge to show that I broke a file up.
That went well! (not -- somehow lost info from one of the merges, and it looks ugly on the graph anyway.)
Anyway, I have a direction I want to go, but I think I need to merge this in first.
Structural changes:
I already have the concept for a new serializer. But we'll keep the old one around for backward compatibility (I know some users are using the user_fmt function, and that won't work with the new one in the same fashion.)
Functional changes:
Attempted fix in PRs pdfwriter: update page-referring objects #60 and Make objects refer to replacement pages in pdfwriter #82 done better (I think). If a page is part of a preexisting hierarchy, the "beads" and "annotations" will be removed from the copy. AFAICT, for a correctly-generated PDF, these are the only objects that can eventually refer back to any original pages. This is a great simplification, but naturally, it changed a bunch of test checksums on files that it removed a lot more objects from. I double-checked the generated PDFs and updated the checksums to match.
The ability to generate a large "canonical" PDF (with every array and dict being indirect to make comparison easier) has been removed. I doubt if anybody was using it, but if they were, they should be smart enough to implement those few lines of code outside the library.
Some things, like adding pages and then setting the trailer, or vice-versa, would silently fail. Now they give exceptions.
That's pretty much it. The code still needs more doc, but it's better than it started out, and I don't want to waste too much time documenting its current structure, because there will be a few tweaks coming up soon.
So unless something looks really evil, I am going to dump it to master and see who complains. Maybe I'll dredge up some tests to add to the regression that way. 👍