[cairo] xlib re-re-re implementing the API

[Keith Packard]

On Tue, 2005-05-17 at 02:12 -0700, Carl Worth wrote:

> The patch didn't include the new cairo-xlib-xrender.h, but you had
> quoted the single prototype in the file, so there's not much mystery
> there.

diff doesn't include new files...

> 
> The complete elimination of code paths involving round trips is quite
> compelling, especially in light of the fact that the interface isn't
> significantly harder to use. (The constructors do require width,height
> now, but at least cairo_xlib_surface_set_size can now be ignored in
> the Pixmap case).

And, in fact, I didn't have any trouble providing dimensions in any of
the test code, so the overall effect was to reduce the amount of code
needed by eliminating the everpresent set_size for pixmaps.

> 
> > +	/* XXX This can't work.  We must convert the data to one of the 
> > +	 * supported pixman formats
> > +	 */
> 
> I assume these failure modes existed prior to the patch as well? (And
> now, we just have a nice, commented block of code to catch the case.)
> This is for when the user provides a visual that doesn't match any
> standard Render/cairo format, right?

No, we have to have code that translates between arbitrary X formats and
those supported by libpixman.   I suggest we take advantage of the
formats that are supported by libpixman as those will cover most
Xorg-based X servers nicely.

For the unusual cases, we will end up with some ugly-and-slow code that
extracts RGBA from an arbitrary format and packs them into suitable
client-side formats.  I don't see any way around this.  And, of course,
we'll have code that goes the other direction.

I think all of this horror show probably belongs in libpixman; we will
have similar issues in any pixel-based environment with existing pixel
formats.

> 
> > +    /* XXX this is wrong */
> >      if (image->depth > 16)
> >  	bitmap_pad = 32;
> >      else if (image->depth > 8)
> 
> What's the right fix here?

I'm sorry, I just punted for now.  The bitmap_pad is specified in the
image format and must match that.  libpixman uses a bitmap_pad of 32 in
all cases, but some X servers may differ.

> 
> I touched up the API documentation for the cairo_xlib_surface
> functions just a bit. Here's the documentation patch.

There's no need to add the 'set_size' Note in the _for_bitmap case.  If
you have a 1-bit window, then you have a visual for it and should use
the visual function.  The _for_bitmap function is strictly for depth-1
bitmaps to be used as masks in some fashion.

Thanks for the other doc changes; they look fine.

-keith

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050517/17f1caf9/attachment.pgp