scaleToWidth() ignores viewBox of SVG images

[YMS444]

Hi,

The scaleToWidth() function seems to ignore the viewBox attribute when handling SVG images.

Modifying the viewBox or other properties in the SVG images is not an option for us, as the images are under the control of our customers, which they again receive from their suppliers.

Thanks a lot for having a look on this!

Best regards,
Yannick

Version

Any since 2.0.0
(we're using 3.0.0 productively, the CodePen below uses 4.0.0-beta5)

In FabricJS 1.7.19 this was working as I would expect it, but not any more since at least 2.0.0-beta3.

Test Case

https://codepen.io/YMS444/pen/poJJmMj

There are two SVG images, both visibly filling only a part of the viewBox (for the yellow one: it's a 60x60 square centered in a 100x100 viewBox). If I just apply them to a canvas, they are correctly displayed with some according border around them (e.g. 20px on all sides). But if I apply scaleToWidth(), while the top/left border remains, FabricJS ignores the bottom/right border and fills the whole rest of the canvas with the square.

See comments in CSS section for what is to be seen where.

Information about environment

Tested in Firefox 72 and Chrome 79 on Windows 10.

Steps to reproduce

Have an SVG image with a viewBox being bigger than the actually filled content, e.g. (simplified):

Add it to a canvas using scaleToWidth(). Neither the "absolute" parameter nor whether add().renderAll() or setBackgroundImage() is used matters.

Expected Behavior

The viewBox would be scaled to the width of the canvas, resulting in the rectangle still only filling the center part of the canvas.

Actual Behavior

The rectangle is scaled to the width of the canvas.
(Top/left empty area remains, resulting in the bottom/right part of the rectangle no longer shown, and the bottom/right empty area disappearing completely.)

[asturur]

i think your expectation are just unmet.
This is more or less what happen, let's talk of the yellow rect in the blu div that is the easiest.

there is an svg that has a size of 100x100. a rect is 60x60 is placed at 20,20.

fabric parse the svg, goes through all the objects and what it find is a rect 60 by 60, positioned at 20,20.
it forgets about the 100 by 100 svg. by design, and gives you just the yellow rect.

If there would have been 2 objects or more, fabric would have created a group, and to respect SVG look alike it would have kept the objects relative to the viewbox, giving what you want

https://codepen.io/asturur/pen/ZEGWYqx

Now i understand here is a matter of opinions.
you can totally open a PR, adding an option to the groupSvgElements function, asking for the viewbox to be respected. This option would override the code found here in order to always create a group.

fabric.js/src/util/misc.js

Line 500 in d3c4959

groupSVGElements: function(elements, options, path) {

what is misbehaving in your case is that early if that return the first element in case there is only one.

[YMS444]

Hi Andrea,

Thanks for your comment. Based on the fact given by you that this behaviour only occurs with only one element in the viewbox, we now work around it by simply adding an invisible second element in that case. So we are fine with what we currently have and closing the issue without any further action would be okay for us.

However, even after your explanation, we still see that behaviour as unexpected (even more so as it was as we would have expected it in FabricJS 1.7). So I still would recommend reconsidering changing the behaviour by default, or adding the option you proposed.

Best regards,
Yannick

[asturur]

Well indeed i suggested you can open a simple PR to make that happen.
Is just an extra argument or an extra key in the provided options, and a switch.