[cairo] Workaround patch for Xserver repeating pattern bug
otaylor at redhat.com
Mon Jun 20 15:42:47 PDT 2005
On Mon, 2005-06-20 at 15:24 -0700, Carl Worth wrote:
> > * I haven't tried to use the Xlib fallback code for the case where
> > we don't have RENDER at all, though it is equally applicable
> > there.
> That one seems worth a comment in the code to me.
> > * A big problem with the core protocol fallback is that the
> > screens must match for the source and destination drawables,
> > but we don't have that information, since the screen or
> > root window isn't a parameter to the Xlib surface constructor.
> > Rather than round-tripping, I've just disabled the optimization
> > for multi-head displays.
> Ouch. We should figure out how to fix this. Obviously we could add
> cairo_xlib_surface_set_screen, but then the user would have no way of
> knowing whether it was important to call this function in order to
> avoid a round-trip. All we could say in the documentation is that the
> user should _always_ call it just in case. But then, aren't we just
> documenting the presence of a performance trap.
> As much as I hate to rev. it again, and as much as I hate to add yet
> another parameter to cairo_xlib_surface_create, perhaps that's what's
> forced on us here, (by what I consider a protocol defect). Keith,
Certainly most of the same arguments apply here as width/height.
I sort of gave up on fighting over the interface then, so I'll leave
it to you and Keith.
(Just like the width/height case, we can frequently avoid the
roundtrip if we are sufficiently smart, but not always.)
> Heck, if nothing else, changing cairo_xlib_surface_create will fix one
> thing I noticed when porting fdclock this morning. The current
> (Display*, Drawable, Visual*, int width, int height)
> just so happens to be type-compatible with an old prototype:
> (Display*, Drawable, Visual*, cairo_format_t, Colormap)
> Which, as you might imagine, led to some not-too-useful results when
> > In general, I'm not terribly happy with the patch though I think
> > it should work OK and I've tried to keep it pretty clean and
> > well documented: the whole way that _cairo_pattern_acquire_surfaces()
> > works makes it hard to do better.
> I agree. But I don't have any quick fix for acquire_surfaces. The
> current patch looks quite functional to me, and is well-commented. The
> redundancy is unfortunate, (both in readability in performance), but I
> think it's clear enough to guide us through a better implementation
> So, given the urgency of this workaround, I think the patch is worth
> committing, (with the important fix noted below).
> > + if (!(!have_mask &&
> > + (operator == CAIRO_OPERATOR_SOURCE ||CAIRO_OPERATOR_OVER)))
> > + return DO_UNSUPPORTED;
> That test is obviously broken. I assume it should instead be:
> if (!(!have_mask &&
> (operator == CAIRO_OPERATOR_SOURCE ||
> operator == CAIRO_OPERATOR_OVER)))
> return DO_UNSUPPORTED;
> The tripled negative does still make it hard to read, but I don't see
> how to do much better.
I applied one level of DeMorgan's to get:
if (have_mask ||
!(operator == CAIRO_OPERATOR_SOURCE ||
operator == CAIRO_OPERATOR_OVER))
I'm not sure that's clearer, but it's at least prettier.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050620/1ba8aabd/attachment.pgp
More information about the cairo