<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><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#c21">Comment # 21</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 <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=89391" name="attach_89391" title="glib-demo: add color selection for new annotations">attachment 89391</a> <a href="attachment.cgi?id=89391&action=edit" title="glib-demo: add color selection for new annotations">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=70981&attachment=89391'>[review]</a>
glib-demo: add color selection for new annotations
Review of <span class=""><a href="attachment.cgi?id=89391" name="attach_89391" title="glib-demo: add color selection for new annotations">attachment 89391</a> <a href="attachment.cgi?id=89391&action=edit" title="glib-demo: add color selection for new annotations">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=70981&attachment=89391'>[review]</a>:
-----------------------------------------------------------------
Looks great, thanks! I've pushed it to git master with some minor
modifications.
::: glib/demo/annots.c
@@ +663,3 @@
<span class="quote">> {
> + GtkTreeIter iter;
> + GtkTreePath *path;</span >
This is only used inside the if (selected) block, so it could be moved there.
@@ +846,4 @@
<span class="quote">> pgd_annots_add_annot (PgdAnnotsDemo *demo)
> {
> PopplerRectangle rect;
> + PopplerPoint start, end;</span >
This is only used for line annots, so it can be moved to the case
POPPLER_ANNOT_LINE: block.
@@ +880,3 @@
<span class="quote">> g_object_unref (annot);
> +
> + pgd_annots_viewer_queue_redraw (demo);</span >
This is already called right after pgd_annots_add_annot
@@ +1013,5 @@
<span class="quote">> + demo->stop.y = event->y;
> +
> + PopplerRectangle rect;
> + PopplerPoint start, end;
> + gdouble width, height;</span >
Declarations block should be at the beginning of the function body.
@@ +1052,2 @@
<span class="quote">>
> demo->start.x = -1;</span >
This could be moved to the finish_add_annot method.
@@ +1054,5 @@
<span class="quote">>
> + if (demo->annotations_idle > 0) {
> + g_source_remove (demo->annotations_idle);
> + demo->annotations_idle = 0;
> + }</span >
finish_add_annot schedules a new redraw, so if there's a pending redraw, it's
better to not cancel it here.</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>