[cairo] Pixmap double buffering in Xlib backend

[Keith Packard]

On Tue, 2005-07-19 at 15:46 -0400, Owen Taylor wrote:

> There is an "philosophical" question here about whether keeping 
> a cairo_t around over multiple expose events and drawing to it makes
> sense. The view I've been pushing is that a cairo_t represents the
> state of a single drawing operation, which makes the question of 
> how you deal with a changing size for the target of a cairo_t 
> irrelevant.
> 
> But I don't think we have to have consensus answer to this question.

Right, and the issue here is within a language binding, so you would
force a particular architecture upon all applications using that
binding...

> Even if this function is added, I don't think I'll change that: it
> would be a bit of a pain to do that with the way GDK is organized,
> and I've designed the API to encourage using GdkDrawable rather
> than cairo_surface_t in any case.

Right, I'm not force people to use this model, but rather that it does
have some value.

> But it does indicate to me that a cairo_surface_t that retains its
> identity across changes in the backing Xlib drawable could be 
> a desirable thing to have. 
> 
> On the other hand, the idea of copying state from cairo_t to cairo_t
> really is unattractive. It doesn't allow any abstraction from the
> user of what is going on, since the cairo_t changes. If we really
> wanted to handle it at the cairo_t level, I'd rather restore
> cairo_set_target().

That's why I like the backend-specific patch best of all, as there is a
logical connection between the two pixmaps.

> So, I'm in agreement with the course you suggest of 
> cairo_xlib_surface_set_drawable(). Looking at the proposed patch,
> 
> +/**
> + * cairo_xlib_surface_set_drawable:
> + * @surface: a #cairo_surface_t for the XLib backend
> + * @drawable: the new drawable for the surface
> + *
> + * Informs cairo of a new X Drawable underlying the
> + * surface. The drawable must match the format
> + * of the surface or the results are undefined
> + **/
> 
> Needs to also say that drawable must also match the existing
> display and screen of the surface. I'd make the wording stronger 
> by dropping "or the results are undefined", since that sounds
> a bit like the worst that could be expected is misdrawing.

/**
 * cairo_xlib_surface_set_drawable:
 * @surface: a #cairo_surface_t for the XLib backend
 * @drawable: the new drawable for the surface
 *
 * Informs cairo of a new X Drawable underlying the
 * surface. The drawable must match the display, screen
 * and format of the existing drawable or the application
 * will get X protocol errors and will probably terminate.
 * No checks are done by this function to ensure this
 * compatibility.
 **/

> 
> There also may need to be documentation of how this interacts
> with mark_dirty() and flush() once we add those functions,
> but that probably needs to wait until we add them...

Yes, I already noted this

> 
> +    /* XXX: and what about this case? */
> +    if (surface->owns_pixmap)
> +       return;
> 
> Definitely an error; the user doesn't have sufficient information
> about the results of create_similar() to be able to create a 
> new backing drawable for it. 

Right, we need to finish the error handling for surface_t so that this
can work correctly.

> The implementation itself looks fine to me.
> 
> While I don't think it matters much, I think I'd keep set_size()
> a separate function. It doesn't hurt anything to have it separate,
> it corresponds to the case of drawing to a Window, so why break
> compatibility for no reason.

I have left set_size as a separate function; the reason for including
the size with set_drawable is to avoid weird ordering issues and
potential weird ordering-related issues
(set-size before or aftet set-drawable?)

Without any dissent, I will commit the patch with the updated comment as
shown above in a couple of days.

Thanks for your review, Owen!

-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/20050719/a7925b84/attachment.pgp