Skip to content

Fix shared use of recording surfaces

Adrian Johnson requested to merge ajohnson/cairo:fix-shared-recording-surface into master

While working on PDF Type 3 color fonts (for color fonts with a CAIRO_SCALED_GLYPH_INFO_RECORDING_SURFACE) and debugging some issues, I found a problem with the recording surface. The problem is the region data created by _cairo_recording_surface_replay_and_create_regions and used by _cairo_recording_surface_replay_region is stored in the recording surface commands. The problem is the recording surface should be immutable as it can be used as a source by multiple targets.

This bug was probably not noticed as in most of the time a recording surface is only created within the paginated surface for one target then destroyed as soon as it has been replayed. When using the recording surface returned by _cairo_scaled_glyph_lookup, this recording may be used as a source by multiple paginated targets.

I've created a test that demonstrates the problem, however it won't fail in CI due to CI testing one target at a time. If you run the test manually with PDF/PS/SVG enabled you will see it fail due to re-using the same recording surface for multiple targets.

I've done some refactoring to move the region data into a separate cairo_recording_regions_array_t struct. There may be more than one of these attached to a recording surface. Each instance is identified by a region id. The functions for creating and destroying region arrays are:

cairo_status_t
_cairo_recording_surface_region_array_attach (cairo_surface_t *surface,
                                              unsigned int    *id);

Creates a new (empty) region array and returns a unique id for it.

void
_cairo_recording_surface_region_array_remove (cairo_surface_t *surface,
                                              unsigned int     id);

Destroy a region array.

_cairo_recording_surface_replay_and_create_regions takes a region id and creates the regions for it. _cairo_recording_surface_replay_region takes an region id and replays the regions for it.

The tricky part is nested recording surfaces. The PDF surface may hold a reference to a source surface after the paginated surface has destroyed the recording. The reference function allows the PDF surface to hold a reference to the the region array when is references a recording surface.

void
_cairo_recording_surface_region_array_reference (cairo_surface_t *surface,
                                                 unsigned int     id);

The only wrinkle is I needed a way to pass the region id to the target during playback when a source is a recording surface. I added a private region id to the pattern to avoid needing to add region ids to the backend operations which would require updating every surface. The wrapper surface makes a temporary copy of the pattern when it needs to read/write the region id field in the pattern so the immutability of the recording surface is not affected.

Merge request reports