[poppler] Annot Improving (VIII)

Jeff Muizelaar jeff at infidigm.net
Sun Dec 30 13:56:22 PST 2007


On Sun, Dec 30, 2007 at 09:32:05PM +0100, Iñigo Martínez wrote:
> Hi:
> 
> Here goes two new patches. The first one improves AnnotLink support,
> anyway it lacks a lot of features. The best option should be to merge
> the actual Link implementation with the AnnotLink one.
> 
> The second patch adds support for AnnotFreeText.
> 
> I have tested both (I have figured out the old problems, and now I can
> test them correctly, anyway I have a few glib test on the way) but I
> would prefer a review.

I had a quick look and things looked pretty good. My comments follow
below.

-Jeff

> diff --git a/poppler/Annot.cc b/poppler/Annot.cc
> index da6cca9..0cb7641 100644
> --- a/poppler/Annot.cc
> +++ b/poppler/Annot.cc
> @@ -82,6 +82,22 @@ AnnotExternalDataType parseAnnotExternalData(Dict* dict) {
>  }
>  
>  //------------------------------------------------------------------------
> +// AnnotQuadPoints
> +//------------------------------------------------------------------------
> +
> +AnnotQuadPoints::AnnotQuadPoints(double x1, double y1, double x2, double y2,
> +    double x3, double y3, double x4, double y4) {
> +  this->x1 = x1;
> +  this->y1 = y1;
> +  this->x2 = x2;
> +  this->y2 = y2;
> +  this->x3 = x3;
> +  this->y3 = y3;
> +  this->x4 = x4;
> +  this->y4 = y4;
> +}
> +
> +//------------------------------------------------------------------------
>  // AnnotBorder
>  //------------------------------------------------------------------------
>  AnnotBorder::AnnotBorder() {
> @@ -1975,6 +1991,106 @@ AnnotLink::AnnotLink(XRef *xrefA, Dict *acroForm, Dict *dict, Catalog *catalog,
>      Annot(xrefA, acroForm, dict, catalog, obj) {
>  
>    type = typeLink;
> +  initialize (xrefA, catalog, dict);
> +}
> +
> +AnnotLink::~AnnotLink() {
> +  /*
> +  if (actionDict)
> +    delete actionDict;
> +
> +  if (uriAction)
> +    delete uriAction;
> +  */
> +  if(quadrilaterals) {
> +    for(int i = 0; i < quadrilateralsLength; i++)
> +      delete quadrilaterals[i];
> +
> +    gfree (quadrilaterals);
> +  }
> +}
> +
> +void AnnotLink::initialize(XRef *xrefA, Catalog *catalog, Dict *dict) {
> +  Object obj1;
> +  /*
> +  if (dict->lookup("A", &obj1)->isDict()) {
> +    actionDict = NULL;
> +  } else {
> +    actionDict = NULL;
> +  }
> +  obj1.free();
> +  */
> +  if (dict->lookup("H", &obj1)->isName()) {
> +    GooString *effect = new GooString(obj1.getName());
> +
> +    if (!effect->cmp("N")) {
> +      linkEffect = effectNone;
> +    } else if (!effect->cmp("I")) {
> +      linkEffect = effectInvert;
> +    } else if (!effect->cmp("O")) {
> +      linkEffect = effectOutline;
> +    } else if (!effect->cmp("P")) {
> +      linkEffect = effectPush;
> +    } else {
> +      linkEffect = effectInvert;
> +    }
> +    delete effect;
> +  } else {
> +    linkEffect = effectInvert;
> +  }
> +  obj1.free();
> +  /*
> +  if (dict->lookup("PA", &obj1)->isDict()) {
> +    uriAction = NULL;
> +  } else {
> +    uriAction = NULL;
> +  }
> +  obj1.free();
> +  */
> +  /*
> +   * TODO:
> +   * QuadPoints should be ignored if any coordinate in the array lies outside
> +   * the region specified by Rect.
> +   */
> +  if(dict->lookup("QuadPoints", &obj1)->isArray()) {
> +    quadrilateralsLength = obj1.arrayGetLength();
> +    if((quadrilateralsLength % 8) == 0) {
> +      Object obj2;
> +
> +      quadrilaterals = (AnnotQuadPoints **) gmallocn
> +        ((quadrilateralsLength / 8), sizeof(AnnotQuadPoints *));

maybe use: new (AnnotQuadPoints *)[quadrilateralsLength/8]? Though I'm
not sure what the existing code tends to do. Likewise, should we be
checking to make sure that quadrilateralsLength is not enormous? Again,
if similar exisiting code doesn't check, it's probably ok. In fact, the
general array handling code should perhaps deal with the problem
anyways...
	
> +      for(int i = 0; i < quadrilateralsLength; i += 8) {
> +        double x1, y1, x2, y2, x3, y3, x4, y4;
> +
> +        (obj1.arrayGet(i, &obj2)->isNum() ? x1 = obj2.getNum() : x1 = 0);
> +        obj2.free();
> +        (obj1.arrayGet((i + 1), &obj2)->isNum() ? y1 = obj2.getNum() : y1 = 0);
> +        obj2.free();
> +        (obj1.arrayGet((i + 2), &obj2)->isNum() ? x2 = obj2.getNum() : x2 = 0);
> +        obj2.free();
> +        (obj1.arrayGet((i + 3), &obj2)->isNum() ? y2 = obj2.getNum() : y2 = 0);
> +        obj2.free();
> +        (obj1.arrayGet((i + 4), &obj2)->isNum() ? x3 = obj2.getNum() : x3 = 0);
> +        obj2.free();
> +        (obj1.arrayGet((i + 5), &obj2)->isNum() ? y3 = obj2.getNum() : y3 = 0);
> +        obj2.free();
> +        (obj1.arrayGet((i + 6), &obj2)->isNum() ? x4 = obj2.getNum() : x4 = 0);
> +        obj2.free();
> +        (obj1.arrayGet((i + 7), &obj2)->isNum() ? y4 = obj2.getNum() : y4 = 0);
> +        obj2.free();
> +
> +        quadrilaterals[i / 8] =
> +          new AnnotQuadPoints(x1, y1, x2, y2, x3, y3, x4, y4);
> +      }

This changes the meaning of quadrilateralsLength. Splitting the two
meanings into two variables is probably better. ie. using a local
quadrilateralsLength variable above and a quadrilateralCount member
variable for this.

> +      quadrilateralsLength /= 8;
> +    } else {
> +      quadrilaterals = NULL;
> +      quadrilateralsLength = 0;
> +    }
> +  } else {
> +    quadrilaterals = NULL;
> +  }
> +  obj1.free();
>  }
>  
>  //------------------------------------------------------------------------
> diff --git a/poppler/Annot.h b/poppler/Annot.h
> index 7456160..16a907d 100644
> --- a/poppler/Annot.h
> +++ b/poppler/Annot.h
> @@ -28,6 +28,29 @@ enum AnnotExternalDataType {
>  };
>  
>  //------------------------------------------------------------------------
> +// AnnotQuadPoints
> +//------------------------------------------------------------------------
> +
> +class AnnotQuadPoints {
> +public:
> +  
> +  AnnotQuadPoints(double x1, double y1, double x2, double y2, double x3,
> +      double y3, double x4, double y4);
> +
> +  double getX1() const { return x1; }
> +  double getY1() const { return y1; }
> +  double getX2() const { return x2; }
> +  double getY2() const { return y2; }
> +  double getX3() const { return x3; }
> +  double getY3() const { return y3; }
> +  double getX4() const { return x4; }
> +  double getY4() const { return y4; }
> +
> +protected:
> +  double x1, y1, x2, y2, x3, y3, x4, y4;
> +};
> +
> +//------------------------------------------------------------------------
>  // AnnotBorder
>  //------------------------------------------------------------------------
>  
> @@ -410,7 +433,35 @@ private:
>  class AnnotLink: public Annot {
>  public:
>  
> +  enum AnnotLinkEffect {
> +    effectNone,     // N
> +    effectInvert,   // I
> +    effectOutline,  // O
> +    effectPush      // P
> +  };
> +
>    AnnotLink(XRef *xrefA, Dict *acroForm, Dict *dict, Catalog *catalog, Object *obj);
> +  virtual ~AnnotLink();
> +
> +  // getters
> +  Dict *getActionDict() const { return actionDict; }
> +  // getDest
> +  AnnotLinkEffect getLinkEffect() const { return linkEffect; }
> +  Dict *getUriAction() const { return uriAction; }
> +  AnnotQuadPoints **getQuadrilaterals() const { return quadrilaterals; }
> +  int getQuadrilateralsLength() const { return quadrilateralsLength; }
> +
> +protected:
> +
> +  void initialize(XRef *xrefA, Catalog *catalog, Dict *dict);
> +
> +  Dict *actionDict;                 // A
> +  //Dest
> +  AnnotLinkEffect linkEffect;       // H          (Default I)
> +  Dict *uriAction;                  // PA
> +
> +  AnnotQuadPoints **quadrilaterals; // QuadPoints
> +  int quadrilateralsLength;
>  };
>  
>  //------------------------------------------------------------------------
> -- 
> 1.5.3.7
> 
> 



> diff --git a/poppler/Annot.cc b/poppler/Annot.cc
> index 0cb7641..8165a05 100644
> --- a/poppler/Annot.cc
> +++ b/poppler/Annot.cc
> @@ -60,6 +60,34 @@
>  // = (4 * (sqrt(2) - 1) / 3) * r
>  #define bezierCircle 0.55228475


Missing space characters after the 'if's.

> +AnnotLineEndingStyle parseAnnotLineEndingStyle(GooString *string) {
> +  if(string != NULL) {
> +    if(string->cmp("Square")) {
> +      return annotLineEndingSquare;
> +    } else if(string->cmp("Circle")) {
> +      return annotLineEndingCircle;
> +    } else if(string->cmp("Diamond")) {
> +      return annotLineEndingDiamond;
> +    } else if(string->cmp("OpenArrow")) {
> +      return annotLineEndingOpenArrow;
> +    } else if(string->cmp("ClosedArrow")) {
> +      return annotLineEndingClosedArrow;
> +    } else if(string->cmp("Butt")) {
> +      return annotLineEndingButt;
> +    } else if(string->cmp("ROpenArrow")) {
> +      return annotLineEndingROpenArrow;
> +    } else if(string->cmp("RClosedArrow")) {
> +      return annotLineEndingRClosedArrow;
> +    } else if(string->cmp("Slash")) {
> +      return annotLineEndingSlash;
> +    } else {
> +      return annotLineEndingNone;
> +    }
> +  } else {
> +    return annotLineEndingNone;
> +  }  
> +}
> +
>  AnnotExternalDataType parseAnnotExternalData(Dict* dict) {
>    Object obj1;
>    AnnotExternalDataType type;
> @@ -82,6 +110,55 @@ AnnotExternalDataType parseAnnotExternalData(Dict* dict) {
>  }
>  
>  //------------------------------------------------------------------------
> +// AnnotBorderEffect
> +//------------------------------------------------------------------------
> +
> +AnnotBorderEffect::AnnotBorderEffect(Dict *dict) {
> +  Object obj1;
> +
> +  if (dict->lookup("S", &obj1)->isName()) {
> +    GooString *effectName = new GooString(obj1.getName());
> +
> +    if(!effectName->cmp("C"))

missing space after 'if'

> +      effectType = borderEffectCloudy;
> +    else
> +      effectType = borderEffectNoEffect;
> +    delete effectName;
> +  } else {
> +    effectType = borderEffectNoEffect;
> +  }
> +  obj1.free();
> +
> +  if ((dict->lookup("I", &obj1)->isNum()) && effectType == borderEffectCloudy) {
> +    intensity = obj1.getNum();
> +  } else {
> +    intensity = 0;
> +  }
> +  obj1.free();
> +}
> +
> +//------------------------------------------------------------------------
> +// AnnotCalloutLine
> +//------------------------------------------------------------------------
> +
> +AnnotCalloutLine::AnnotCalloutLine(double x1, double y1, double x2, double y2) {
> +  this->x1 = x1;
> +  this->y1 = y1;
> +  this->x2 = x2;
> +  this->y2 = y2;
> +}
> +
> +//------------------------------------------------------------------------
> +// AnnotCalloutMultiLine
> +//------------------------------------------------------------------------
> +
> +AnnotCalloutMultiLine::AnnotCalloutMultiLine(double x1, double y1, double x2,
> +    double y2, double x3, double y3) : AnnotCalloutLine(x1, y1, x2, y2) {
> +  this->x3 = x3;
> +  this->y3 = y3;
> +}
> +
> +//------------------------------------------------------------------------
>  // AnnotQuadPoints
>  //------------------------------------------------------------------------
>  
> @@ -2094,6 +2171,159 @@ void AnnotLink::initialize(XRef *xrefA, Catalog *catalog, Dict *dict) {
>  }
>  
>  //------------------------------------------------------------------------
> +// AnnotFreeText
> +//------------------------------------------------------------------------
> +
> +AnnotFreeText::AnnotFreeText(XRef *xrefA, Dict *acroForm, Dict *dict, Catalog *catalog, Object *obj) :
> +    Annot(xrefA, acroForm, dict, catalog, obj), AnnotMarkup(xref, acroForm, dict, catalog, obj) {
> +  type = typeFreeText;
> +  initialize(xrefA, catalog, dict);
> +}
> +
> +AnnotFreeText::~AnnotFreeText() {
> +  delete appearanceString;
> +
> +  if (styleString)
> +    delete styleString;
> +
> +  if (calloutLine)
> +    delete calloutLine;
> +
> +  if (borderEffect)
> +    delete borderEffect;
> +
> +  if (rectangle)
> +    delete rectangle;
> +}
> +
> +void AnnotFreeText::initialize(XRef *xrefA, Catalog *catalog, Dict *dict) {
> +  Object obj1;
> +
> +  if (dict->lookup("DA", &obj1)->isString()) {
> +    appearanceString = obj1.getString()->copy();
> +  } else {
> +    appearanceString = new GooString();
> +    error(-1, "Bad appearance for annotation");
> +    ok = gFalse;
> +  }
> +  obj1.free();
> +
> +  if (dict->lookup("Q", &obj1)->isInt()) {
> +    quadding = (AnnotFreeTextQuadding) obj1.getInt();
> +  } else {
> +    quadding = quaddingLeftJustified;
> +  }
> +  obj1.free();
> +
> +  if (dict->lookup("DS", &obj1)->isString()) {
> +    styleString = obj1.getString()->copy();
> +  } else {
> +    styleString = NULL;
> +  }
> +  obj1.free();
> +
> +  if (dict->lookup("CL", &obj1)->isArray() && obj1.arrayGetLength() >= 4) {
> +    double x1, y1, x2, y2;
> +    Object obj2;
> +
> +    (obj1.arrayGet(0, &obj2)->isNum() ? x1 = obj2.getNum() : x1 = 0);
> +    obj2.free();
> +    (obj1.arrayGet(1, &obj2)->isNum() ? y1 = obj2.getNum() : y1 = 0);
> +    obj2.free();
> +    (obj1.arrayGet(2, &obj2)->isNum() ? x2 = obj2.getNum() : x2 = 0);
> +    obj2.free();
> +    (obj1.arrayGet(3, &obj2)->isNum() ? y2 = obj2.getNum() : y2 = 0);
> +    obj2.free();
> +
> +    if(obj1.arrayGetLength() == 6) {
> +      double x3, y3;
> +      (obj1.arrayGet(4, &obj2)->isNum() ? x3 = obj2.getNum() : x3 = 0);
> +      obj2.free();
> +      (obj1.arrayGet(5, &obj2)->isNum() ? y3 = obj2.getNum() : y3 = 0);
> +      obj2.free();
> +      calloutLine = new AnnotCalloutMultiLine(x1, y1, x2, y2, x3, y3);
> +    } else {
> +      calloutLine = new AnnotCalloutLine(x1, y1, x2, y2);
> +    }
> +  } else {
> +    calloutLine = NULL;
> +  }
> +  obj1.free();
> +
> +  if (dict->lookup("IT", &obj1)->isName()) {
> +    GooString *intentName = new GooString(obj1.getName());
> +
> +    if (!intentName->cmp("FreeText")) {
> +      intent = intentFreeText;
> +    } else if (!intentName->cmp("FreeTextCallout")) {
> +      intent = intentFreeTextCallout;
> +    } else if (!intentName->cmp("FreeTextTypeWriter")) {
> +      intent = intentFreeTextTypeWriter;
> +    } else {
> +      intent = intentFreeText;
> +    }
> +    delete intentName;
> +  } else {
> +    intent = intentFreeText;
> +  }
> +  obj1.free();
> +
> +  if (dict->lookup("BE", &obj1)->isDict()) {
> +    borderEffect = new AnnotBorderEffect(obj1.getDict());
> +  } else {
> +    borderEffect = NULL;
> +  }
> +  obj1.free();
> +
> +  if (dict->lookup("RD", &obj1)->isArray() && obj1.arrayGetLength() == 4) {
> +    Object obj2;
> +    rectangle = new PDFRectangle();
> +
> +    (obj1.arrayGet(0, &obj2)->isNum() ? rectangle->x1 = obj2.getNum() :
> +      rectangle->x1 = 0);
> +    obj2.free();
> +    (obj1.arrayGet(1, &obj2)->isNum() ? rectangle->y1 = obj2.getNum() :
> +      rectangle->y1 = 0);
> +    obj2.free();
> +    (obj1.arrayGet(2, &obj2)->isNum() ? rectangle->x2 = obj2.getNum() :
> +      rectangle->x2 = 1);
> +    obj2.free();
> +    (obj1.arrayGet(3, &obj2)->isNum() ? rectangle->y2 = obj2.getNum() :
> +      rectangle->y2 = 1);
> +    obj2.free();
> +
> +    if (rectangle->x1 > rectangle->x2) {
> +      double t = rectangle->x1;
> +      rectangle->x1 = rectangle->x2;
> +      rectangle->x2 = t;
> +    }
> +    if (rectangle->y1 > rectangle->y2) {
> +      double t = rectangle->y1;
> +      rectangle->y1 = rectangle->y2;
> +      rectangle->y2 = t;
> +    }
> +
> +    if ((rectangle->x1 + rectangle->x2) > (rect->x2 - rect->x1))
> +      rectangle->x1 = rectangle->x2 = 0;
> +
> +    if ((rectangle->y1 + rectangle->y2) > (rect->y2 - rect->y1))
> +      rectangle->y1 = rectangle->y2 = 0;
> +  } else {
> +    rectangle = NULL;
> +  }
> +  obj1.free();
> +
> +  if (dict->lookup("LE", &obj1)->isName()) {
> +    GooString *styleName = new GooString(obj1.getName());
> +    endStyle = parseAnnotLineEndingStyle(styleName);
> +    delete styleName;
> +  } else {
> +    endStyle = annotLineEndingNone;
> +  }
> +  obj1.free();
> +}
> +
> +//------------------------------------------------------------------------
>  // Annots
>  //------------------------------------------------------------------------
>  
> @@ -2147,7 +2377,7 @@ Annot *Annots::createAnnot(XRef *xref, Dict *acroForm, Dict* dict, Catalog *cata
>      } else if(!typeName->cmp("Link")) {
>        annot = new AnnotLink(xref, acroForm, dict, catalog, obj);
>      } else if(!typeName->cmp("FreeText")) {
> -      annot = new Annot(xref, acroForm, dict, catalog, obj);
> +      annot = new AnnotFreeText(xref, acroForm, dict, catalog, obj);
>      } else if(!typeName->cmp("Line")) {
>        annot = new Annot(xref, acroForm, dict, catalog, obj);
>      } else if(!typeName->cmp("Square")) {
> diff --git a/poppler/Annot.h b/poppler/Annot.h
> index 16a907d..e0f2971 100644
> --- a/poppler/Annot.h
> +++ b/poppler/Annot.h
> @@ -22,12 +22,86 @@ class GfxFontDict;
>  class FormWidget;
>  class PDFRectangle;
>  
> +enum AnnotLineEndingStyle {
> +  annotLineEndingSquare,        // Square
> +  annotLineEndingCircle,        // Circle
> +  annotLineEndingDiamond,       // Diamond
> +  annotLineEndingOpenArrow,     // OpenArrow
> +  annotLineEndingClosedArrow,   // ClosedArrow
> +  annotLineEndingNone,          // None
> +  annotLineEndingButt,          // Butt
> +  annotLineEndingROpenArrow,    // ROpenArrow
> +  annotLineEndingRClosedArrow,  // RClosedArrow
> +  annotLineEndingSlash          // Slash
> +};
> +
>  enum AnnotExternalDataType {
>    annotExternalDataMarkupUnknown,
>    annotExternalDataMarkup3D       // Markup3D
>  };
>  
>  //------------------------------------------------------------------------
> +// AnnotCalloutLine
> +//------------------------------------------------------------------------
> +
> +class AnnotCalloutLine {
> +public:
> +
> +  AnnotCalloutLine(double x1, double y1, double x2, double y2);
> +  virtual ~AnnotCalloutLine() { }
> +
> +  double getX1() const { return x1; }
> +  double getY1() const { return y1; }
> +  double getX2() const { return x2; }
> +  double getY2() const { return y2; }
> +  
> +protected:
> +
> +  double x1, y1, x2, y2;
> +};
> +
> +//------------------------------------------------------------------------
> +// AnnotCalloutMultiLine
> +//------------------------------------------------------------------------
> +
> +class AnnotCalloutMultiLine: public AnnotCalloutLine {
> +public:
> +
> +  AnnotCalloutMultiLine(double x1, double y1, double x2, double y2,
> +    double x3, double y3);
> +
> +  double getX3() const { return x3; }
> +  double getY3() const { return y3; }
> +
> +protected:
> +
> +  double x3, y3;
> +};
> +
> +//------------------------------------------------------------------------
> +// AnnotBorderEffect
> +//------------------------------------------------------------------------
> +
> +class AnnotBorderEffect {
> +public:
> +
> +  enum AnnotBorderEffectType {
> +    borderEffectNoEffect, // S
> +    borderEffectCloudy    // C
> +  };
> +
> +  AnnotBorderEffect(Dict *dict);
> +
> +  AnnotBorderEffectType getEffectType() const { return effectType; }
> +  double getIntensity() const { return intensity; }
> +
> +private:
> +
> +  AnnotBorderEffectType effectType; // S  (Default S)
> +  double intensity;                 // I  (Default 0)
> +};
> +
> +//------------------------------------------------------------------------
>  // AnnotQuadPoints
>  //------------------------------------------------------------------------
>  
> @@ -465,6 +539,59 @@ protected:
>  };
>  
>  //------------------------------------------------------------------------
> +// AnnotFreeText
> +//------------------------------------------------------------------------
> +
> +class AnnotFreeText: public Annot, public AnnotMarkup {
> +public:
> +
> +  enum AnnotFreeTextQuadding {
> +    quaddingLeftJustified,  // 0
> +    quaddingCentered,       // 1
> +    quaddingRightJustified  // 2
> +  };
> +
> +  enum AnnotFreeTextIntent {
> +    intentFreeText,           // FreeText
> +    intentFreeTextCallout,    // FreeTextCallout
> +    intentFreeTextTypeWriter  // FreeTextTypeWriter
> +  };
> +
> +  AnnotFreeText(XRef *xrefA, Dict *acroForm, Dict *dict, Catalog *catalog, Object *obj);
> +  virtual ~AnnotFreeText();
> +
> +  // getters
> +  GooString *getAppearanceString() const { return appearanceString; }
> +  AnnotFreeTextQuadding getQuadding() const { return quadding; }
> +  // return rc
> +  GooString *getStyleString() const { return styleString; }
> +  AnnotCalloutLine *getCalloutLine() const {  return calloutLine; }
> +  AnnotFreeTextIntent getIntent() const { return intent; }
> +  AnnotBorderEffect *getBorderEffect() const { return borderEffect; }
> +  PDFRectangle *getRectangle() const { return rectangle; }
> +  AnnotLineEndingStyle getEndStyle() const { return endStyle; }
> +
> +protected:
> +
> +  void initialize(XRef *xrefA, Catalog *catalog, Dict *dict);
> +
> +  // required
> +  GooString *appearanceString;      // DA
> +
> +  // optional
> +  AnnotFreeTextQuadding quadding;   // Q  (Default 0)
> +  // RC
> +  GooString *styleString;           // DS
> +  AnnotCalloutLine *calloutLine;    // CL
> +  AnnotFreeTextIntent intent;       // IT
> +  AnnotBorderEffect *borderEffect;  // BE
> +  PDFRectangle *rectangle;          // RD
> +  // inherited  from Annot
> +  // AnnotBorderBS border;          // BS
> +  AnnotLineEndingStyle endStyle;    // LE (Default None)
> +};
> +
> +//------------------------------------------------------------------------
>  // Annots
>  //------------------------------------------------------------------------
>  
> -- 
> 1.5.3.7
> 


More information about the poppler mailing list