<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - glib-demo: Add support for simple line annotations" href="https://bugs.freedesktop.org/show_bug.cgi?id=70981#c2">Comment # 2</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - glib-demo: Add support for simple line annotations" href="https://bugs.freedesktop.org/show_bug.cgi?id=70981">bug 70981</a> from <a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> Carlos Garcia Campos</a> <pre>Comment on <a href="attachment.cgi?id=88267" name="attach_88267" title="glib: Add support for simple line annotations">attachment 88267</a> <a href="attachment.cgi?id=88267&action=edit" title="glib: Add support for simple line annotations">[details]</a> <a href='page.cgi?id=splinter.html&bug=70981&attachment=88267'>[review]</a> glib: Add support for simple line annotations Review of <a href="attachment.cgi?id=88267" name="attach_88267" title="glib: Add support for simple line annotations">attachment 88267</a> <a href="attachment.cgi?id=88267&action=edit" title="glib: Add support for simple line annotations">[details]</a> <a href='page.cgi?id=splinter.html&bug=70981&attachment=88267'>[review]</a>: ----------------------------------------------------------------- ::: glib/poppler-annot.cc @@ +351,5 @@ > +{ > +} > + > +/** > +* poppler_annot_line_new: This line is not correctly indented. @@ +359,5 @@ > + * Creates a new Line annotation that will be > + * located on @rect when added to a page. See > + * poppler_page_add_annot() > + * > + * Return value: A newly #PopplerAnnotLine annotation newly created @@ +361,5 @@ > + * poppler_page_add_annot() > + * > + * Return value: A newly #PopplerAnnotLine annotation > + * > + * Since: 0.25 0.26. @@ +1439,5 @@ > } > + > +/* PopplerAnnotLine */ > +void > +poppler_annot_line_set_width (PopplerAnnotLine *poppler_annot, gdouble width) This should be documented. Use a new line for every parameter- @@ +1447,5 @@ > + g_return_if_fail (POPPLER_IS_ANNOT_LINE (poppler_annot)); > + > + annot = static_cast<AnnotLine *>(POPPLER_ANNOT (poppler_annot)->annot); > + annot->setLeaderLineLength(width); > + annot->setLeaderLineExtension(width); This is confusing, because this only applies to annot lines using leader lines. Note that simply calling this, changes the meaning of the vertices. In a normal lines the width is determined but its vertices, since it's a rectangle in the end. I'm not sure how to expose the leader lines thing, though. @@ +1452,5 @@ > + // annot->setOpacitiy(0.5); > +} > + > +void > +poppler_annot_line_set_vertices (PopplerAnnotLine *poppler_annot, This should be documented too. @@ +1454,5 @@ > + > +void > +poppler_annot_line_set_vertices (PopplerAnnotLine *poppler_annot, > + gdouble x1, gdouble y1, > + gdouble x2, gdouble y2) I wonder if we should to expose PopplerPoint, I guess it will be required if we eventually expose Polygon annotations. @@ +1498,5 @@ > +/** > + * poppler_annot_line_get_start_style: > + * @poppler_annot: a #PopplerAnnotLine > + * > + * Return value: (transfer none): The ending style located at the start of the line. You don't need to use transfer annotations for a method returning an enum. @@ +1524,5 @@ > + * > + * Since: 0.25 > + **/ > +PopplerAnnotLineEndingStyle > +poppler_annot_line_get_end_style (PopplerAnnotLine *poppler_annot) I think I would merge both methods into a single poppler_annot_line_get_ending_style that returns both start and end as out parameters, allowing to pass NULL if you are not interested in any of them. @@ +1556,5 @@ > + > + annot = static_cast<AnnotLine *>(POPPLER_ANNOT (poppler_annot)->annot); > + > + annot->setStartEndStyle ((AnnotLineEndingStyle)start_style, > + (AnnotLineEndingStyle)end_style); Why are we just casting the enums here, but using a convert method in the getters? We can either always cast (adding a compile assert to make sure we break the build if the enums are not in sync) or use converter methods, but not mix it.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>