[cairo] API Shakeup: Making set_source consistent, (renaming cairo_set_rgb_color and cairo_set_pattern, eliminating cairo_set_alpha)

[David Reveman]

On Wed, 2005-04-13 at 15:28 -0400, Carl Worth wrote:
> On Wed, 13 Apr 2005 13:28:18 -0400, Owen Taylor wrote:
> > > So, I think it's about time to start some real backwards-incompatible
> > > API changes, (which we've been mostly avoiding up until now in the API
> > > Shakeup).
> > 
> > Sounds good to me. 
> 
> OK. I'll let you land the rectangle patch, and then go forward with
> this. The conflicts are quite minor.
> 
> > Spent some time looking through the patch - I didn't see any problems
> > with the cairo-pattern.c changes (or even removed optimizations), other
> > than the premultiplication problems noted below.
> 
> Thanks for looking through this so carefully.
> 
> > 0xffff of course
> 
> Fixed.
> 
> > -       _cairo_pattern_init_solid (&pattern.solid, 1.0, 1.0, 1.0);
> > +       _cairo_color_init_rgba (&opaque_white, 1.0, 1.0, 1.0, 1.0);
> > +       _cairo_pattern_init_solid (&pattern.solid, &opaque_white);
> > 
> > You used init_rgb() in other similar cases elsewhere.
> 
> Fixed.
> 
> > Hmm, should this be non-premultiplied? But I guess this is used
> > only by cairo_get_rgb_color() which will be deprecated? (Doesn't
> > seem to be in your patch)
> 
> Fixed to use _cairo_color_get_rgb which handles
> un-premultiplication. But yes, this can go away with
> cairo_get_rgb_color, (which was marked deprecated even before the
> patch).
> 
> > src->stops->color is premultiplied, right?
> 
> Yes. Good catch. Fixed.
> 
> > +       _cairo_color_get_rgba (&src_solid->color, &red, &green, &blue,
> > &alpha);
> > +       alpha *= mask_solid->color.alpha;
> > +
> > +       _cairo_color_init_rgba (&combined, red, green, blue, alpha);
> > 
> > The unpremultipication followed by premultiplication is going to give
> > problems for super-luminescent colors, I think? (if alpha is 0, say)
> 
> I think you're right.
> 
> > Perhaps having a _cairo_color_multiply_alpha () would make sense
> > since this occurs in a couple of places.
> 
> Done.
> 
> > In general, I think the number of places where the code does, something
> > like:
> > 
> >  cairo_color_t opaque_white; 
> >  _cairo_color_init_rgb (&opaque, 1.0, 1.0, 1.0);
> >  /* use &opaque_white once */
> > 
> > is ugly.
> 
> Good point.
> 
> >  _cairo_pattern_init_solid (_cairo_stock_color (CAIRO_WHITE));
> 
> I've added this now, and also macros to avoid nasty wrapping in some
> cases. So it now looks like:
> 
>    _cairo_pattern_init_solid (CAIRO_COLOR_WHITE);
> 
> > The private API could be added without the public API, of course.
> 
> Thats what I've got now, (without comment on the merits of the public
> API proposal).
> 
> The patch below has the changes mentioned above, plus it has been
> merged with Owen's recent rectangle optimization work. I'm still
> getting one failure in an SVG file with a radial gradient with
> translucent color stops. The colors are coming out dark, so it looks
> to me like a pre-multiplication confusion (such that the colors are
> being multiplied by alpha twice).

I haven't looked at your patch yet but color stop interpolation have to
be done to unpre-multiplied colors. At least if we want to match other
SVG renderers (I don't know what the SVG spec says about this). So for
this to work my gradient code in cairo_pattern.c is pre-multiplying
colors after interpolation so you'll need to give it unpre-multiplied
color stops.

> 
> If anyone wants to look, the SVG file is at:
> 
> http://cairographics.org/~cworth/images/gradPatt-stop-BE-10.svg
> 
> And the before and after results are at:
> 
> http://cairographics.org/~cworth/images/gradPatt-stop-BE-10-correct.png
> http://cairographics.org/~cworth/images/gradPatt-stop-BE-10-broken.png
> 
> I'll add a test case to cairo/test to capture this and I'll make sure
> it's fixed before committing.
> 
> -Carl
> 
> _______________________________________________
> cairo mailing list
> cairo at cairographics.org
> http://lists.freedesktop.org/mailman/listinfo/cairo
-David