Also for some places might be useful to have member function in PdfObject
which will dereference it in case of reference. Something like this:
PdfObject *PdfObject::Deref()
{
*if*( !IsReference() ) *return* *this*;
*if*( !m_pOwner )
{
PODOFO_RAISE_ERROR_INFO( ePdfError_InvalidHandle, "Object is a reference
but does not have an owner!" );
}
*return* m_pOwner->GetObject( GetReference() );
}
And maybe MustDeref which will throw instead of returning null in case of
reference to non-existing object.
On Fri, Feb 21, 2020 at 7:27 PM Michal Sudolsky <sudols...@gmail.com> wrote:
> Hi,
>
> Why is automatic object ownership still not merged into master? When it
> will be?
>
> That thing which I mentioned was here in similar form also before:
>
> *if* (descendant.size() && descendant[0].IsReference())
>
> {
>
> pFontObject = pObject->GetOwner()->GetObject( descendant[0].
> GetReference() );
>
>
> pDescriptor = pFontObject->GetIndirectKey( "FontDescriptor" );
>
> }
>
> And these changes make it easier to fix other problems like:
>
> - parameters of stream like Length, Filter and DecodeParams can be either
> direct or indirect but only Length is properly handled, others are passed
> as PdfDictionary into PdfFilterFactory::CreateDecodeStream so without this
> path would be needed to make bigger changes to fix this
> - handling of arrays which can contain both direct and indirect objects,
> FindAt can really help here but maybe I would add also MustFindAt which
> will throw if index is out of bounds or reference to non-existing object
> and MustFindKey like similar MustGetKey for dictionaries because there are
> many places where code expect that non-null object is returned, this
> problem is correctly handled only on few places and incorrectly on many
> more like that example in PdfFontFactory where it either expect always
> direct object or always indirect object
>
> Maybe would be good to also have possibility to have indirection
> automatically resolved in enumerations.
>
> Most places in podofo really does not want or need PdfReference but rather
> referenced object. There are much less places which really wants references
> like copying/sharing resources without duplications.
>
> On Tue, Feb 18, 2020 at 7:06 PM Michal Sudolsky <sudols...@gmail.com>
> wrote:
>
>> 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
