[cairo] Clip region problems

[Owen Taylor]

On Tue, 2005-05-17 at 23:00 -0700, Keith Packard wrote:
> On Mon, 2005-05-02 at 16:48 -0400, Owen Taylor wrote:
> 
> (a long discussion about clip lists in surfaces)
> 
> I've redone the nickle binding (cairo-5c) and the
> new restriction that each surface be targetted by only one cairo_t at a
> time is causing existing code to break.
> 
> I have a multi-threaded application drawing several different objects to
> the same surface; each thread wants to have a private graphics state,
> but they all want to share the same surface.

I assume you have application locking in place here? Or are you
proposing we add explicit locking for the internal state of
cairo_surface_t and the various backends.

(Note that we do things like temporarily change the picture 
attributes of surfaces when using them as sources... the need
to lock *two* surfaces for a drawing operation would definitely
raise some fun deadlock issues)

> Before the BAD_NESTING state, this worked just fine.  

Actually, no it didn't, not if you were using clipping :-)

> Now I get an assertion failure and my program aborts.

I'm actually curious why you got an assertion failure rather than
a silent status (I have the suspicion that there might be some bugs in 
cairo in this area)

But in any case, it seems that the BAD_NESTING error handling is
achieving its purpose ... to catch problems and make people think
about better ways of handling the situation :-)

> I think we should revisit the discussion which prompted the current
> state and see if we really want to keep things like this.
> 
> To me, a surface is an object without any state aside from the image
> drawn there. 

Any *public* state presumably. But this model really only applies at
most to "pixel" surfaces. There is obviously other state in, for 
example, a paginated surface.

> Ok, so I believe the BAD_NESTING status was added to allow the cairo_t
> to place clipping information directly into the backend for efficient
> rectangular pixel-aligned clipping.  A key requirement for efficient
> cairo usage in a windowed environment.

Well, that's a bit ahistorical ... the clipping was placed into the
backend, and then later the clipping stack (and thus the BAD_NESTING
error) was added to make it actually work.

(Clipping isn't usable in 0.4.0, because it leaks out onto the next
user of the surface. This was a huge problem for GTK+.)

One of the understandings of adding nesting restrictions is that they
could always be loosened later if we figured out better ways of doing
things.

> The cairo_surface_t clip region is usually just a copy of the gstate
> clip region, at which times it would be more efficient to just use the
> gstate clip_region directly and not store it in the cairo_surface_t as
> well.  The only unusual case is when compositing trapezoids, the
> cairo_surface_t clip region is temporarily set to the trapezoid region.
> 
> It seems to me what we want is instead of making the surface push/pop
> clip lists, we just want the gstate to tell the surface which clip list
> is in use for each operation and then have the surface cache updates to
> the backend itself.

One thing to be aware of here is that we almost certainly need clip
*paths* in the surface vtable for the metasurface and the PDF/PS
backends. The interface that I've been discussing with Kristian is
along the lines of:

 surface.intersect_clip_with_path (path);
 surface.reset_clip ();

(The reason it's intersect_clip_with_path() rather than set_clip_path()
is to allow us to avoid doing complex computation geometry for 
path intersection)

So any solution that is specific to rectangular clip lists (for example,
the COW pixman_region objects that were discussed earlier), isn't going
to work.

> We just need a way to globally identify region contents and the surface
> can tell when the backend must be informed about a new clip list.  In a
> similar situation, the X server uses 'serial numbers' -- integers
> created by a global counter.  This will be hard in cairo without
> locking, but as a cairo_t (and gstate, by associatio) can only be used
> with a single surface_t, it seems like we can place a serial number
> generator in each surface and use that.
> 
> Surely I'm missing something here; this seems like it will be both more
> general (allowing parallel cairo_t objects to reference the same
> surface) and more efficient (by eliminating duplicate regions in the
> cairo_surface_t).

So, what you are proposing is basically, that before every drawing
operation, we do:

 if (gstate->clip_serial != gstate->surface->clip_serial)
    gstate_reestablish_clip (gstate);

(With some available invalid value used initially for gstate-
>clip_serial)

It's an approach I hadn't thought of, and might in fact be workable.

* It does remove some information from the backend. It's going to 
be harder to optimize sequences like:

 - Clip to one path
 - Save
 - Clip to another path
 - draw
 - Restore
 - draw 

In the PS or PDF backend, because the surface no longer sees the
stack of clips. While in PS/PDF it would be possible to encode the
above with only two clips, with the clip serial approach, the
end result is:

 - Save
 - Clip to one path
 - Clip to another path
 - draw
 - Restore
 - Save
 - Clip to one path
 - draw
 - Restore

Nested clipping when drawing to paper isn't probably common enough
to make this a big worry.

* Using a surface as a source for drawing to itself is going to be a
problem, but it's already a problem, so that's nothing new.

In general, it seems like a promising idea.

Regards,
						Owen

-------------- 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/20050518/207f6b08/attachment.pgp