Re: Inline SVG images and the need to prefix identifiers

On Sat, Oct 24, 2015 at 10:36:33AM +0200, Christian von Schultz wrote:
> On Fri, 2015-10-23 at 19:59 +0200, Uli Schlachter wrote:
> > This needs a "Since: 1.16".
> 
> New patch attached.
> 
> Cheers,
> Christian

> From c8acd6b6f73a8604e9d45c94fb562495f608a683 Mon Sep 17 00:00:00 2001
> From: Christian von Schultz <[email protected]>
> Date: Fri, 23 Oct 2015 15:05:29 +0200
> Subject: [PATCH] cairo-svg: Added cairo_svg_surface_set_id_attribute_prefix()
>  and related code.
> 
> Inserts document->idprefix in every id attribute of the generated SVG file.
> This is useful if several SVG files will be merged in a single
> XML document (e.g. XHTML with inline SVG): You can prevent otherwise
> inevitable id clashes by giving the different SVG files different
> idprefix values.
> ---
>  src/cairo-svg-surface.c | 136 ++++++++++++++++++++++++++++++++++++------------
>  src/cairo-svg.h         |   4 ++
>  2 files changed, 108 insertions(+), 32 deletions(-)

Sorry I'm tardy about a year in commenting on this, but this does seem
like it would be a handy feature for the generated SVG files, where
the contents will be merged into another document.  I'd love to see
other people's thoughts.

I did a cursory look through the code and didn't spot any errors, and
agree with the general concept so:

Acked-by: Bryce Harrington <[email protected]>


> diff --git a/src/cairo-svg-surface.c b/src/cairo-svg-surface.c
> index 2e023b3..9f3c111 100644
> --- a/src/cairo-svg-surface.c
> +++ b/src/cairo-svg-surface.c
> @@ -139,6 +139,8 @@ struct cairo_svg_document {
>      cairo_output_stream_t *xml_node_defs;
>      cairo_output_stream_t *xml_node_glyphs;
>  
> +    char *idprefix;
> +
>      unsigned int linear_pattern_id;
>      unsigned int radial_pattern_id;
>      unsigned int pattern_id;
> @@ -330,6 +332,38 @@ _extract_svg_surface (cairo_surface_t		 *surface,
>  }
>  
>  /**
> + * cairo_svg_surface_set_id_attribute_prefix:
> + * @surface: a SVG #cairo_surface_t
> + * @prefix:  a char*
> + *
> + * Inserts @prefix in every id attribute of the generated SVG file.
> + * This is useful if several SVG files will be merged in a single
> + * XML document (e.g. XHTML with inline SVG): You can prevent otherwise
> + * inevitable id clashes by giving the different SVG files different
> + * @prefix values.
> + *
> + * Since: 1.16
> + **/
> +cairo_status_t
> +cairo_svg_surface_set_id_attribute_prefix (cairo_surface_t	*abstract_surface,
> +					   const char		*prefix)
> +{
> +    cairo_svg_surface_t *surface = NULL; /* hide compiler warning */
> +    size_t size;
> +
> +    if (! _extract_svg_surface (abstract_surface, &surface))
> +	return CAIRO_STATUS_SURFACE_TYPE_MISMATCH;
> +
> +    size = strlen(prefix) + 1;
> +    surface->document->idprefix = realloc (surface->document->idprefix, size);
> +    if (unlikely (surface->document->idprefix == NULL))
> +	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
> +    strncpy(surface->document->idprefix, prefix, size);
> +
> +    return CAIRO_STATUS_SUCCESS;
> +}
> +
> +/**
>   * cairo_svg_surface_restrict_to_version:
>   * @surface: a SVG #cairo_surface_t
>   * @version: SVG version
> @@ -444,8 +478,9 @@ _cairo_svg_surface_clipper_intersect_clip_path (cairo_surface_clipper_t *clipper
>  	return CAIRO_STATUS_SUCCESS;
>  
>      _cairo_output_stream_printf (document->xml_node_defs,
> -				 "<clipPath id=\"clip%d\">\n"
> +				 "<clipPath id=\"%sclip%d\">\n"
>  				 "  <path ",
> +				 document->idprefix,
>  				 document->clip_id);
>      _cairo_svg_surface_emit_path (document->xml_node_defs, path, NULL);
>  
> @@ -454,8 +489,9 @@ _cairo_svg_surface_clipper_intersect_clip_path (cairo_surface_clipper_t *clipper
>  				 "</clipPath>\n");
>  
>      _cairo_output_stream_printf (surface->xml_node,
> -				 "<g clip-path=\"url(#clip%d)\" "
> +				 "<g clip-path=\"url(#%sclip%d)\" "
>  				 "clip-rule=\"%s\">\n",
> +				 document->idprefix,
>  				 document->clip_id,
>  				 fill_rule == CAIRO_FILL_RULE_EVEN_ODD ?
>  				 "evenodd" : "nonzero");
> @@ -840,7 +876,8 @@ _cairo_svg_document_emit_glyph (cairo_svg_document_t	*document,
>      cairo_int_status_t	     status;
>  
>      _cairo_output_stream_printf (document->xml_node_glyphs,
> -				 "<symbol overflow=\"visible\" id=\"glyph%d-%d\">\n",
> +				 "<symbol overflow=\"visible\" id=\"%sglyph%d-%d\">\n",
> +				 document->idprefix,
>  				 font_id,
>  				 subset_glyph_index);
>  
> @@ -1026,14 +1063,15 @@ _cairo_svg_surface_emit_alpha_filter (cairo_svg_document_t *document)
>  	return;
>  
>      _cairo_output_stream_printf (document->xml_node_defs,
> -				 "<filter id=\"alpha\" "
> +				 "<filter id=\"%salpha\" "
>  				 "filterUnits=\"objectBoundingBox\" "
>  				 "x=\"0%%\" y=\"0%%\" "
>  				 "width=\"100%%\" height=\"100%%\">\n"
>  				 "  <feColorMatrix type=\"matrix\" "
>  				 "in=\"SourceGraphic\" "
>  				 "values=\"0 0 0 0 1 0 0 0 0 1 0 0 0 0 1 0 0 0 1 0\"/>\n"
> -				 "</filter>\n");
> +				 "</filter>\n",
> +				 document->idprefix);
>  
>      document->alpha_filter = TRUE;
>  }
> @@ -1300,7 +1338,8 @@ _cairo_svg_surface_emit_surface (cairo_svg_document_t *document,
>      assert (is_bounded);
>  
>      _cairo_output_stream_printf (document->xml_node_defs,
> -				 "<image id=\"image%d\" width=\"%d\" height=\"%d\"",
> +				 "<image id=\"%simage%d\" width=\"%d\" height=\"%d\"",
> +				 document->idprefix,
>  				 surface->unique_id,
>  				 extents.width, extents.height);
>  
> @@ -1356,9 +1395,10 @@ _cairo_svg_surface_emit_composite_surface_pattern (cairo_output_stream_t   *outp
>  	assert (is_bounded);
>  
>  	_cairo_output_stream_printf (output,
> -				     "<pattern id=\"pattern%d\" "
> +				     "<pattern id=\"%spattern%d\" "
>  				     "patternUnits=\"userSpaceOnUse\" "
>  				     "width=\"%d\" height=\"%d\" ",
> +				     svg_surface->document->idprefix,
>  				     pattern_id,
>  				     extents.width, extents.height);
>  	_cairo_svg_surface_emit_transform (output,
> @@ -1368,7 +1408,8 @@ _cairo_svg_surface_emit_composite_surface_pattern (cairo_output_stream_t   *outp
>      }
>  
>      _cairo_output_stream_printf (output,
> -				 "<use xlink:href=\"#image%d\"",
> +				 "<use xlink:href=\"#%simage%d\"",
> +				 svg_surface->document->idprefix,
>  				 pattern->surface->unique_id);
>      if (extra_attributes)
>  	_cairo_output_stream_printf (output, " %s", extra_attributes);
> @@ -1437,9 +1478,10 @@ _cairo_svg_surface_emit_recording_surface (cairo_svg_document_t      *document,
>      if (! svg_surface->is_base_clip_emitted) {
>  	svg_surface->is_base_clip_emitted = TRUE;
>  	_cairo_output_stream_printf (document->xml_node_defs,
> -				     "<clipPath id=\"clip%d\">\n"
> +				     "<clipPath id=\"%sclip%d\">\n"
>  				     "  <rect width=\"%f\" height=\"%f\"/>\n"
>  				     "</clipPath>\n",
> +				     document->idprefix,
>  				     svg_surface->base_clip,
>  				     svg_surface->width,
>  				     svg_surface->height);
> @@ -1448,16 +1490,21 @@ _cairo_svg_surface_emit_recording_surface (cairo_svg_document_t      *document,
>      if (source->base.content == CAIRO_CONTENT_ALPHA) {
>  	_cairo_svg_surface_emit_alpha_filter (document);
>  	_cairo_output_stream_printf (document->xml_node_defs,
> -				     "<g id=\"surface%d\" "
> -				     "clip-path=\"url(#clip%d)\" "
> -				     "filter=\"url(#alpha)\">\n",
> +				     "<g id=\"%ssurface%d\" "
> +				     "clip-path=\"url(#%sclip%d)\" "
> +				     "filter=\"url(#%salpha)\">\n",
> +				     document->idprefix,
>  				     source->base.unique_id,
> -				     svg_surface->base_clip);
> +				     document->idprefix,
> +				     svg_surface->base_clip,
> +				     document->idprefix);
>      } else {
>  	_cairo_output_stream_printf (document->xml_node_defs,
> -				     "<g id=\"surface%d\" "
> -				     "clip-path=\"url(#clip%d)\">\n",
> +				     "<g id=\"%ssurface%d\" "
> +				     "clip-path=\"url(#%sclip%d)\">\n",
> +				     document->idprefix,
>  				     source->base.unique_id,
> +				     document->idprefix,
>  				     svg_surface->base_clip);
>      }
>  
> @@ -1529,9 +1576,10 @@ _cairo_svg_surface_emit_composite_recording_pattern (cairo_output_stream_t	*outp
>  
>      if (pattern_id != invalid_pattern_id) {
>  	_cairo_output_stream_printf (output,
> -				     "<pattern id=\"pattern%d\" "
> +				     "<pattern id=\"%spattern%d\" "
>  				     "patternUnits=\"userSpaceOnUse\" "
>  				     "width=\"%d\" height=\"%d\"",
> +				     document->idprefix,
>  				     pattern_id,
>  				     recording_surface->extents.width,
>  				     recording_surface->extents.height);
> @@ -1540,7 +1588,8 @@ _cairo_svg_surface_emit_composite_recording_pattern (cairo_output_stream_t	*outp
>      }
>  
>      _cairo_output_stream_printf (output,
> -				 "<use xlink:href=\"#surface%d\"",
> +				 "<use xlink:href=\"#%ssurface%d\"",
> +				 document->idprefix,
>  				 recording_surface->base.unique_id);
>  
>      if (pattern_id == invalid_pattern_id) {
> @@ -1620,8 +1669,9 @@ _cairo_svg_surface_emit_surface_pattern (cairo_svg_surface_t	 *surface,
>  	return status;
>  
>      _cairo_output_stream_printf (style,
> -				 "%s:url(#pattern%d);",
> +				 "%s:url(#%spattern%d);",
>  				 is_stroke ? "stroke" : "fill",
> +				 document->idprefix,
>  				 pattern_id);
>  
>      return CAIRO_STATUS_SUCCESS;
> @@ -1824,9 +1874,10 @@ _cairo_svg_surface_emit_linear_pattern (cairo_svg_surface_t    *surface,
>      assert (status == CAIRO_STATUS_SUCCESS);
>  
>      _cairo_output_stream_printf (document->xml_node_defs,
> -				 "<linearGradient id=\"linear%d\" "
> +				 "<linearGradient id=\"%slinear%d\" "
>  				 "gradientUnits=\"userSpaceOnUse\" "
>  				 "x1=\"%f\" y1=\"%f\" x2=\"%f\" y2=\"%f\" ",
> +				 document->idprefix,
>  				 document->linear_pattern_id,
>  				 pattern->pd1.x, pattern->pd1.y,
>  				 pattern->pd2.x, pattern->pd2.y);
> @@ -1845,8 +1896,9 @@ _cairo_svg_surface_emit_linear_pattern (cairo_svg_surface_t    *surface,
>  				 "</linearGradient>\n");
>  
>      _cairo_output_stream_printf (style,
> -				 "%s:url(#linear%d);",
> +				 "%s:url(#%slinear%d);",
>  				 is_stroke ? "stroke" : "fill",
> +				 document->idprefix,
>  				 document->linear_pattern_id);
>  
>      document->linear_pattern_id++;
> @@ -1898,10 +1950,11 @@ _cairo_svg_surface_emit_radial_pattern (cairo_svg_surface_t    *surface,
>  	unsigned int n_stops = pattern->base.n_stops;
>  
>  	_cairo_output_stream_printf (document->xml_node_defs,
> -				     "<radialGradient id=\"radial%d\" "
> +				     "<radialGradient id=\"%sradial%d\" "
>  				     "gradientUnits=\"userSpaceOnUse\" "
>  				     "cx=\"%f\" cy=\"%f\" "
>  				     "fx=\"%f\" fy=\"%f\" r=\"%f\" ",
> +				     document->idprefix,
>  				     document->radial_pattern_id,
>  				     x1, y1,
>  				     x1, y1, r1);
> @@ -1979,10 +2032,11 @@ _cairo_svg_surface_emit_radial_pattern (cairo_svg_surface_t    *surface,
>  	}
>  
>  	_cairo_output_stream_printf (document->xml_node_defs,
> -				     "<radialGradient id=\"radial%d\" "
> +				     "<radialGradient id=\"%sradial%d\" "
>  				     "gradientUnits=\"userSpaceOnUse\" "
>  				     "cx=\"%f\" cy=\"%f\" "
>  				     "fx=\"%f\" fy=\"%f\" r=\"%f\" ",
> +				     document->idprefix,
>  				     document->radial_pattern_id,
>  				     x1, y1,
>  				     fx, fy, r1);
> @@ -2027,8 +2081,9 @@ _cairo_svg_surface_emit_radial_pattern (cairo_svg_surface_t    *surface,
>  				 "</radialGradient>\n");
>  
>      _cairo_output_stream_printf (style,
> -				 "%s:url(#radial%d);",
> +				 "%s:url(#%sradial%d);",
>  				 is_stroke ? "stroke" : "fill",
> +				 document->idprefix,
>  				 document->radial_pattern_id);
>  
>      document->radial_pattern_id++;
> @@ -2427,10 +2482,14 @@ _cairo_svg_surface_mask (void		    *abstract_surface,
>      mask_id = _cairo_svg_document_allocate_mask_id (document);
>  
>      _cairo_output_stream_printf (mask_stream,
> -				 "<mask id=\"mask%d\">\n"
> -				 "%s",
> -				 mask_id,
> -				 discard_filter ? "" : "  <g filter=\"url(#alpha)\">\n");
> +				 "<mask id=\"%smask%d\">\n",
> +				 document->idprefix,
> +				 mask_id);
> +    if (!discard_filter) {
> +	_cairo_output_stream_printf (mask_stream,
> +				     "  <g filter=\"url(#%salpha)\">\n",
> +				     document->idprefix);
> +    }
>      status = _cairo_svg_surface_emit_paint (mask_stream, surface, CAIRO_OPERATOR_OVER, mask, source, NULL);
>      if (unlikely (status)) {
>  	cairo_status_t ignore = _cairo_output_stream_destroy (mask_stream);
> @@ -2448,7 +2507,8 @@ _cairo_svg_surface_mask (void		    *abstract_surface,
>      if (unlikely (status))
>  	return status;
>  
> -    snprintf (buffer, sizeof buffer, "mask=\"url(#mask%d)\"",
> +    snprintf (buffer, sizeof buffer, "mask=\"url(#%smask%d)\"",
> +	      document->idprefix,
>  	      mask_id);
>      status = _cairo_svg_surface_emit_paint (surface->xml_node, surface, op, source, 0, buffer);
>      if (unlikely (status))
> @@ -2558,8 +2618,9 @@ _cairo_svg_surface_show_glyphs (void			*abstract_surface,
>  	    return status;
>  
>  	_cairo_output_stream_printf (surface->xml_node,
> -				     "  <use xlink:href=\"#glyph%d-%d\" "
> +				     "  <use xlink:href=\"#%sglyph%d-%d\" "
>  				     "x=\"%f\" y=\"%f\"/>\n",
> +				     document->idprefix,
>  				     subset_glyph.font_id,
>                                       subset_glyph.subset_glyph_index,
>  				     glyphs[i].x, glyphs[i].y);
> @@ -2663,11 +2724,17 @@ _cairo_svg_document_create (cairo_output_stream_t	 *output_stream,
>      if (unlikely (document == NULL))
>  	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
>  
> +    document->idprefix = strdup("");
> +    if (unlikely (document->idprefix == NULL)) {
> +	status = _cairo_error (CAIRO_STATUS_NO_MEMORY);
> +	goto CLEANUP_DOCUMENT;
> +    }
> +
>      /* The use of defs for font glyphs imposes no per-subset limit. */
>      document->font_subsets = _cairo_scaled_font_subsets_create_scaled ();
>      if (unlikely (document->font_subsets == NULL)) {
>  	status = _cairo_error (CAIRO_STATUS_NO_MEMORY);
> -	goto CLEANUP_DOCUMENT;
> +	goto CLEANUP_IDPREFIX;
>      }
>  
>      document->output_stream = output_stream;
> @@ -2706,6 +2773,8 @@ _cairo_svg_document_create (cairo_output_stream_t	 *output_stream,
>    CLEANUP_NODE_DEFS:
>      status_ignored = _cairo_output_stream_destroy (document->xml_node_defs);
>      _cairo_scaled_font_subsets_destroy (document->font_subsets);
> +  CLEANUP_IDPREFIX:
> +    free (document->idprefix);
>    CLEANUP_DOCUMENT:
>      free (document);
>      return status;
> @@ -2736,6 +2805,7 @@ _cairo_svg_document_destroy (cairo_svg_document_t *document)
>  
>      status = _cairo_svg_document_finish (document);
>  
> +    free (document->idprefix);
>      free (document);
>  
>      return status;
> @@ -2817,7 +2887,8 @@ _cairo_svg_document_finish (cairo_svg_document_t *document)
>  		page = _cairo_array_index (&surface->page_set, i);
>  		_cairo_output_stream_printf (output, "<page>\n");
>  		_cairo_output_stream_printf (output,
> -					     "<g id=\"surface%d\">\n",
> +					     "<g id=\"%ssurface%d\">\n",
> +					     document->idprefix,
>  					     page->surface_id);
>  		_cairo_memory_stream_copy (page->xml_node, output);
>  		_cairo_output_stream_printf (output, "</g>\n</page>\n");
> @@ -2826,7 +2897,8 @@ _cairo_svg_document_finish (cairo_svg_document_t *document)
>  	} else if (surface->page_set.num_elements > 0) {
>  	    page = _cairo_array_index (&surface->page_set, surface->page_set.num_elements - 1);
>  	    _cairo_output_stream_printf (output,
> -					 "<g id=\"surface%d\">\n",
> +					 "<g id=\"%ssurface%d\">\n",
> +					 document->idprefix,
>  					 page->surface_id);
>  	    _cairo_memory_stream_copy (page->xml_node, output);
>  	    _cairo_output_stream_printf (output, "</g>\n");
> diff --git a/src/cairo-svg.h b/src/cairo-svg.h
> index 592c645..71ac4f5 100644
> --- a/src/cairo-svg.h
> +++ b/src/cairo-svg.h
> @@ -64,6 +64,10 @@ cairo_svg_surface_create_for_stream (cairo_write_func_t	write_func,
>  				     double		width_in_points,
>  				     double		height_in_points);
>  
> +cairo_public cairo_status_t
> +cairo_svg_surface_set_id_attribute_prefix (cairo_surface_t	*abstract_surface,
> +					   const char		*prefix);
> +
>  cairo_public void
>  cairo_svg_surface_restrict_to_version (cairo_surface_t 		*surface,
>  				       cairo_svg_version_t  	 version);
> -- 
> 2.4.10
> 

> -- 
> cairo mailing list
> [email protected]
> http://lists.cairographics.org/mailman/listinfo/cairo

-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo