[cairo] more pattern rewrite stuff...
Kristian Høgsberg
krh at bitplanet.net
Mon Jan 31 20:00:57 PST 2005
David Reveman wrote:
> On Mon, 2005-01-31 at 15:14 -0500, Kristian Høgsberg wrote:
>
>>David Reveman wrote:
>>
>>>ok, I couldn't stop myself from trying to get this right. This patch
>>>includes new changes to the pattern system, this time I'm pretty happy
>>>with how it turned out.
>>>
>>>It removes backend functions set_filter, set_repeat and set_matrix.
>>>Makes the "pattern <-> gstate" and "pattern <-> backend" connections
>>>more appropriate and cleans up a lot of stuff. Looking at the diff is
>>>probably not good enough.
>>
>>We discussed this in IRC a couple of days ago, and general consensus was
>>that filter, repeat and transform should be moved out of the surface and
>>into the pattern. Owen summarized it here:
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=2397
>
>
> ok, I've done the first step of this then. The the repeat, filter and
> matrix attributes are still left in the surface only to be picked up by
> caior_show_surface. Moving filter attribute into the gstate and removing
> the surface matrix should now be simple, but as it's a quite large
> change to public API I think it can wait until we get my current patch
> merged with Owen's fall-back bits. We could also remove the width and
> height parameters from show_surface, at the same time.
>
>
>>>Futher on, we should make the mask surface passed to surface_composite a
>>>pattern as well,
>>
>>I think we should get this done sooner rather than later. I can take a
>>look at that once you get your patch landed.
>
> good, it shouldn't be to hard once we get the _cairo_pattern_get_surface
> stuff working well.
>>> static cairo_int_status_t
>>>+_cairo_image_surface_change_attributes (cairo_image_surface_t *surface,
>>>+ cairo_surface_attributes_t *attributes)
>>
>>So, instead of this, we would set the pixman surface attributes from the
>>pattern.
>
> _cairo_image_surface_change_attributes is actually doing this.
> cairo_surface_attributes_t is basically the same as cairo_pattern_info_t
> in Owen's patch. Hmm, _cairo_pattern_get_surface returns a surface
> representing the pattern so I think cairo_surface_attributes_t is more
> appropriate.
What I mean is, why not just pass a cairo_pattern_t instead of
cairo_surface_attributes_t, since the pattern will contain all the
information needed.
Another point is that these attributes aren't necessarily surface
attributes, that's just an artifact^Wimplementation detail of the
render/pixman based backends. For the PDF backend there are no surfaces
involved in solid color or gradient patterns.
Anyway, I guess this will look all different when you get it merged with
current CVS.
>>>@@ -380,33 +387,60 @@
>>> unsigned int width,
>>> unsigned int height)
>>> {
>>>- cairo_image_surface_t *dst = abstract_dst;
>>>- cairo_image_surface_t *src;
>>>- cairo_image_surface_t *mask = (cairo_image_surface_t *) generic_mask;
>>>- int x_offset, y_offset;
>>>-
>>>+ cairo_surface_attributes_t attributes;
>>>+ cairo_image_surface_t *dst = abstract_dst;
>>>+ cairo_image_surface_t *src;
>>>+ cairo_image_surface_t *mask = (cairo_image_surface_t *) generic_mask;
>>>+ cairo_image_surface_t *mask_alpha = NULL;
>>>+ cairo_int_status_t status;
>>>+ double alpha = pattern->color.alpha;
>>>+
>>>+ if (generic_mask)
>>>+ {
>>>+ if (generic_mask->backend != dst->base.backend)
>>>+ return CAIRO_INT_STATUS_UNSUPPORTED;
>>>+ }
>>>+ else if (pattern->type == CAIRO_PATTERN_SURFACE && alpha != 1.0)
>>>+ {
>>>+ mask = mask_alpha = (cairo_image_surface_t *)
>>>+ _cairo_surface_create_similar_solid (&dst->base, CAIRO_FORMAT_A8,
>>>+ 1, 1, &pattern->color);
>>>+ if (!mask)
>>>+ return CAIRO_STATUS_NO_MEMORY;
>>>+
>>>+ cairo_surface_set_repeat (&mask->base, 1);
>>>+ alpha = 1.0;
>>>+ }
>>>+
>>> src = (cairo_image_surface_t *)
>>>- _cairo_pattern_get_surface (pattern, &dst->base,
>>>+ _cairo_pattern_get_surface (pattern, alpha, &dst->base,
>>> src_x, src_y, width, height,
>>>- &x_offset, &y_offset);
>>
>>Why are you reading out the alpha value from the pattern color only to
>>pass it back to _cairo_pattern_get_surface, which use it as it used to
>>use pattern->color.alpha?
>
> If no mask surface or eventually mask pattern is passed to composite, we
> can use the mask to accelerate overall surface alpha. The alpha
> parameter tells _cairo_pattern_get_surface what alpha to use for surface
> patterns. Hence, passing alpha=1.0 and _cairo_pattern_get_surface will
> not do overall alpha for us.
Yeah, but why don't you just change the alpha value in the pattern
instead of overriding it by this new alpha argument?
> The gradient struct will need to have alpha as well. How about this?
>
> struct _cairo_pattern {
> unsigned int ref_count;
>
> cairo_extend_t extend;
> cairo_filter_t filter;
> cairo_matrix_t matrix;
>
> cairo_pattern_type_t type;
> union {
> double alpha;
> struct {
> double alpha;
> double red, green, blue;
> } color;
> struct {
> double alpha;
> cairo_surface_t *source;
> } surface;
> struct {
> double alpha;
> cairo_point_double_t point0;
> cairo_point_double_t point0;
> cairo_color_stop_t *stops;
> int num_stops;
> } gradient;
> } u;
> };
Yeah, that looks pretty good.
cheers,
Kristian
More information about the cairo
mailing list