Bug 71727 - glib-demo: Add color selection for new annotations
Summary: glib-demo: Add color selection for new annotations
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: glib frontend (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 70981 70982
  Show dependency treegraph
 
Reported: 2013-11-18 08:59 UTC by Germán Poo-Caamaño
Modified: 2013-11-19 17:08 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
glib-demo: add color selection for new annotations (4.09 KB, patch)
2013-11-18 09:08 UTC, Germán Poo-Caamaño
Details | Splinter Review

Description Germán Poo-Caamaño 2013-11-18 08:59:09 UTC
As the summary says.  Add in the demo a way to add color to new annotations.
Comment 1 Germán Poo-Caamaño 2013-11-18 09:08:13 UTC
Created attachment 89390 [details] [review]
glib-demo: add color selection for new annotations
Comment 2 Carlos Garcia Campos 2013-11-19 17:08:29 UTC
Comment on attachment 89390 [details] [review]
glib-demo: add color selection for new annotations

Review of attachment 89390 [details] [review]:
-----------------------------------------------------------------

Pushed to git master with the modifications I suggest below. Thanks!

::: glib/demo/annots.c
@@ +67,4 @@
>      ModeType         mode;
>  
>      cairo_surface_t *surface;
> +    PopplerColor    *annot_color;

This is allocated when the widget is created and deallocated when it's destroyed, so I think it could be stack allocated instead.

@@ +451,5 @@
> +    gtk_color_button_get_rgba (GTK_COLOR_BUTTON (button), &color);
> +#endif
> +    demo->annot_color->red = CLAMP ((guint) (color.red * 65535), 0, 65535);
> +    demo->annot_color->green = CLAMP ((guint) (color.green * 65535), 0, 65535);
> +    demo->annot_color->blue = CLAMP ((guint) (color.blue * 65535), 0, 65535);

Instead of doing this conversion every time the color changes in the selector, we could keep a GdkRGBA in the demo struct and convert it to a PopplerColor right before calling poppler_annot_set_color. I think it will simplify the code in general.

@@ +981,4 @@
>  
>      pgd_annots_finish_add_annot (demo);
>  
> +    demo->start.x = -1;

This is unrelated to this patch.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.