[cairo] add solid/gradient pattern info getters

Behdad Esfahbod behdad at behdad.org
Thu Sep 14 09:40:01 PDT 2006


Thanks Vlad,

+ * @r: a double to return the red color value in.
+ * @g: a double to return the green color value in.
+ * @b: a double to return the blue color value in.
+ * @a: a double to return the alpha value in.

Now that you added the if's, care to add "or %NULL"? :)  A few style
guidelines for docs:

  - Argument docs do not end in period.  They are not a sentence.

  - All enum and parameterless macros should be marked like %NULL and %
CAIRO_SOMETHING.  The ampersand will link them to their definition (and
use a slightly smaller font size or smallcaps.)

  - Try to match other usage.  For example, for colors, we have "xxx
component of color" in cairo_set_source_rgb, and have "return value for
X coordinate of the current point" in cairo_get_current_point().  This
logically leads to "return value for xxx component of color, or %NULL".
That's more concise and as informative, as the double type is already
there.


Also, I still think we want red, green, blue, alpha as argument names.
That makes the prototype fully documented (think of someone not knowing
what rgba is.)

+ *
+ * cairo_pattern_get_color_stops() is used to retrieve the color stops
+ * associated with a gradient pattern.  Passing NULL for @stop_data
will
+ * return the number of color stops in the pattern in @num_stops.
+ * The caller should allocate 5*@num_stops doubles to hold the data,
+ * and call the function again with @num_stops set to the returned
+ * value and @stop_data set to the newly allocated array.
+ *
+ * The array contents will be filled in with the stop offset, red,
+ * green, blue, alpha values in sequence, per stop.
+ *
+ * Return value: CAIRO_STATUS_SUCCESS, or CAIRO_STATUS_INVALID_COUNT
+ * if @stop_data is not NULL and @num_stops does not equal the actual
+ * number of color stops in the gradient. If the pattern is not a
+ * gradient pattern, CAIRO_STATUS_PATTERN_TYPE_MISMATCH is returned.
+ * CAIRO_NULL_POINTER is returned if @num_stops is NULL.
+ *

Four paragraphs to document a getter is enough evidence that it's not
good API :-).  First, the 5*@num thing is scary. If you are not going to
allocate the buffer (which is good), then I don't see the point of
returning @num and the data in the same call.  At the cost of one more
call, we can get back to your original proposal plus a getter for the
number of entries.

  cairo_pattern_get_color_stop_rgba (*pattern, *offset, *r, *g, *b, *a,
index);
  cairo_pattern_get_num_color_stops (*pattern, *num);

Not sure about get_dash().

Also:

 + * CAIRO_NULL_POINTER is returned if @num_stops is NULL.

You don't need to document how we handle user errors.  @num_stops is not
documented to be NULL, so it shouldn't be.  That we return this error
message is an implementation detail.


> > Also, the controversial part of this API (and all the others involving
> > color) is, make sure we don't return clamped values.  Just return
> > whatever user set.
> 
> I'm not sure what you mean here -- or rather, I am, but we only keep
> that data around for solid patterns.  For gradient endpoint patterns,
> the data structures are all pixman ones, which keep everything as
> 16-bit colors.

We should fix that then.  We should not return clamped values; that will
limit our colorspace from future expansion.  We can add tests for now
and make sure we fix that before release.


behdad

On Wed, 2006-09-13 at 21:56 -0400, Vladimir Vukicevic wrote:
> Ok, updated with comments.  This patch also implements
> cairo_get_dash().  Test cases for get_dash added to get-and-set, and a
> new pattern-getters testcase added for the other stuff.
> 
> The new functions now looks like:
> 
> cairo_public cairo_status_t
> cairo_get_dash (cairo_t *cr, int *num_dashes, double *dashes, double *offset);
> 
> 
> cairo_public cairo_status_t
> cairo_pattern_get_rgba (cairo_pattern_t *pattern,
> 			double *r, double *g, double *b, double *a);
> 
> cairo_public cairo_status_t
> cairo_pattern_get_surface (cairo_pattern_t *pattern,
> 			   cairo_surface_t **surface);
> 
> cairo_public cairo_status_t
> cairo_pattern_get_color_stops (cairo_pattern_t *pattern,
> 			       int *num_stops, double *stop_data);
> 
> cairo_public cairo_status_t
> cairo_pattern_get_linear_points (cairo_pattern_t *pattern,
> 				 double *x0, double *y0,
> 				 double *x1, double *y1);
> 
> cairo_public cairo_status_t
> cairo_pattern_get_radial_circles (cairo_pattern_t *pattern,
> 				  double *x0, double *y0, double *r0,
> 				  double *x1, double *y1, double *r1);
> 
>     - Vlad
-- 
behdad
http://behdad.org/

"Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill"
        -- Dan Bern, "New American Language"



More information about the cairo mailing list