<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - Add getter and setter for annotation's rectangle" href="https://bugs.freedesktop.org/show_bug.cgi?id=70901#c1">Comment # 1</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - Add getter and setter for annotation's rectangle" href="https://bugs.freedesktop.org/show_bug.cgi?id=70901">bug 70901</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=88161" name="attach_88161" title="glib: Add getter and setter for annotation's rectangle">attachment 88161</a> <a href="attachment.cgi?id=88161&action=edit" title="glib: Add getter and setter for annotation's rectangle">[details]</a> <a href='page.cgi?id=splinter.html&bug=70901&attachment=88161'>[review]</a> glib: Add getter and setter for annotation's rectangle Review of <a href="attachment.cgi?id=88161" name="attach_88161" title="glib: Add getter and setter for annotation's rectangle">attachment 88161</a> <a href="attachment.cgi?id=88161&action=edit" title="glib: Add getter and setter for annotation's rectangle">[details]</a> <a href='page.cgi?id=splinter.html&bug=70901&attachment=88161'>[review]</a>: ----------------------------------------------------------------- ::: glib/poppler-annot.cc @@ +682,5 @@ > + * > + * Retrieves the rectangle representing the page coordinates where the > + * annotation @poppler_annot is placed. > + * > + * Return value: %TRUE if #PopplerRectangle was correctly filled, %FALSE otherwise How can this return false? Rect is required field in annot dict, if it is not present the annot is not even created. @@ +684,5 @@ > + * annotation @poppler_annot is placed. > + * > + * Return value: %TRUE if #PopplerRectangle was correctly filled, %FALSE otherwise > + * > + * Since: 0.25 This should be 0.26, the next stable release. @@ +696,5 @@ > + > + g_return_val_if_fail (POPPLER_IS_ANNOT (poppler_annot), FALSE); > + g_return_val_if_fail (poppler_rect != NULL, FALSE); > + > + annot = static_cast<Annot *>(POPPLER_ANNOT (poppler_annot)->annot); You don't need to casts, poppler_annot is already a PopplerAnnot * and poppler_annot->annot is already Annot * @@ +704,5 @@ > + poppler_rect->x2 = annot_rect->x2; > + poppler_rect->y1 = annot_rect->y1; > + poppler_rect->y2 = annot_rect->y2; > + > + return TRUE; This should be void, we are unconditionally returning TRUE. @@ +715,5 @@ > + * > + * Move the annotation to the rectangle representing the page coordinates where the > + * annotation @poppler_annot should be placed. > + * > + * Since: 0.25 0.26 @@ +719,5 @@ > + * Since: 0.25 > + **/ > +void > +poppler_annot_set_rectangle (PopplerAnnot *poppler_annot, > + PopplerRectangle poppler_rect) Use a pointer here, to avoid copying the whole struct. The caller can use a stack allocated struct and pass the stack memory address. @@ +721,5 @@ > +void > +poppler_annot_set_rectangle (PopplerAnnot *poppler_annot, > + PopplerRectangle poppler_rect) > +{ > + Annot *annot; Extra spaces there. @@ +728,5 @@ > + > + annot = static_cast<Annot *>(POPPLER_ANNOT (poppler_annot)->annot); > + > + annot->setRect (poppler_rect.x1, poppler_rect.y1, > + poppler_rect.x2, poppler_rect.y2); You don't need the casts here either, this could be just: poppler_annot->annot->setRect(...);</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>