Re: GtkHSV



Hi,

So the first step in either deciding to make this thing public or fix
the API is to get the use cases where people want the widget. 

 - writing a similar-but-slightly-different color selector
 - <more stuff here, I can't think of too much>

We generally try to avoid widgets only a very few apps will use, 
though I understand there's a bit of a "it's there anyway" argument
for this particular widget.

Ross Burton <ross burtonini com> writes: 
> The current API is quick and simple:

I'd start with the name - why is it called GtkHSV? I have no idea. ;-)
Maybe it should be GtkColorWheel or something.

> GtkWidget* gtk_hsv_new          (void);
> void       gtk_hsv_set_color    (GtkHSV    *hsv,
> 				 double     h,
> 				 double     s,
> 				 double     v);
> void       gtk_hsv_get_color    (GtkHSV    *hsv,
> 				 gdouble   *h,
> 				 gdouble   *s,
> 				 gdouble   *v);

So I'd have these renamed to set_hsv() and have set_color() 
take a GdkColor in RGB format.

> void       gtk_hsv_set_metrics  (GtkHSV    *hsv,
> 				 gint       size,
> 				 gint       ring_width);
> void       gtk_hsv_get_metrics  (GtkHSV    *hsv,
> 				 gint      *size,
> 				 gint      *ring_width);

This is busted - should have separate setter/getter for each
parameter, rather than a single function that takes two args.

> void       gtk_hsv_to_rgb       (gdouble    h,
> 				 gdouble    s,
> 				 gdouble    v,
> 				 gdouble   *r,
> 				 gdouble   *g,
> 				 gdouble   *b);
> void       gtk_rgb_to_hsv       (gdouble    r,
> 				 gdouble    g,
> 				 gdouble    b,
> 				 gdouble   *h,
> 				 gdouble   *s,
> 				 gdouble   *v);
> 

The RGB bits should be GdkColor, HSV args can remain doubles I
suppose. In any case clearly we shouldn't have something like
"color->red = hue" that would be confusing.
 
> 1) no documentation :)

Indeed. ;-)

> 2) Surely GdkColor should be used?
> 3) gtk_hsv_to_rgb and gtk_rgb_to_hsv should really be in GdkColor.

Yep.


I'd also point out that we should probably underscore-prefix 
all these functions for 2.0.0. If you rewrite you might want to do
that while you're at it...

Havoc



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]