[Poppler-bugs] [Bug 51487] [PATCH] Add support for TextMarkup Annotations in glib frontend

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=51487

--- Comment #26 from Carlos Garcia Campos <carlosgc at gnome.org> ---
Comment on attachment 88273
  --> https://bugs.freedesktop.org/attachment.cgi?id=88273
glib: Add PopplerAnnotTextMarkup class and subtypes

Review of attachment 88273:
-----------------------------------------------------------------

::: glib/poppler-annot.cc
@@ +325,5 @@
> + * located on @rect when added to a page. See poppler_page_add_annot()
> + *
> + * Return value: A newly created #PopplerAnnotTextMarkup annotation
> + *
> + * Since: 0.25

0.26

@@ +329,5 @@
> + * Since: 0.25
> + */
> +PopplerAnnot *
> +poppler_annot_text_markup_new_highlight (PopplerDocument  *doc,
> +					 PopplerRectangle *rect)

quad points is required field in text markup annot dict, so I think the
constructors should receive the quadrilaterals.

@@ +1536,5 @@
> + * Since: 0.25
> + **/
> +void
> +poppler_annot_text_markup_set_quadrilaterals (PopplerAnnotTextMarkup *poppler_annot,
> +                                              GList *quad_list)

The length of the quad list is usually known, so I think we could use an array
instead of a GList. This way the PopplerQuadrilateral structs can be stack
allocated.

@@ +1549,5 @@
> +  g_return_if_fail (POPPLER_IS_ANNOT_TEXT_MARKUP (poppler_annot));
> +
> +  annot = static_cast<AnnotTextMarkup *>(POPPLER_ANNOT (poppler_annot)->annot);
> +
> +  quads = (AnnotQuadrilaterals::AnnotQuadrilateral **) g_new0 (

This will be freed by AnnotQuadrilaterals destructor with gfree, you should use
gmalloc intead.

@@ +1594,5 @@
> +
> +  annot = static_cast<AnnotTextMarkup *>(POPPLER_ANNOT (poppler_annot)->annot);
> +
> +  back_quads = annot->getQuadrilaterals();
> +  length_quads = back_quads->getQuadrilateralsLength();

Same here, the size is known, we can return an array instead of a GList.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20131103/5b619284/attachment.html>