<div dir="ltr">What about chaining effects, the API seems to only deal with one effect at time, is that correct?</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 18, 2016 at 6:27 PM, Bill Spitzak <span dir="ltr"><<a href="mailto:spitzak@gmail.com" target="_blank">spitzak@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Oct 17, 2016 at 11:15 AM, Bryce Harrington<br>
<<a href="mailto:bryce@osg.samsung.com">bryce@osg.samsung.com</a>> wrote:<br>
> In September 2013, Henry proposed[0] adding a feature for Gaussian blur<br>
> and shadow, based on work carried in his cairogles branch[1]. The<br>
> proposed API[2] was felt to have too many entry points to the context<br>
> for setting all the various parameters (shadow color and type, blur<br>
> direction, etc.), and Chris suggested[3] coming up with a more generic<br>
> API that would handle path effects in a way more analogous to patterns.<br>
><br>
> In 2014 I posted[4] a rough cut at such an API, following<br>
> cairo_pattern's API design, and a v2 shortly thereafter[5]. Following<br>
> is a v3 of that proposal, incorporating some of Chris Wilson's suggested<br>
> changes.<br>
><br>
> In particular, this divides the API into a base class (Effects), and<br>
> sub-class (Path Effects), allowing for future introduction of pen<br>
> effects, material effects, etc. This common API is intended to<br>
> provide a consistent way of handling all effects in the rendering<br>
> pipeline.<br>
><br>
> v2:<br>
> + Add get_data, set_data, reference_count for the cairo_object_t api<br>
> + Rename enums<br>
> v3:<br>
> + Path Effects are handled as a subclass of Effects<br>
><br>
> ---<br>
><br>
> /*<br>
> * Effect lifecycle management<br>
> */<br>
> cairo_public cairo_status_t<br>
> cairo_effect_status (cairo_effect_t *effect);<br>
><br>
> cairo_public cairo_effect_t *<br>
> cairo_effect_reference (cairo_effect_t *effect);<br>
><br>
> cairo_public void<br>
> cairo_effect_destroy (cairo_effect_t *effect);<br>
><br>
> cairo_public unsigned int<br>
> cairo_effect_get_reference_<wbr>count (cairo_effect_t *effect);<br>
><br>
> cairo_public void *<br>
> cairo_effect_get_user_data (cairo_effect_t *effect,<br>
> const cairo_user_data_key_t *key);<br>
> cairo_public cairo_status_t<br>
> cairo_effect_set_user_data (cairo_effect_t *effect,<br>
> const cairo_user_data_key_t *key,<br>
> void *user_data,<br>
> cairo_destroy_funct_t destroy);<br>
><br>
> /*<br>
> * Effect properties<br>
> */<br>
> typedef enum _cairo_effect_type {<br>
> CAIRO_EFFECT_TYPE_NONE = 0,<br>
> CAIRO_EFFECT_TYPE_PATH<br>
> } cairo_effect_type_t;<br>
><br>
> /* Future types might include:<br>
> * CAIRO_EFFECT_TYPE_PEN<br>
> * CAIRO_EFFECT_TYPE_MATERIAL<br>
> * CAIRO_EFFECT_TYPE_OPERATOR<br>
> * CAIRO_EFFECT_TYPE_CLIP<br>
> * ...<br>
> */<br>
><br>
> cairo_public cairo_effect_type_t<br>
> cairo_effect_get_type (cairo_effect_t *effect);<br>
><br>
><br>
><br>
> /* ------------------------------<wbr>------------------------------<wbr>---------- */<br>
><br>
> /*<br>
> * Path effect creation<br>
> *<br>
> * These create effects of type CAIRO_EFFECT_TYPE_PATH.<br>
> */<br>
> cairo_public cairo_effect_t *<br>
> cairo_effect_create_drop_<wbr>shadow_path (double x_offset, double y_offset,<br>
> double x_blur, double y_blur);<br>
><br>
> cairo_public cairo_effect_t *<br>
> cairo_effect_create_inset_<wbr>shadow_path (double x_inset, double y_inset,<br>
> double x_blur, double y_blur);<br>
><br>
> /*<br>
> * Applying path effects to the context<br>
> */<br>
> cairo_public void<br>
> cairo_set_path_effect (cairo_t *cr, cairo_effect_t *source);<br>
><br>
> cairo_public cairo_effect_t *<br>
> cairo_get_path_effect (cairo_t *cr);<br>
><br>
> /*<br>
> * Path effect properties<br>
> */<br>
> typedef enum _cairo_path_effect_type {<br>
> CAIRO_PATH_EFFECT_NONE = 0,<br>
> CAIRO_PATH_EFFECT_DROP_SHADOW,<br>
> CAIRO_PATH_EFFECT_INSET_SHADOW<br>
> } cairo_path_effect_type_t;<br>
><br>
> /* Future path effect types could include:<br>
> * CAIRO_PATH_EFFECT_PATTERN<br>
> * CAIRO_PATH_EFFECT_UNION<br>
> * CAIRO_PATH_EFFECT_DIFFERENCE<br>
> * CAIRO_PATH_EFFECT_INTERSECTION<br>
> * ...<br>
> */<br>
><br>
> cairo_public cairo_path_effect_type_t<br>
> cairo_path_effect_get_subtype (cairo_effect_t *effect);<br>
><br>
> cairo_public void<br>
> cairo_path_effect_set_rgba (cairo_path_effect_t *path_effect,<br>
> double red, double green, double blue,<br>
> double alpha);<br>
><br>
> Internally, both the effect type and subtype can be tracked in a single<br>
> property, e.g.:<br>
><br>
> type = CAIRO_EFFECT_TYPE_PATH << 24 | CAIRO_PATH_EFFECT_DROP_SHADOW;<br>
><br>
><br>
> Henry's API proposal included provision for shadow caching by the<br>
> application. That certainly made a huge difference in the demos, so<br>
> maybe it's still going to be needed. An idea is to provide a<br>
> backend/user preference as to whether effects should be cached or<br>
> applied on the fly, e.g. cairo_effect_set_cached.<br>
><br>
> Bryce<br>
><br>
> 0: <a href="https://lists.cairographics.org/archives/cairo/2013-September/024596.html" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/archives/cairo/2013-<wbr>September/024596.html</a><br>
> 1: The cairogles branch is no longer available publically. The private<br>
> branch is at <a href="https://github.com/SRA-SiliconValley/cairogles" rel="noreferrer" target="_blank">https://github.com/SRA-<wbr>SiliconValley/cairogles</a><br>
> 2: <a href="https://lists.cairographics.org/archives/cairo/2013-September/024598.html" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/archives/cairo/2013-<wbr>September/024598.html</a><br>
> 3: <a href="https://lists.cairographics.org/archives/cairo/2013-September/024597.html" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/archives/cairo/2013-<wbr>September/024597.html</a><br>
> 4: <a href="https://lists.cairographics.org/archives/cairo/2014-October/025748.html" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/archives/cairo/2014-<wbr>October/025748.html</a><br>
> 5: <a href="https://lists.cairographics.org/archives/cairo/2014-October/025766.html" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/archives/cairo/2014-<wbr>October/025766.html</a><br>
> --<br>
> cairo mailing list<br>
> <a href="mailto:cairo@cairographics.org">cairo@cairographics.org</a><br>
> <a href="https://lists.cairographics.org/mailman/listinfo/cairo" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/mailman/listinfo/cairo</a><br>
<br>
</div></div>Seems to me there is excessive namespacing here. There is no need for<br>
an "effect type", it should just be clear from the effect description<br>
whether it has an effect on stroke, fill, or text, or which parts of<br>
the inputs it changes.<br>
<br>
Possibly you are thinking that the effects would be mutually exclusive<br>
and that the type is to identify the mutually exclusive set, but that<br>
seems really limiting and I think you will end up with one "type" for<br>
each effect. Instead just allow arbitrary sets of effects to be<br>
active. If two effects can't be done at the same time, it is either an<br>
error, or one of them takes precedence.<br>
<br>
I would also remove "PATH" from all your enumerations and function<br>
names. Some of them are really confusing now, for instance<br>
"cairo_effect_create_inset_<wbr>shadow_path" does not create a path, it<br>
creates an effect.<br>
<div class="HOEnZb"><div class="h5">--<br>
cairo mailing list<br>
<a href="mailto:cairo@cairographics.org">cairo@cairographics.org</a><br>
<a href="https://lists.cairographics.org/mailman/listinfo/cairo" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/mailman/listinfo/cairo</a></div></div></blockquote></div><br></div>