ENH: Preserve OCGs when merging PDFs

[mTangtrongwanit]

The goal of this PR is to preserve OCGs when appending PDFs together.

In essence, this implementation is based on how while pypdf does track the OCGs via "OC_PROPERTIES" (or "/OCProperties"), it does not preserve them in the merge process.

By translating OCG references from the source PDFs to be appended into the writer to preserve the OCGs, the nested layer ordering and functionality is maintained.

This comes as a result of work by Esri developers to resolve PDF issues in our software and the insights provided in #92.

[stefan6419846]

Thanks for the PR.

I have left some basic inline remarks, but did not do a full review yet as there are too much changes to be done before. In the future, consider commenting on the issue first about possible implementation details.

Besides the inline comments, where I only marked one occurrence of each issue, please have a look at the test failures - your are referencing sample files which do not exist at the moment and where I am not aware of a parallel PR at the corresponding repository at the moment as well.

[mTangtrongwanit]

Thanks for the PR.

I have left some basic inline remarks, but did not do a full review yet as there are too much changes to be done before. In the future, consider commenting on the issue first about possible implementation details.

Besides the inline comments, where I only marked one occurrence of each issue, please have a look at the test failures - your are referencing sample files which do not exist at the moment and where I am not aware of a parallel PR at the corresponding repository at the moment as well.

Thanks for the quick response! There were a number of items I missed in the contribution doc so I apologize for the trouble there.

On that note, I wasn't able to find a process for uploading test files so I just uploaded them to the "resource" folder in my fork though it appears I still need to review the docs on test setup some more.

And to confirm, instead of submitting the PR first, it would have been better to post to the OCG thread (#92) first right?

I'll also review the provided comments as well.

[stefan6419846]

On that note, I wasn't able to find a process for uploading test files so I just uploaded them to the "resource" folder in my fork though it appears I still need to review the docs on test setup some more.

Please avoid uploading such large files to the repository directly. If you are able to provide them under the terms of the sample files repository at https://github.com/py-pdf/sample-files/, you can create a PR there to get them integrated. Otherwise, we would have to download them on the fly, which has its own issues.

In general it is appreciated to have small sample files inside the sample files repository.

And to confirm, instead of submitting the PR first, it would have been better to post to the OCG thread (#92) first right?

As this thread is a larger one, yes, even if it is just about a part of the requested aspects. Especially the correct module to put the code into could have been discussed before, instead of causing both you and me the overhead of at least partially doing it twice.

[mTangtrongwanit]

I do see at least 2 error s on the "windows" pytest ("FileNotFoundError: [Errno 2] No such file or directory: 'D:\a\pypdf\pypdf\resources\ocg_map_test.pdf'") which I hope are resolved once the sample files PR is commited (py-pdf/sample-files#42) as it is from my test file "test_ocg_append.py".

The other error from the "test_workflows.py" is definitely something to dig into though. The error: "Failed: len(actual) = 217,997 vs len(expected) = 217,857" at a glance might indicate that the PDF (with the OCG structures preserved) is bigger than the test's "expected" amount but might be inline with the new OCG preserving behavior?

[stefan6419846]

As soon as the associated sample files is merged, we can update the submodule commit in pypdf and thus allow the new file to be used. Until this is done, I will not look into further reviewing this PR.

The other error from the "test_workflows.py" is definitely something to dig into though. The error: "Failed: len(actual) = 217,997 vs len(expected) = 217,857" at a glance might indicate that the PDF (with the OCG structures preserved) is bigger than the test's "expected" amount but might be inline with the new OCG preserving behavior?

I have not checked whether the original file had OCGs. To resolve this, I would recommend you to run the corresponding test locally and create a proper diff on the data (this would fail CI for too large data). Based on this diff, we can check further.