[cairo] new pattern interface and OpenGL patch

[Carl Worth]

On Mar 24, David Reveman wrote:
 > I like it, but shouldn't the
 > cairo_pattern_create_for_surface (cairo_surface_t *surface);
 > be
 > cairo_pattern_create_for_surface (cairo_t *cr, 
 >                                   cairo_surface_t *surface);
 > just like the linear and radial ones.

Yes. Good catch.

 > > 	cairo_status_t
 > > 	cairo_pattern_add_color_stop (cairo_pattern_t *pattern, double offset,
 > > 				      double red, double green, double blue,
 > > 				      double alpha);
 > > 
 > > Oh, and up until now, I had been avoiding putting red, green, blue,
 > > and alpha into a single function call, since it raises the question of
 > > whether the color is represented with premultiplied alpha or not. So,
 > > we'll have to answer that question here.
 > > 
 > > One option would be to finally cave in a declare a cairo_color_t
 > > structure. Owen argued in the past that applications are going to want
 > > that anyway, so we should provide it as a convenience.
 > 
 > ok.

Even though I mentioned it, after some thought, I don't think I like
this idea. For Owen's wants, only an exposed structure would help. But
if we expose the structure, then we cannot hide the question of
premultiplied alpha. So that wouldn't solve the problem, it would only
move it.

Plus, we can't hide the fact that the image backend wants data with
premultiplied alpha. So, maybe the right thing to do is to be
consistent and just specify that this function accepts premultiplied
alpha.

Another issue that needs to be specified is the color space in which
the gradient is computed. Keith makes a compelling argument to provide
an interface of just linear sRGB interpolation, letting users that
want something else approximate it as close as they wish.

With those two points in mind, maybe we should also change
cairo_set_rgb_color to also use pre-multiplied and to not require a
superfluous "rgb":

	void
	cairo_set_color (cairo_t *cr, double red, double green,
			 double blue, double alpha);

Although this would impact the current behavior of cairo_show_surface
which does use the current alpha value when compositing the
surface. We really want a more general way to mask an image anyway, so
this may not be a big deal.

 > the 'options' parameter is a bit mask of the above options, so if we should
 > use a cairo_gl_option_t type for them, we need to replace the 'options' 
 > parameter with a list of cairo_gl_option_t's or something.  

You can still use a bit mask. I was just suggesting a typedef to help
guide the user to the symbolic mask values.

 > We don't actually need to have any cairo_gl_surface* functions in the 
 > public API. We could remove the realize function or change it into
 > two functions, a glx one and one agl one.

Good. I like that idea much better.

 > >     +XVisualInfo *
 > >     +cairo_glx_find_best_visual_info (Display *dpy,
 > >     +                                 int screen,
 > >     +                                 int options);
 > > 
 > > What's this function? It seems a bit out of place.
 >
 > You need the XVisualInfo to create a Window which can be used
 > with cairo_glx_surface_create_for_window. The 'options' parameter
 > must be the same for both functions.
 > 
 > I'm sure this can be a bit confusing but it shouldn't be to weird
 > for you who are familiar with GLX. It's similar to:
 > glXFindVisualInfo and glXCreateWindow.

I see. The problem is that this function doesn't match cairo's
argument passing convention, (ie. it looks like it should perhaps
accept a cairo_glx_t * as a first parameter).

Well, if the user is going to have to call glXCreateWindow then why
doesn't the user also call glXFindVisualInfo? Then this cairo_glx
function could be dropped?

If so, I suppose that the options parameter cairo_set_target_glx might
have to change to be compatible with GLX, but that seems like a good
thing anyway. (Currently, the API exposes the GLC header file which
would not be necessary if it did not also expose GLC options).

 > >     +cairo_status_t
 > >     +cairo_gl_surface_show (cairo_surface_t *surface,
 > >     +                       int x,
 > >     +                       int y,
 > >     +                       unsigned int width,
 > >     +                       unsigned int height);
 > >
 > > What are the semantics of these functions? I just want to make sure
 > > that whatever general functionality we need lives in the general
 > > interface.
 > 
 > surface_show is used for swapping buffers of double buffered surfaces or
 > flushing drawing operations of single buffered ones. This isn't needed as
 > we can use copy_page and show_page for this.

I had previously suggested overloading show_page to do buffer
swapping, and Keith argued that there are enough semantic differences
that the functions should remain separate. I think he's probably right.

Also, "surface_show" strikes me as much too similar to "show_surface"
though the semantics are wildly different. So, I'd prefer a more
descriptive name. And this is not the only backend that may need some
sort of flush operation, so I'd prefer to have a top-level
cairo_flush, (or whatever we decide to call it).

That may require you to add a cairo_glx_surface_update_size, but that's
similar to the cairo_xlib_surface_update_size that I was also about to
add. (Hmmm, maybe we'll want a general cairo_surface_update_size).

Keep up the good work.

-Carl