SVGs with percentages for width and height result in inconsistent output

[jseppi]

If an SVG buffer with percentage-based width and height attributes is passed to mapnik.fromSVGBytes (or I assume any of the other fromSVG methods), the callback receives no error and a non-null image result.

However, the image result is inconsistent. If the SVG has width="100%" height="100%", we get back an image that, when encoded to PNG, is 1px x 1px.

If the SVG has non-100% values, like width="90%" height="90%", we get back an image, that when encoded to PNG, throws an error: Could not write to empty stream.

Desired behavior: Since it seems like percentage-based width and height values do not really produce a usable image in any case, I think the callback to fromSVGBytes should instead receive a error (maybe like "svg must have pixel-based width and height values") and null result.

---

Example test cases:

test('SVG with 100% width and height', t => {
  const percent = '100%';
  const svgPercentageWidthHeight = Buffer.from(
    `<svg xmlns='http://www.w3.org/2000/svg' width='${percent}' height='${percent}'><rect fill='#f0f' width='24' height='24'/></svg>`
  );

  const options = {
    max_size: 512,
    scale: 1
  };

  mapnik.Image.fromSVGBytes(svgPercentageWidthHeight, options, (err, image) => {
    // These two tests fail, but we would like to have have an error and not get an image
    t.ok(err, 'should have an error');
    t.notOk(image, 'should not have an image'); 

    // The image when encoded to PNG produces a 1px x 1px image
    const buffer = image.encodeSync('png');
    fs.writeFileSync('./100percentage_image.png', buffer);
    t.end();
  });
});

test('SVG with other percentage width and height', t => {
  const percent = '90%';
  const svgPercentageWidthHeight = Buffer.from(
    `<svg xmlns='http://www.w3.org/2000/svg' width='${percent}' height='${percent}'><rect fill='#f0f' width='24' height='24'/></svg>`
  );

  const options = {
    max_size: 512,
    scale: 1
  };

  mapnik.Image.fromSVGBytes(svgPercentageWidthHeight, options, (err, image) => {
    // These two tests fail, but we would like to have have an error and not get an image
    t.ok(err, 'should have an error');
    t.notOk(image, 'should not have an image'); 

    // The image when encoded to PNG throws an error:
    // Error: Could not write to empty stream
    const buffer = image.encodeSync('png');
    fs.writeFileSync('./percentage_image.png', buffer);
    t.end();
  });
});

test('SVG with no width and height', t => {
  const svgNoHeightWidth = Buffer.from(
    "<svg xmlns='http://www.w3.org/2000/svg'><rect fill='#f0f' width='24' height='24'/></svg>"
  );

  const options = {
    max_size: 512,
    scale: 1
  };

  mapnik.Image.fromSVGBytes(svgNoHeightWidth, options, (err, image) => {
    // These tests pass, as expected
    t.ok(err, 'should have an error'); 
    t.equal(
      err.message, 
      'image created from svg must have a width and height greater then zero',
      'has expected message'
    );
    t.notOk(image, 'should not have an image');
    t.end();
  });
});

cc @artemp

[artemp]

@jseppi - looks inconsistent indeed. I'll take a look, thanks.

[springmeyer]

@artemp what is the status of this ticket?

[artemp]

I'm working to address this issue as part of mapnik/mapnik#4225
Throwing an exception when meaningful image dimensions can't be deduced (e.g width and height defined using % and no viewBox attribute) would be reasonable approach but I think we can do better. I'm planning to add "fallback" width and height as optional parameters. When specified these params will allow interpreting % values as relative to them. I'll post detailed explanation once implementation is in place, but from user point of view following SVG

<svg xmlns='http://www.w3.org/2000/svg' width='100%' height='100%'> <!-- NOTE: no "viewBox"-->
<rect fill='#f0f' width='100%' height='100%'/>
</svg>

and

mapnik.Image.fromSVGBytes(svg_bytes, {width:32, height:24} , (err, image) => {
}

will result in 32x24 image

/cc @springmeyer @jseppi

[artemp]

mapnik/mapnik@654a3c1
improved viewport logic [WIP]
/cc @jseppi @springmeyer

[artemp]

Initial implementation : https://github.com/mapnik/mapnik/tree/svg-viewport

svg2png --scale-factor= 4 --width=25 --height=20 // overwrite "width" and  "height" outermost `<svg>` element

Resulting png is 100x80px. I'm still wondering if this logic should only apply to width and height expressed in % (?)
There is also interesting effects when SVG paths are using absolute coordinates..
/cc @springmeyer @jseppi