SVG images with height and/or width being percentages

[ghost]

I was investigating, how GNS3 handles SVG images with height and/or width being percentages. I found commit GNS3/gns3-server@fd5df00, which implements that.

In this commit I found an error. In case the width or height are percentages, it sets width_attr and height_attr, but not width and height. So the get_size function returns the default, normally 0.

I fixed that, but that didn't change anything. The size of SVG symbols stays the same, no matter what get_size() returns. So fixing that doesn't have any impact.

But I found another strange behavior.

Older QT versions (verified with 5.9.2) use the percentage as height/width in pixel. So SVG images with height/width of 100% are 100 pixel high/wide. The viewBox is completely ignored for calculating the image size.

Newer QT versions (tested with 5.11.2) multiply the percent value with the viewBox value. So a height of 20% and a viewBox height of 300 results in a symbol height of 60 pixel.

This has a big impact of users using the affinity symbols https://github.com/ecceman/affinity as shown by David Bobal in https://youtu.be/51zaAHhdgzQ. Currently (with QT 5.9) the symbols are a bit bigger than normal (100x100px), but that's manageable. When QT gets upgraded the image are three times bigger (300x300px), that's really big. And I see no way to solve this.

Just for reference here my patch to calculate the SVG image size as it's done by newer QT versions. As I said it has no impact. GNS3/QT seems to ignore the size returned by get_size(), at least for SVG:

Update: Some additional fine tuning:

• default for missing height/width is "100%" as defined in the SVG specification
• better error message, if viewBox attribute is missing
• removal of "%" in percent more fault tolerant by using rstrip("%")

diff --git a/gns3server/utils/picture.py b/gns3server/utils/picture.py
index d7c3103b..a4574952 100644
--- a/gns3server/utils/picture.py
+++ b/gns3server/utils/picture.py
@@ -16,6 +16,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import io
+import re
 import struct
 from xml.etree.ElementTree import ElementTree, ParseError
 
@@ -103,13 +104,21 @@ def get_size(data, default_width=0, default_height=0):
         root = tree.getroot()
 
         try:
-            width_attr = root.attrib.get("width", "0")
-            height_attr = root.attrib.get("height", "0")
+            width_attr = root.attrib.get("width", "100%")
+            height_attr = root.attrib.get("height", "100%")
             if width_attr.endswith("%") or height_attr.endswith("%"):
                 # check to viewBox attribute if width or height value is a percentage
-                _, _, width_attr, height_attr = root.attrib.get("viewBox").split()
+                viewbox = root.attrib.get("viewBox")
+                if not viewbox:
+                    raise ValueError("Invalid SVG file: missing viewBox attribute")
+                _, _, viewbox_width, viewbox_height = re.split(r'[\s,]+', viewbox)
+            if width_attr.endswith("%"):
+                width = _svg_convert_size(viewbox_width, width_attr)
             else:
                 width = _svg_convert_size(width_attr)
+            if height_attr.endswith("%"):
+                height = _svg_convert_size(viewbox_height, height_attr)
+            else:
                 height = _svg_convert_size(height_attr)
         except (AttributeError, IndexError) as e:
             raise ValueError("Invalid SVG file: {}".format(e))
@@ -117,11 +126,12 @@ def get_size(data, default_width=0, default_height=0):
     return width, height, filetype
 
 
-def _svg_convert_size(size):
+def _svg_convert_size(size, percent=None):
     """
     Convert svg size to the px version
 
     :param size: String with the size
+    :param percent: String with the percentage, None = 100%
     """
 
     # https://www.w3.org/TR/SVG/coords.html#Units
@@ -133,8 +143,12 @@ def _svg_convert_size(size):
         "in": 90,
         "px": 1
     }
-    if len(size) > 3:
+    factor = 1.0
+    if len(size) >= 3:
         if size[-2:] in conversion_table:
-            return round(float(size[:-2]) * conversion_table[size[-2:]])
+            factor = conversion_table[size[-2:]]
+            size = size[:-2]
+    if percent:
+        factor *= float(percent.rstrip("%")) / 100.0
 
-    return round(float(size))
+    return round(float(size) * factor)

I'm using re.split(), because the SVG specification allows whitespace and/or a comma as separators.

[ghost]

Just tested, in which QT version the SVG size calculation changed.

Up to (and including) QT 5.10.1 it uses the percentage as height/width in pixel.
Starting with QT 5.11.1 it multiplies the percent value with the viewBox value.

As I'm using the QT embedded in the PyQT5 PyPI package I can't (easily) test the QT version 5.11.0. But I'm pretty confident, that the change happens in v5.11.1, see this bug report:
https://bugreports.qt.io/browse/QTBUG-2279

[ghost]

Currently I see two possibilities to deal with that:

a) It's possible to tell QT, what image size we want, without too much changes in GNS3
Then the different types of the QT size calculation don't matter. We tell QT, that we want the size returned by get_size(). If QT assumes a different "natural" size, it has to scale.
Furthermore if a symbol exceeds a certain max height (for example 80px), we can scale the height/width to a reasonable value.

b) It's not easy to tell QT our desired size
Then we can easily change the SVG data:
If the viewBox attribute is not set, we must add it (viewBox="0 0 <width> <height>".
Then we can change the height and width attributes to our needs.
That can be used also for the other image formats, as they internally are converted to SVG.

Here a small script, that I created to resize SVGs: svg_resize

[grossmj]

I initially wanted to tell Qt to scale the QGraphicsSvgItem corresponding to nodes when the SVG symbol is above 80px by using something like this:

relative_size = min(80, 80)
self.setScale(relative_size / self.boundingRect().height())

It worked excepting that the node label was resized too, this resulted in a really small label. I tried to block the scaling for the label but this created other drawing issues, for instance when hovering the node.

I finally opted to take part of your code (thanks!) to resize the SVG data directly from the QSvgRender. I tested with both Qt 5.9.1 and Qt 5.11.2, everything seems to work well.

Only thing left is I think the 80px limit should be user configurable. I will probable back-port this to the 2.1 branch once everything is in order.

[ghost]

Looks good, just two comments:

• I think it's enough to resize, when the height is above some limit. For example the standard symbol cloud.svg is wider than 80 px. Furthermore calling renderer.resize(80) would not limit the width. In case of cloud.svg it would make it only a bit higher (from 71px to 80 px) and even wider.
• My resize script works with the binary data, because get_size() needs binary for image types != SVG and I don't want to change that mode later in the script. But in your case the self._svg data is from type str, so it should be easier just to stay with string types instead of bytes in renderer.resize().

[grossmj]

Only thing left is I think the 80px limit should be user configurable. I will probable back-port this to the 2.1 branch once everything is in order.

Should we have a setting to limit node symbols to a max height/width of 80px or should we have one to resize all node symbols to a user preference?

[grossmj]

I think it's enough to resize, when the height is above some limit. For example the standard symbol cloud.svg is wider than 80 px. Furthermore calling renderer.resize(80) would not limit the width. In case of cloud.svg it would make it only a bit higher (from 71px to 80 px) and even wider.

This is correct, we need to handle the width too.

My resize script works with the binary data, because get_size() needs binary for image types != SVG and I don't want to change that mode later in the script. But in your case the self._svg data is from type str, so it should be easier just to stay with string types instead of bytes in renderer.resize().

Indeed, I thought about it but was a bit lazy ;)

[ghost]

Should we have a setting to limit node symbols to a max height/width of 80px or should we have one to resize all node symbols to a user preference?

I think, that resizing to a one height/width might be not the best choice. For example the standard symbols all have different sizes, because that way they are creating a consistent look. Limiting is also not nice, it's more a guard against too big symbols.

The best way would be, when the user can scale each non-standard symbols individually to the size, that looks best. But that would require a new symbol importer/resizer dialog, what's a bit too much. GNS3 is a network simulator, not a network diagram program.

So I would just implement a limitation of the height to guard against too big symbols, maybe with an option to disable that limit.

[ghost]

Looks good for me. Close it?

[grossmj]

Sounds good however still wondering if I should take the risk to back-port this to the 2.1 branch. We will release version 2.1.12 this week. I don't think we should do it because this is not a critical fix, just a nice to have feature. Any thoughts?

[ghost]

Normally I would agree, this is 2.2 stuff. The only reason I'm also unsure is, that users using the affinity symbols (promoted by David Bombal) will get really big symbols, if a v2.1 version will use QT 5.12.

[grossmj]

Actually we have plan to use the latest Qt 5.12 LTS #2636 when PyQt 5.12 is released on https://pypi.org/project/PyQt5/

Therefore it makes a lot of sense to do it now. There is still the option to disable to limit in case something goes wrong ;)

[grossmj]

Implemented in 2.1 too now. Thanks for everything 👍