[cairo] new pixman questions

[Soeren Sandmann]

David Hill <David.Hill at Sun.COM> writes:

> hey - a couple of questions related to the integration of the new pixman 
> code. These came from my first, very hack and slash to get it to compile 
> pass. I am sure I will have more questions as I get back to all the code 
> I commented out.

Did you see the mail I sent earlier today? I already have a version of
cairo that links to the new pixman shared library (which is also used
by the X server). It's available in

        /home/sandmann/cairo-pixman/

on people.freedesktop.org. 

> 1) in the new pixman repo, the source is in pixman/pixman, and in the 
> cairo it is pixman/src. Anyone mind if I leave it pixman/src ?

I'm not sure what you mean here. Do you want to change name of the
directory in the library? The directory in the cairo source tree would
of course go away once cairo links to the pixman library.
 
> 2) why pixman_region_num_rects() -> pixman_region_n_rects() ? Is there a 
> semantic difference I need to be worried about ?

No, they are the same. 

> 3) in the new code, pixman_region_rects() exists but is not in pixman.h. 
> Is this an oversight or on purpose ?

I don't think that function is needed anymore, so it should probably
just be deleted. There is a different function now called
pixman_region_rectangles() which also returns the number of rectangles
in an out-argument. You can pass NULL if for some reason you don't
want the number of rectangles.

> 4) there does not seem to be a pixman_image_destroy() and I would think 
> there needs to be one to free image internal data. Oversight ?

The new pixman_image data structure is ref counted, so it's called
pixman_image_unref() now.

> 5) pixman_image_set_transform() does not return an error condition when 
> it detects out of memory. I am guessing we should return a bool_t at a 
> minimum, and possibly an error code ? After all the recent error 
> handling additions, perhaps a few error codes and an int return make
> sense.

In the latest version, pixman_image_set_transform() does return a
boolean indicating whether the operation succeeded.

> 6) pixman_bits_t is gone. It was a typedef for uint32_t, which is used 
> in a few places in the code. This is going to force a cast when dealing 
> with RGB565 and other short data. Do we want to leave it as uint32_t or 
> perhaps use a typedef to hide things, or perhaps even make it a void* so 
> that no cast is needed on the public interface ?

Reintroducing the pixman_bits_t typedef is fine with me. Doing so
would require going through the code and undoing the
s/FbBits/uint32_t/ that I did. 

I don't think making it a void* is a good idea because we rely on
pointer arithmetic in many places, so you'd have to cast internally
anyway.



Thanks,
Soren