Hi,

- Evaluate deprecation of PdfObject::GetIndirectKey() as same
> functionality is now provided directly inside PdfDictionary in
> a more consistent API.


I think GetIndirectKey is still useful as GetDictionary will throw if
object is not dictionary and better is to ignore possibly incorrect pdf
like many pdf software do as there are many broken pdf files. On the other
hand there is also MustGetIndirectKey which is used in many places in cases
when there is really needed non-null pointer.

In this part of patch PdfVecObjects::GetObject may return NULL into
pFontObject (in case of invalid pdf is better to throw than crash):

+ if ( descendant.IsReference() )
+ {
+ pFontObject = pObject->GetOwner()->GetObject( descendant.GetReference() );
+ pDescriptor = pFontObject->GetIndirectKey( "FontDescriptor" );
+ }

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

Reply via email to