[cairo] [cairo-commit] src/cairo-pdf-surface.c src/cairo-pdf-surface-private.h src/cairo-recording-surface.c src/cairo-recording-surface-private.h
Uli Schlachter
psychon at znc.in
Wed Sep 11 05:43:40 PDT 2013
Hi,
I got two comments about this commit:
On 11.09.2013 13:50, Adrian Johnson wrote:
> src/cairo-pdf-surface-private.h | 2
> src/cairo-pdf-surface.c | 125 +++++++++++++++++++++++++---------
> src/cairo-recording-surface-private.h | 8 ++
> src/cairo-recording-surface.c | 113 ++++++++++++++++++++++++++++++
> 4 files changed, 215 insertions(+), 33 deletions(-)
>
> New commits:
> commit 8addb4798c918000eaa6f6dab138e0abb0efa946
> Author: Adrian Johnson <ajohnson at redneon.com>
> Date: Sun Apr 8 10:57:23 2012 +0930
>
> pdf: avoid making groups a transparency group if not required
>
> If the group contains only a combination of clear and opaque alpha and
> only OPERATOR_OVER is used in the group and to paint the group, a
> transparency group is not required. This allows the pdf viewer to
> replay the group in place.
>
[...]
> +static void
> +_cairo_recording_surface_merge_source_attributes (cairo_recording_surface_t *surface,
> + cairo_operator_t op,
> + const cairo_pattern_t *source)
> +{
> + if (op != CAIRO_OPERATOR_OVER)
> + surface->has_only_op_over = FALSE;
> +
> + if (source->type == CAIRO_PATTERN_TYPE_SURFACE) {
> + cairo_surface_pattern_t *surf_pat = (cairo_surface_pattern_t *) source;
> + cairo_surface_t *surf = surf_pat->surface;
> + cairo_surface_t *free_me;
free_me should be initialized to NULL (gcc warns about an uninitialized variable
being passed to cairo_surface_destroy()).
> + if (_cairo_surface_is_snapshot (surf))
> + free_me = surf = _cairo_surface_snapshot_get_target (surf);
> +
> + if (surf->type == CAIRO_SURFACE_TYPE_RECORDING) {
> + cairo_recording_surface_t *rec_surf = (cairo_recording_surface_t *) surf;
> +
> + if (! _cairo_recording_surface_has_only_bilevel_alpha (rec_surf))
> + surface->has_bilevel_alpha = FALSE;
> +
> + if (! _cairo_recording_surface_has_only_op_over (rec_surf))
> + surface->has_only_op_over = FALSE;
> +
> + } else if (surf->type == CAIRO_SURFACE_TYPE_IMAGE) {
> + cairo_image_surface_t *img_surf = (cairo_image_surface_t *) surf;
> +
> + if (_cairo_image_analyze_transparency (img_surf) == CAIRO_IMAGE_HAS_ALPHA)
> + surface->has_bilevel_alpha = FALSE;
> +
> + }
This function does nothing if I pass in e.g. a cairo-gl surface with an alpha
channel. I think that the above "if" needs another else case:
} else {
if (!_cairo_pattern_is_clear (source)
&& !_cairo_pattern_is_opaque (source, NULL))
surface->has_bilevel_alpha = FALSE;
}
(At least these are the most suitable functions that I found for this...)
> + cairo_surface_destroy (free_me);
> + return;
> +
> + } else if (source->type == CAIRO_PATTERN_TYPE_RASTER_SOURCE) {
> + cairo_surface_t *image;
> + cairo_surface_t *raster;
> +
> + image = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, 1, 1);
> + raster = _cairo_raster_source_pattern_acquire (source, image, NULL);
> + cairo_surface_destroy (image);
> + if (raster) {
> + if (raster->type == CAIRO_SURFACE_TYPE_IMAGE) {
> + if (_cairo_image_analyze_transparency ((cairo_image_surface_t *)raster) == CAIRO_IMAGE_HAS_ALPHA)
> + surface->has_bilevel_alpha = FALSE;
> + }
> +
> + _cairo_raster_source_pattern_release (source, raster);
> + if (raster->type == CAIRO_SURFACE_TYPE_IMAGE)
> + return;
> + }
> + }
> +
> + if (!_cairo_pattern_is_clear (source) && !_cairo_pattern_is_opaque (source, NULL))
> + surface->has_bilevel_alpha = FALSE;
> +}
[...]
Are my observations correct? Should I commit and push these changes? Should a
suitable test be added to the test suite?
Cheers,
Uli
--
A normal person is just someone you don't know well enough yet.
- Nettie Wiebe
More information about the cairo
mailing list