[poppler] [Patch] Parse Additional Actions for Widget Annots.
jose.aliste at gmail.com
jose.aliste at gmail.com
Thu Mar 31 05:19:24 PDT 2011
Hi,
now patch 1 contains the memleak fix. and patch2 is the updated patch
addressing review comments.
Greets
José
On Thu, Mar 31, 2011 at 6:57 AM, Carlos Garcia Campos
<carlosgc at gnome.org> wrote:
> Excerpts from jose.aliste at gmail.com's message of mié mar 30 20:50:44 +0200 2011:
>> Carlos, thanks for the review.
>>
>> Here is an updated patch. I finally decided to have only one enum for
>> TriggerEvents, so we can have methods
>> AnnotScreen::getAdditionAction(AnnotTriggerEvent event)
>> and AnnotWidget::getAdditionAction(AnnotTriggerEvent event)
>> of giving eventFocusOut or eventFocusIn to a screen Annotation will
>> just give you a NULL.
>>
>> I changed to use a stack allocated pointer of LinkActions. If you feel
>> there is a better data struct for this, let me know.
>>
>
>> From e1576c8d6be31721ffeab52c4aa936eef1195d19 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Jos=C3=A9=20Aliste?= <jaliste at src.gnome.org>
>> Date: Tue, 29 Mar 2011 04:27:15 -0400
>> Subject: [PATCH] Parse additionActions dictionary for Widget annots.
>
>> ---
>> poppler/Annot.cc | 102 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>> poppler/Annot.h | 31 +++++++++++++---
>> 2 files changed, 118 insertions(+), 15 deletions(-)
>
>> diff --git a/poppler/Annot.cc b/poppler/Annot.cc
>> index 1f77c71..faab471 100644
>> --- a/poppler/Annot.cc
>> +++ b/poppler/Annot.cc
>> @@ -1350,6 +1350,72 @@ void Annot::draw(Gfx *gfx, GBool printing) {
>> obj.free();
>> }
>
>> +// Parse Actions in AdditionActions dictionary that are common to Screen and Widget annots
>> +void Annot::parseAdditionActions (LinkAction **additionActions, Object *addActionDict, GooString *baseURI)
>> +{
>> + Object obj2;
>> +
>> + if (addActionDict->dictLookup("E", &obj2)->isDict())
>> + additionActions[eventCursorEnter] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + if (addActionDict->dictLookup("X", &obj2)->isDict())
>> + additionActions[eventCursorExit] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + if (addActionDict->dictLookup("D", &obj2)->isDict())
>> + additionActions[eventMouseDown] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + if (addActionDict->dictLookup("U", &obj2)->isDict())
>> + additionActions[eventMouseUp] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + if (addActionDict->dictLookup("PO", &obj2)->isDict())
>> + additionActions[eventPageOpen] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + if (addActionDict->dictLookup("PC", &obj2)->isDict())
>> + additionActions[eventPageClose] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + if (addActionDict->dictLookup("PV", &obj2)->isDict())
>> + additionActions[eventPageVisible] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + if (addActionDict->dictLookup("PI", &obj2)->isDict())
>> + additionActions[eventPageInvisible] = LinkAction::parseAction (&obj2, baseURI);
>> + obj2.free();
>> +
>> + }
>> +
>> +// Delete additionActions that are common to Screen and Widget annots
>> +void Annot::deleteAdditionActions(LinkAction **additionActions) {
>
> You don't need this. See below
>
>> + if (additionActions[eventCursorEnter])
>> + delete additionActions[eventCursorEnter];
>> +
>> + if (additionActions[eventCursorExit])
>> + delete additionActions[eventCursorExit];
>> +
>> + if (additionActions[eventMouseDown])
>> + delete additionActions[eventMouseDown];
>> +
>> + if (additionActions[eventMouseUp])
>> + delete additionActions[eventMouseUp];
>> +
>> + if (additionActions[eventPageOpen])
>> + delete additionActions[eventPageOpen];
>> +
>> + if (additionActions[eventPageClose])
>> + delete additionActions[eventPageClose];
>> +
>> + if (additionActions[eventPageVisible])
>> + delete additionActions[eventPageVisible];
>> +
>> + if (additionActions[eventPageInvisible])
>> + delete additionActions[eventPageInvisible];
>> +}
>> +
>> //------------------------------------------------------------------------
>> // AnnotPopup
>> //------------------------------------------------------------------------
>> @@ -2715,12 +2781,16 @@ AnnotWidget::~AnnotWidget() {
>
>> if (action)
>> delete action;
>> -
>> - if (additionActions)
>> - delete additionActions;
>> -
>> +
>> + Annot::deleteAdditionActions(additionActions);
>> + if (additionActions[eventFocusIn])
>> + delete additionActions[eventFocusIn];
>> + if (additionActions[eventFocusOut])
>> + delete additionActions[eventFocusOut];
>> +
>
> delete is null-safe so here you only need a for loop
>
> for (int i = 0; i < EventsNumber; i++)
> delete additionActions[i];
>
>> if (parent)
>> delete parent;
>> +
>> }
>
> Remove this new line please.
>
>> void AnnotWidget::initialize(XRef *xrefA, Catalog *catalog, Dict *dict) {
>> @@ -2758,10 +2828,20 @@ void AnnotWidget::initialize(XRef *xrefA, Catalog *catalog, Dict *dict) {
>> }
>> obj1.free();
>
>> + memset (additionActions, 0, EventsNumber);
>> +
>> if(dict->lookup("AA", &obj1)->isDict()) {
>> - additionActions = NULL;
>> - } else {
>> - additionActions = NULL;
>> + Object obj2;
>> +
>> + Annot::parseAdditionActions(additionActions, &obj1, catalog->getBaseURI());
>
> You don't need the Annot:: prefix here.
>
>> + if (obj1.dictLookup("Fo", &obj2)->isDict())
>> + additionActions[eventFocusIn] = LinkAction::parseAction(&obj2, catalog->getBaseURI());
>> + obj2.free();
>> +
>> + if (obj1.dictLookup("Bl", &obj2)->isDict())
>> + additionActions[eventFocusOut] = LinkAction::parseAction(&obj2, catalog->getBaseURI());
>> + obj2.free();
>> }
>> obj1.free();
>
>> @@ -4066,7 +4146,7 @@ AnnotScreen::~AnnotScreen() {
>> if (action)
>> delete action;
>
>> - additionAction.free();
>> + Annot::deleteAdditionActions(additionActions);
>
> Ditto.
>
>> }
>
>> void AnnotScreen::initialize(XRef *xrefA, Catalog *catalog, Dict* dict) {
>> @@ -4088,8 +4168,12 @@ void AnnotScreen::initialize(XRef *xrefA, Catalog *catalog, Dict* dict) {
>> ok = gFalse;
>> }
>> }
>> + obj1.free();
>
> This looks unrelated, it there's a memleak there, please use a
> different commit, so that we can push it now.
>
>> - dict->lookup("AA", &additionAction);
>> + memset(additionActions, 0, EventsNumber);
>> + if (dict->lookup("AA", &obj1)->isDict()) {
>> + Annot::parseAdditionActions (additionActions, &obj1, catalog->getBaseURI());
>
> Don't need Annot:: here either.
>
>> + }
>
>> appearCharacs = NULL;
>> if(dict->lookup("MK", &obj1)->isDict()) {
>> diff --git a/poppler/Annot.h b/poppler/Annot.h
>> index 93f82bf..ad31118 100644
>> --- a/poppler/Annot.h
>> +++ b/poppler/Annot.h
>> @@ -453,6 +453,21 @@ public:
>> type3D // 3D 25
>> };
>
>> + enum AnnotTriggerEvent {
>> + eventCursorEnter, // E
>> + eventCursorExit, // X
>> + eventMouseDown, // D
>> + eventMouseUp, // U
>> + eventFocusIn, // Fo
>> + eventFocusOut, // Bl
>> + eventPageOpen, // PO
>> + eventPageClose, // PC
>> + eventPageVisible, // PV
>> + eventPageInvisible, // PI
>> + EventsNumber
>> + };
>> +
>> +
>> Annot(XRef *xrefA, PDFRectangle *rectA, Catalog *catalog);
>> Annot(XRef *xrefA, Dict *dict, Catalog *catalog);
>> Annot(XRef *xrefA, Dict *dict, Catalog *catalog, Object *obj);
>> @@ -525,7 +540,9 @@ protected:
>> void createResourcesDict(char *formName, Object *formStream, char *stateName,
>> double opacity, char *blendMode, Object *resDict);
>> GBool isVisible(GBool printing);
>> -
>> + static void parseAdditionActions(LinkAction **additionActions, Object *additionActionsDict,
>> + GooString *baseURI);
>> + static void deleteAdditionActions (LinkAction **additionActions);
>> // Updates the field key of the annotation dictionary
>> // and sets M to the current time
>> void update(const char *key, Object *value);
>> @@ -721,8 +738,9 @@ class AnnotScreen: public Annot {
>
>> AnnotAppearanceCharacs *getAppearCharacs() { return appearCharacs; }
>> LinkAction* getAction() { return action; }
>> - Object* getAdditionActions() { return &additionAction; }
>> -
>> + LinkAction **getAdditionActions() { return additionActions; }
>> + LinkAction *getAdditionAction(AnnotTriggerEvent event) { return additionActions[event]; }
>
> Please, rename it to AdditionalActions.
>
>> private:
>> void initialize(XRef *xrefA, Catalog *catalog, Dict *dict);
>
> Regards,
> --
> Carlos Garcia Campos
> PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-a-memleak-in-AnnotScreen-initialize.patch
Type: text/x-patch
Size: 641 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20110331/862d7894/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Parse-additionActions-dictionary-for-Widget-annots.patch
Type: text/x-patch
Size: 6947 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20110331/862d7894/attachment-0001.bin>
More information about the poppler
mailing list