[cairo] new pattern interface and OpenGL patch
Carl Worth
cworth at east.isi.edu
Mon Mar 22 18:43:53 PST 2004
On Mar 22, David Reveman wrote:
> I've put together a pattern/OpenGL patch which I'd appreciate if you,
> who are interested, looked over.
David,
This is looking really great. I've really been wanting to have a good
implementation of linear/radial gradients and it looks like you've
done all the hard work. Well done.
> This pattern interface allow all combinations of patterns and
> shader functions to be accelerated by the OpenGL backend using libglc's
> new color range interface. Even pattern extend types repeat and reflect
> are supported by the OpenGL backend.
Fantastic. As I thought about the pattern interface I was worried
about inventing something that would be hard to accelerate. Now that
you've already got something accelerated, I'm much less concerned
about that. ;-)
> I've left the old interface with surface_set_repeat, surface_set_matrix,
> surface_set_filter... in there, so that old applications still work. The
> old and new interface work fine together but we should probably remove
> the old one when the new interface has been proven to work
> correctly.
Yes. More comments on the top-level interface below.
> Please look over the patch. I'd like to get the OpenGL backend committed
> to cairo CVS and the best would be if we could get the new pattern
> interface committed at the same time, as it's very important for the
> OpenGL backend to work efficiently. I can off course put together a
> OpenGL backend patch without the pattern interface if it becomes
> necessary.
I don't think we'll have much problem getting both pieces in soon. I
had mentioned separating your patch earlier, but I think I wasn't
clear. I would prefer to see the new pattern work committed separately
from the new GL backend. I don't care which goes in first.
I think it's important to discuss changes to the public API here on
the list, so I'll make some comments on those portions of the patch
here. In each case, my comments follow the portion of the patch which
I quote.
+void
+cairo_set_target_glx (cairo_t *cr,
+ Display *dpy,
+ int screen,
+ int options,
+ Window window);
What's the options parameter here? Will the correct usage of this
parameter be obvious to an experience GLX user?
+void
+cairo_set_pattern_linear (cairo_t *cr,
+ double from_x, double from_y,
+ double to_x, double to_y);
+
+void
+cairo_set_pattern_radial (cairo_t *cr,
+ double center_x, double center_y,
+ double radius_dx, double radius_dy,
+ double focal_x, double focal_y);
I think I know now how I want the pattern API to work. I knew that
things like cairo_surface_set_repeat where wrong, and I had
recommended reworking them as cairo_set_pattern_repeat, but now I
think I would instead like:
cairo_pattern_t *
cairo_pattern_create_for_surface (cairo_surface_t *surface);
cairo_status_t
cairo_pattern_set_extend (cairo_pattern_t *pattern, cairo_extend_t extend);
cairo_status_t
cairo_pattern_set_matrix (cairo_pattern_t *pattern, cairo_matrix_t *matrix);
cairo_status_t
cairo_pattern_get_matrix (cairo_pattern_t *pattern, cairo_matrix_t *matrix);
void
cairo_set_pattern (cairo_t *cr, cairo_pattern_t *pattern);
Then, in this style, the two functions above might be:
cairo_pattern_t *
cairo_pattern_create_linear (cairo_t *cr,
double x0, double y0,
double x1, double y1);
cairo_pattern_t *
cairo_pattern_create_radial (cairo_t *cr,
double center_x, double center_y,
double radius_dx, double radius_dy,
double focal_x, double focal_y);
What do you think of that?
Oh, and PostScript provides a (very) slightly different model for the
radial pattern parameters. Following that model would give:
cairo_pattern_t *
cairo_pattern_create_radial (cairo_t *cr,
double cx0, double cy0, double radius0,
double cx1, double cy1, double radius1);
Having a single radius rather than separate radius_dx/radius_dy values
matches the style used by cairo_arc. Also, we need a radius for the
final point as well, right?
+void
+cairo_add_pattern_color_stop (cairo_t *cr, double offset,
+ double red, double green, double blue,
+ double alpha);
Continuing the trend:
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.
+
+typedef enum {
+ CAIRO_SHADER_LINEAR,
+ CAIRO_SHADER_GAUSSIAN,
+ CAIRO_SHADER_FUNCTION
+} cairo_shader_t;
Could we get away with just using the same cairo_filter_t type for
this purpose instead?
+/* component order of colors are RGBA and factor is the distance
+ from color stop 1 (0.0 < factor < 1.0). */
+typedef void (*cairo_shader_function_t) (unsigned char *color1,
+ unsigned char *color2,
+ double factor,
+ unsigned char *result_color);
+
+/* shoud we introduce an additional function for setting the
+ shader function? */
+void
+cairo_set_pattern_shader (cairo_t *cr,
+ cairo_shader_t shader,
+ cairo_shader_function_t function);
+
+cairo_shader_t
+cairo_current_pattern_shader (cairo_t *cr,
+ cairo_shader_function_t *function);
Is it important to have function-based filtering here? I'd fell more
comfortable not adding this unless there is a compelling reason.
+void
+cairo_set_pattern_transform (cairo_t *cr, cairo_matrix_t *matrix);
+
+void
+cairo_current_pattern_transform (cairo_t *cr, cairo_matrix_t
*matrix);
These were covered above.
+typedef enum {
+ CAIRO_EXTEND_NONE,
+ CAIRO_EXTEND_REPEAT,
+ CAIRO_EXTEND_REFLECT
+} cairo_extend_t;
+
+void
+cairo_set_pattern_extend (cairo_t *cr, cairo_extend_t extend);
This is a good improvement over set_repeat. Thanks. Of course, in my
proposal the interface is cairo_pattern_set_extend as above.
+cairo_extend_t
+cairo_current_pattern_extend (cairo_t *cr);
I'd prefer:
cairo_extend_t
cairo_pattern_get_extend (cairo_pattern_t *pattern);
+/* Rectangular extents */
+void
+cairo_stroke_extents (cairo_t *cr, double *x1, double *y1, double
*x2, double *y2);
+
+void
+cairo_fill_extents (cairo_t *cr, double *x1, double *y1, double *x2,
double *y2);
These should be quite handy. Thanks.
+#define CAIRO_GL_OPTION_DOUBLEBUFFER_MASK \
+ GLC_FORMAT_OPTION_DOUBLEBUFFER_MASK
+#define CAIRO_GL_OPTION_SINGLEBUFFER_MASK \
+ GLC_FORMAT_OPTION_SINGLEBUFFER_MASK
+#define CAIRO_GL_OPTION_MULTISAMPLE_MASK \
+ GLC_FORMAT_OPTION_MULTISAMPLE_MASK
+#define CAIRO_GL_OPTION_NO_MULTISAMPLE_MASK \
+ GLC_FORMAT_OPTION_NO_MULTISAMPLE_MASK
Ah, these are the options for the option parameter above? I might
suggest typedef'ing that int to cairo_gl_option_t so that it would be
easier to search for these symbolic values within cairo.h.
+cairo_status_t
+cairo_gl_surface_realize (cairo_surface_t *surface);
Hmmm... I was a little confused at first when I saw "cairo_gl_surface"
as opposed to cairo_glx_surface. But I guess what you're doing here is
providing a function that works for both the GLX and AGL backends.
Unfortunately, the cairo_surface_t type doesn't provide enough
information to enforce the association between custom surface
functions and surface types.
So, in the absence of good type-checking, I've been trying to at least
match the function names, eg.:
cairo_surface_t *
cairo_foo_surface_create (...);
cairo_status_t
cairo_foo_surface_bar (cairo_surface_t *surface, ...);
Where foo is also the name of a backend that can be configured with
--enable-foo, tested with CAIRO_HAS_FOO_SURFACE, etc.
So, I don't know what the best thing to do here would be.
+XVisualInfo *
+cairo_glx_find_best_visual_info (Display *dpy,
+ int screen,
+ int options);
What's this function? It seems a bit out of place.
+cairo_surface_t *
+cairo_glx_surface_create_for_window (Display *dpy, /* ... */
Each of the new surface_create functions look just fine.
+typedef struct _cairo_gl_convolution_t {
+ double matrix[3][3];
+} cairo_gl_convolution_t;
+
+cairo_status_t
+cairo_gl_surface_set_convolution (cairo_surface_t *surface,
+ cairo_gl_convolution_t *convolution);
Hmmm.... would we want a general function to be able to apply
convolutions to all types of surfaces?
+cairo_status_t
+cairo_gl_surface_show (cairo_surface_t *surface,
+ int x,
+ int y,
+ unsigned int width,
+ unsigned int height);
+
+cairo_status_t
+cairo_gl_surface_gl_begin (cairo_surface_t *surface);
+
+cairo_status_t
+cairo_gl_surface_gl_end (cairo_surface_t *surface);
What are the semantics of these functions? I just want to make sure
that whatever general functionality we need lives in the general
interface.
Phew. Thanks for sticking through all that with me.
-Carl
More information about the cairo
mailing list