Hi, There were added two "FIXME" comments like this:
- if( rhs.m_pStream && m_pOwner ) - m_pStream = m_pOwner->CreateStream( *(rhs.m_pStream) ); + // FIXME: + // Copying stream is currently broken: + // 1) PdfVecObjects::CreateStream( const PdfStream & ) is broken as it just returns NULL + // 2) Stream should be copyable also when m_pOwner is NULL (which is the case for copy constructor) + //if( rhs.m_pStream && m_pOwner ) + // m_pStream = m_pOwner->CreateStream( *(rhs.m_pStream) ); Does this fix Introduce some new breakage or was such situation also before? On Mon, Oct 28, 2019 at 11:22 PM Francesco Pretto <cez...@gmail.com> wrote: > 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 >
_______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
