[Cairo] virtualizing the surface interface

[Carl Worth]

On Sep 26, Jamey Sharp wrote:
 > So I first started reading Cairo sources less than 24 hours ago, and
 > here I have a fairly significant patch I'd like to propose for it.

Jamey,

I'm very glad to have you looking into this. Thanks for the patch!
I've only briefly glanced at it, but I'll go ahead and respond to some
points you bring up in your message. More specific comments on the
patch itself will come later.

 > The big changes are the introduction of the 'struct cairo_surface_impl'
 > type, with function pointers for implementations of the operations on
 > surfaces; 

Great. I prefer names without abbreviations, but I won't reject the
patch because of that. It's just internal and I'll just rename the
struct later if it bugs me too much.

 > and the removal of almost all of Cairo's Xlib-specific code to
 > a new source file called cairo_surface_xlib.c.

I had been planning to name the graphics system-specific surfaces
cairo_foo_surface, so I would suggest cairo_xlib_surface.c
instead. The idea was that the user would then get access to all
Xlib-dependent functionality by including cairo-xlib.h. (Yes, there's
still the issue about how to name the font system-specific headers.)

Similarly, I'd like to rename cairo_surface_create_for_drawable to
cairo_xlib_surface_create and move it from cairo.h to cairo-xlib.h.

 > The remaining X dependencies are on Xft (in cairo_gstate.c and
 > cairo_font.c), and this patch #if 0's those bits into no-ops. I
 > don't understand fonts well enough to fix that yet, so I'm hoping
 > somebody else will. :-)

The Xft dependencies in cairo_gstate are bogus. These should be pushed
down one level. Then we should probably virtualize cairo_font in a
manner similar to cairo_surface. There are probably still some
unresolved issue about the interface between the various cairo_font
and cairo_surface implementations. Those should become more clear as
the new font API work comes together, (I'm hoping to see something
from Graydon soon).

 > For the moment, I've chosen to prefer the server, compositing on
 > the client only if the server can't do it or the destination is not
 > on an X display. The existing code chose the other way.

I know the decisions about where to do compositing in the existing
code are nowhere near optimal. There is some interesting
experimentation to be done about where data lives, (library, server
main memory, or server video memory) and how it moves.

 > If there is a problem, my patch will probably show it even more
 > than the current library (but is anyone actually using Cairo with
 > multiple Displays in a single client?).

I'm sure there are plenty of bugs here. Untested code is broken code
and the multiple display stuff is totally untested. Since nobody's
complained I know that nobody has tried to use it. The cool part of
the library is that supporting this bizarre case isn't much work; it
just falls out of the architecture. (Of course, making it actually
work and optimizing it would take some work).

 > I think this patch fixes the problem with GetGeometry being called on an
 > already-freed Pixmap, without re-adding the pixmap leak.

Thanks for looking into this.

-Carl