[cairo] API Shakeup: cairo_current_path -> cairo_copy_path_data

[Carl Worth]

On Sat, 12 Mar 2005 09:53:16 +0100, Hans Breuer wrote:
> Not looking at the code but isn't it allocating path data and not
> just copying? Anyway to me it looks like a bad API which opens the
> door for mixing allocation and deallocation from different domains

I agree. The current proposal, (and as committed to CVS), that tells
the user to free() the data is a mistake.

What I'm now leaning toward is adding a new cairo_path_t structure to
wrap the cairo_path_data_t array, provide a length for it, and allow
us to have a cairo_path_destroy function with obvious usage. In
concrete terms, the new stuff would be:

	typedef struct cairo_path {
	    cairo_path_data_t *data;
	    int num_data;
	} cairo_path_t;

	void
	cairo_path_destroy (cairo_path_t *path);

along with the other existing stuff that's mostly unchanged, (but for
using the new cairo_path_t return type):

	typedef union {
	    struct {
	        enum {
	            CAIRO_PATH_MOVE_TO,
	            CAIRO_PATH_LINE_TO,
	            CAIRO_PATH_CURVE_TO,
	            CAIRO_PATH_CLOSE_PATH
	        } type;
	        int length;
	    } header;
	    struct {
	        double x, y;
	    } point;
	} cairo_path_data_t;
	
	cairo_path_t *
	cairo_copy_path (cairo_t *cr);

	cairo_path_t *
	cairo_copy_path_flat (cairo_t *cr);

	void
	cairo_append_path (cairo_t      *cr,
	                   cairo_path_t *path);

The other change in the above is that I've eliminated the
CAIRO_PATH_END_PATH type formerly used to terminate the path. It's not
needed now that we have path.num_data, and it was also a bit annoying
in that the recommended iteration loop had a switch statement that
didn't handle this enum value, (it didn't need to since the loop
termination condition prevented it, but the compiler still warned
about it).

So, the recommended usage could be changed as follows:

-	cairo_path_data_t *path, *p;
+	cairo_path_t *path;
+	cairo_path_data_t *data;
+	int i;

-	path = cairo_copy_path_data (cr);
+	path = cairo_copy_path (cr);

-	for (p = path; p->header.type != CAIRO_PATH_END; p += p->header.length) {
+	for (i=0; i < path->num_data; i += path->data[i].header.length) {
+	    data = &path->data[i];
-	    switch (p->header.type) {
+	    switch (data->header.type) {
	    ...
	    }
	}
-	free (path);
+	cairo_path_destroy (path);

If that's not controversial, I've got a patch ready to commit, (though
it's pretty boring to be posted here).

There is one issue I'm still a bit unsettled on. Introducing this new
public cairo_path_t data structure means that we'll need a different
name for the private data structure (which was formerly called
cairo_path_t).

Currently, my patch renames the internal structure to
cairo_path_real_t, but it leaves its functions with cairo_path names
in cairo_path.c. Meanwhile, the new structure still has internal
cairo_path_data function names defined in cairo_path_data.c. So that
will need some cleaning up.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050316/648a8a14/attachment.pgp