[cairo] invalid size error, nth ping

[Paolo Bonzini]

Thanks for moving forward in the discussion!

> Well, users are not necessarily better than computers in predicting...
> Their setting may work good on one surface, but not the other.  And what does
> set_caching_policy (AUTO) mean anyway?  Shouldn't AUTO be the default?

AUTO is not "whether" to cache (the full name is
CAIRO_CACHE_ENABLE_AUTO, so the cache is enabled), but "how".

The policy defines what to do with dirty cached data.  MANUAL (use dirty
data unless explicitly flushed) was only for backwards-compatibility
with the Quartz image surface, because that's what it implemented.  AUTO
and DISCARD make a difference only if >1 cached representations are
there, say Quartz and Glitz: if they are dirty and the Glitz backend
requires its representation, DISCARD throws away the Quartz
representation, while AUTO asks the Quartz backend what to do.

If you decide only one (probably AUTO) is needed, I can remove the other
one and just have CAIRO_CACHE_{DISABLE,ENABLE}.

> 1) For image surfaces we know if the user has direct access to the data.
> 2) Technically users are supposed to call cairo_surface_mark_dirty if they
> modify it.  No one does now of course because that call's pretty much a nop.

Oh, that's good news.  On the other hand the chance of bugs is high,
because mark_dirty_rectangle is only implemented by the OS/2 surface
now.  So I'm not sure that it means we can start caching stuff
automatically, especially if the widely used Xlib surface starts using
the cache (e.g. for Xshm).

One alternative could be to add optional caching in 1.10 with a big
warning on the release notes, and add heuristics in 1.12.  Either
approach has its drawbacks of course.  Even if heuristics are to be
added in 1.10, it can be done on top of the patches in the branch (i.e.
I won't disappear).

A note on the brain dump from Chris, at least the part that I understood:

> cairo_surface_t *cairo_drm_surface_map (cairo_surface_t *);
> void cairo_drm_surface_unmap (cairo_surface_t *drm,
>                               cairo_surface_t *image);

I don't know if it considered proper design in Cairo to have two
surfaces with similar backends return the same surface type.  If it is,
I don't see the need for cairo_drm_surface_unmap.  You could instead use
cairo_surface_flush to copy CPU->GPU, and cairo_surface_mark_dirty to
copy back GPU->CPU without calling again cairo_drm_surface_map.

--

So, to sum up, what I did was:

1) Extract the first four patches to a "snapshot-opt" branch.  They were
quite unrelated, and can be reviewed separately:

    [quartz] remove unused argument
    [surface] add surface snapshots
    [surface] add acquire_snapshot_image
    [quartz] do not snapshot sources if not necessary

2) I renamed the functions to {get,set}_caching_policy.

3) I made my branch delete the quartz-image surface completely.  It is
not used widely anyway, and the new mechanism subsumes it even though
incompatibly (cairo_surface_mark_dirty vs. cairo_surface_flush).

4) Therefore, I removed the manual caching policy.  It is separated into
a different patch that I'm keeping during review, but I don't plan on
that one being applied.  These are the patches now in the branch, which
is based over the snapshot-opt branch:

    [quartz-image] delete quartz image surface
    [image/quartz] add image surface cache and use it for Quartz
    [image] add manual cache management
    [glitz] use image cache

Paolo