SVG dimensions get (wrongly) rounded

[Pomax]

I tried using image-size with (autogenerated) SVG files that specify their dimensions in ex units, such as:

<svg width="69.167ex" height="2.667ex" viewBox="0 -904.8 29770.5 1176.8">
...
</svg>

but the dimensions get reported as being width 69 and height 2, with the rather important decimal fraction part removed. For bitmap images this makes sense, but for SVG this means the dimensions are very wrong and cannot be relied upon.

Worth not rounding dims for vector format images?

[mdmoreau]

Running into this same issue when getting SVG dimensions from the width/height and viewbox. For SVGs it seems like you'd definitely not want to round the values at all.

[Pomax]

I've filed #67 to fix this, hopefully @LinusU, @netroy, @shinnn, or @zeke can merge that in soon.

[puzrin]

@Pomax @mdmoreau could you comment SVG use cases? I'm designing SVG support API, and not sure that mixing floats & ints is good idea nodeca/probe-image-size#4.

• usually people expect that dimentions are integer (people who work with pixels don't expect SVG vectors at all)
• not clear what to do with dimentions units

Mixin everything into existing properties can require additional checks after getting result for many cases where SVG is not expected). Solution could be:

1. Place SVG details to additional props (svgWith / svgHeight), and keep default ones integer.
2. Disable SVG by default and enable via options
   • may be option to whitelist desired formats

What do you think?

[Pomax]

If you work with SVG, you will always run into this. Tons of SVG tools like inkscape, Sketch, etc. will throw SVG exports at you with decimal values.

• No they don't, not when it's their job, or even just their choice, to work with SVG =)
• Pretty easy: ignore them. All you need is parseFloat instead of parseInt to get a result people can rely on just fine.

As for the proposed solutions:

1. this sounds like a bad idea, because SVG is just another image format. Its fractional width and height as just as legal as integer width and height for bitmaps.
2. that's even worse, that makes the package even less useful than it is now.

I filed a PR that fixes this problem months ago, just have a look at that.

[puzrin]

I've seen your commit, and asked questions after that.

1. Your PR makes api good for you, but less comfortable for those who don't use SVG (but can have it on input by accident). Because result type can vary. I mean, good api should be convenient for all, and tried to suggest those. May be you have better ideas?
2. In your PR dimentions units data (px/pt/em/ex) is still lost. Looks suspicious & candidate for next bug reports. If we leave units, then result type will vary even worse: integer / float / string. That was a reason why i suggested to control output somehow, instead of force user to check everything.

[mdmoreau]

I think the distinction is what @Pomax mentioned initially - bitmap images will always be integers while SVGs are designed to use integers and/or decimals. Not supporting decimals ignores a pretty core feature of the SVG format in my opinion, especially since rounding will negatively impact most SVGs.

My primary use case involves using the dimensions to build a responsive container for an SVG image. Part of a mustache template I have uses the following snippet to generate some CSS:

.svgstore {
  position: relative;
  pointer-events: none;

  svg {
    position: absolute;
    width: 100%;
    height: 100%;
  }
}

.svgstore--{{ name }} {
  padding-bottom: {{ height }} / {{ width }} * 100%;
}

I then have a nunjucks macro that outputs the code which allows the SVG to scale correctly without needing to have the inline viewbox attributes:

<div class="svgstore svgstore--{{ svg }}">
  <svg>
    <use xlink:href="/img/svgstore.svg#{{ svg }}"></use>
  </svg>
</div>

When the numbers are rounded, this ends up throwing off the padding-bottom value that scales the container, messing up the display of the SVG.

[puzrin]

@mdmoreau I agree, if use-case is "keep aspect ratio", then units are not needed. Need to understand if there are other cases, where lack of units can become a problem.

My case is post-processing images at forum posts, to define width/height and avoid reflows on page load. But we don't care much about SVG insert. It's even better to ignore "not pixels" for our case.

[Pomax]

@puzrin the change does not affect processing of integer values, as parseFloat and parseInt yield the same results on integer inputs (up to 2^53, which is essentially irrelevant because current bitmap formats can't encode images with dimensions of that magnitude, nor are browsers or Node allow the memory necessary to unpack images of that size). The code behaviour of this library will not change as far as bitmap images are concerned with this fix.

As for 2, losing the unit is not going to ruin anyone's day nearly as much as losing the fractions currently does. Losing the unit just means you might have to apply a uniform scale, and as developer you know how to best solve that (in app, CSS, etc). While it might be a bother, you can easily fix it. On the other hand, losing the fractions means you've lost all image aspect integrity, and that's something you cannot fix without uninstalling this library and using something else entirely (like manual XML parsing of the SVG source). So: land it, and you've reduced the current bug's surface to a much, much smaller set of cases, which unlike before do have a workable solution even when users run into them. And then for good process, you file a follow-up issue to investigate whether the loss of units is a genuine problem, while in the mean time you've vastly improved the library for everyone, and maintaining backward compatibility.

[puzrin]

@Pomax if we speak about existing image-size library - no question, your change will not break compatibility. Because you can say that SVG can be detected with invalid units.

If we speak about probe-image-size - it's supposed to give correct result (because have no SVG support and no related problems). But i'm not sure that allowing bad units is good idea.

What do you think about adding property ratio to result? Will it solve your problem?

[Pomax]

I don't use that library, so my main concern is to fix this issue, for this library, in a way that is backward compatible. Given that it is, any library that relies on this one will not be impacted and any additional work that happens specifically in dependents is honestly something to discuss on their repos, not something that should hold up progress in this library.

[puzrin]

SVG support is now available in https://github.com/nodeca/probe-image-size. With floats, units info etc.

[zeke]

That's awesome, @puzrin 👏

You might want to update the GitHub repo description to mention SVG:

[puzrin]

@zeke thanks, updated :) . I hope code quality is high, but still interested in massive feedback.

[Pomax]

nice @puzrin!