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

Reply via email to