Introduce the Buf type
#47
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
# Conflicts: # src/functions.rs
laurmaedje
reviewed
Nov 12, 2024
laurmaedje
reviewed
Nov 27, 2024
| assert_eq!( | ||
| chunk.as_bytes(), | ||
| b"1 0 obj | ||
| << |
There was a problem hiding this comment.
Asserting the PDF in the example is a bit unconventional. Do you think we need this?
There was a problem hiding this comment.
Print it instead? Or just remove it completely?
There was a problem hiding this comment.
The other examples write it to a file
There was a problem hiding this comment.
Hmm yeah but the other examples also create an actually valid PDF file. :p
There was a problem hiding this comment.
I can change it so it’s a valid PDF too, but maybe a bit unnecessary if it’s just about showing how to use the Limits API.
There was a problem hiding this comment.
I see. Maybe we can just completely ignore it?
There was a problem hiding this comment.
So remove all asserts? Or just the one from the PDF buffer?
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.
So, the problem I'm trying to solve is that for PDF 1.4 (and by extension PDF/A1), there are a couple of more architectural limits that need to be upheld, and doing so on the caller-side is very hard.
The idea is simple: Right now, must writers (i.e. mainly the
Chunktype, but also the writers for cmaps and postscript code) use a simple vector of bytes to write to). This PR introduces aBuftype, which is a very thin wrapper around a vector of bytes, but in addition to that it holds a struct which tracks the limits of the data types that were used in the buffer. The caller can then collect those and update them for each chunk it creates, using themergemethod. And in the very end, we can check whether at any location in the PDF, we've used a limit that is higher than the allowed one. I've already integrated this inkrilla, and it seems to work pretty nicely.It would be nice if the merging would happen in
pdf-writerby forcing the user to pass aBufinstead ofVec<u8>wherever possible (for example, when writing a content stream), but the problem is that the user has to be able to add arbirary data to a stream (since any kind of binary data can appear in a stream, instead of just content streams), and the user currently has no way of constructing aBufmanually using a vector of bytes. I could change that (and if I do that I might as well implementDerefMutfor Buf so that I don't need to wrapreservefor example), but I think it's fine if for now, the user has to merge the limits themselves manually. Let me know if you think this would be better, though.If you are happy with the overall approach, I'll try to add some test cases for limits merging, too.