[cairo] add solid/gradient pattern info getters

[Vladimir Vukicevic]

On 9/14/06, Behdad Esfahbod <behdad at behdad.org> wrote:
> 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.

Will do.

> 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.)

Sure, though if they don't know what rgba is, they're very much in the
wrong place :)

> + *
> + * 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);

Eh.. I'd still prefer the API I suggested, but I don't care enough
about it to argue for it :)  So, sure, though I'll probably do
get_color_stop_count and get_color_stop_rgba.

> Not sure about get_dash().

I'd have get_dash follow the same -- get_dash_count() and 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.

I'm don't really agree with this -- the error message isn't an
implementation detail, it's something that's visible to the user.  If
the user sees CAIRO_NULL_POINTER as a return value from a public API
call, it would be very helpful for the documentation to list the cases
where CAIRO_NULL_POINTER might be returned.

> > > 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.

Sure, I can do that.

    - Vlad