Dear Dominik, Matthew, Zyx, and all PoDoFo develpers, I kindly ask some of you to review this work as I consider it important for the future of PoDoFo. It has been extensively tested for almost one year with important number of automatic test cases (100+) by very different users. High quality standards have been followed while designing/developing it and I haven't performed a single modification on this feature/rework after the last iteration so I still consider it robust enough to be merged as is, if the new suggested API is accepted. Also I can take care of the merge into trunk if there's positive feedback about. So far it still applies cleanly with just a minimal conflict with a recent bug fix.
Regards, Francesco On Wed, 15 May 2019 at 18:22, Francesco Pretto <cez...@gmail.com> wrote: > > Hello, > > a branch[1] has been created to add automatic ownership handling > to PdfObject. The issue with current handling of PdfObject ownership > is that it's inconsistent and hacky: objects retrieved directly > from PdfDictionary or PdfArray won't have owner set, even if one > PdfObject is actually storing these containers. The hack currently > performed is to set the owner when retrieving objects trough > PdfObject::GetIndirectKey(), but this work only for dictionaries > (not arrays) and looks fragile as the objects inside dictionaries > have inconsistent state despite being part of the document hierarchy. > > The patch introduce a mechanism to maintain the ownership of PdfObject > automatically by: > - Adding PdfObject ownership to both PdfDictionary and PdfArray by > inheriting a common PdfOwnedDataType; > - Adding a proper semantics for setting owner of PdfObjects: copying > objects won't copy owner (it will stay NULL) and assigning objects will > keep current owner; > - Inserting an object into a PdfDctionary or a PdfArray will set the > ownership in the copied object recursively. > > Some convenience method are also added to PdfArray and PdfDictionaries: > - PdfDictionary::FindKey(key) : find objects following references, > same as previous PdfObject::GetIndirectKey(); > - PdfDictionary::FindKeyParent(key): find objects following references > and "/Parent" references. Useful for PdfField and super classes; > - PdfArray::FindAt(idx) : find objects following references. > > The patch introduces some ABI changes to add some common base class > functionalities to PdfDictionary, PdfArray. It's also potentially API > breaking since PdfObject::SetOwner() is now private. Of course it's > possible to leave it public but I don't recommend it since owner handling > should be totally hidden to API user and the compilation fix > is just to remove the call. > > The branch applies consists of 3 commits: > 1) PdfArray: Removed inheritance from std::vector > 2) PdfObject: Introduced automatic ownership handling > 3) PdfFontFactory: Fix lookup Type 0 fonts with inline DescendantFonts > array > > 1) Clean PdfArray from fishy inheritance from std::vector and follows > approach of PdfDictionary to just wrap it preserving API compatibility. > 2) is the core change while 3) is the first PoDoFo bug fixed taking > advantage of the change that allows to fix it without hacks. > > I have been tested this change for several months now. I also ran > the changeset against unit tests and it succeeded. > > Next steps could be: > - Extensively use PdfDictionary::FindKeyParent(key) in PdfField and super > classes to support fields hierarchies; > - Evaluate deprecation of PdfObject::GetIndirectKey() as same > functionality is now provided directly inside PdfDictionary in > a more consistent API. > > Attached is the class diagram of the proposed PdfOwnedDatatype and the > whole change as a single patch against trunk r1990. > > [1] https://svn.code.sf.net/p/podofo/code/podofo/branches/ObjectAutoOwnership _______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
