<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [PATCH] Add support for TextMarkup Annotations in glib frontend"
href="https://bugs.freedesktop.org/show_bug.cgi?id=51487#c26">Comment # 26</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [PATCH] Add support for TextMarkup Annotations in glib frontend"
href="https://bugs.freedesktop.org/show_bug.cgi?id=51487">bug 51487</a>
from <span class="vcard"><a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=88273" name="attach_88273" title="glib: Add PopplerAnnotTextMarkup class and subtypes">attachment 88273</a> <a href="attachment.cgi?id=88273&action=edit" title="glib: Add PopplerAnnotTextMarkup class and subtypes">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=51487&attachment=88273'>[review]</a>
glib: Add PopplerAnnotTextMarkup class and subtypes
Review of <span class=""><a href="attachment.cgi?id=88273" name="attach_88273" title="glib: Add PopplerAnnotTextMarkup class and subtypes">attachment 88273</a> <a href="attachment.cgi?id=88273&action=edit" title="glib: Add PopplerAnnotTextMarkup class and subtypes">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=51487&attachment=88273'>[review]</a>:
-----------------------------------------------------------------
::: glib/poppler-annot.cc
@@ +325,5 @@
<span class="quote">> + * located on @rect when added to a page. See poppler_page_add_annot()
> + *
> + * Return value: A newly created #PopplerAnnotTextMarkup annotation
> + *
> + * Since: 0.25</span >
0.26
@@ +329,5 @@
<span class="quote">> + * Since: 0.25
> + */
> +PopplerAnnot *
> +poppler_annot_text_markup_new_highlight (PopplerDocument *doc,
> + PopplerRectangle *rect)</span >
quad points is required field in text markup annot dict, so I think the
constructors should receive the quadrilaterals.
@@ +1536,5 @@
<span class="quote">> + * Since: 0.25
> + **/
> +void
> +poppler_annot_text_markup_set_quadrilaterals (PopplerAnnotTextMarkup *poppler_annot,
> + GList *quad_list)</span >
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 @@
<span class="quote">> + 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 (</span >
This will be freed by AnnotQuadrilaterals destructor with gfree, you should use
gmalloc intead.
@@ +1594,5 @@
<span class="quote">> +
> + annot = static_cast<AnnotTextMarkup *>(POPPLER_ANNOT (poppler_annot)->annot);
> +
> + back_quads = annot->getQuadrilaterals();
> + length_quads = back_quads->getQuadrilateralsLength();</span >
Same here, the size is known, we can return an array instead of a GList.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>