SVG visibility support

General JUCE discussion
1

I have some SVGs generated with Inkscape where some elements have been hidden using the following attribute

display="none"

I have noticed that when those SVGs are loaded with JUCE, those elements aren't hidden as expected.

Would it be possible to add support for this to the current SVG parser?

Thanks in advance

2

Sounds like a good request - do you know if all elements support this, or just some of them?

3

https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/display

4

In the mean time, I have hacked my own quick-and-dirty solution this way in juce_SVGParser.cpp, see if it can be of any help/inspiration

    void parseSubElements (const XmlPath& xml, DrawableComposite& parentDrawable)
    {
        forEachXmlChildElement (*xml, e)
        {
            Drawable* drawable = parseSubElement (xml.getChild (e));
            if (drawable == nullptr)
                continue;
            drawable->setVisible (e->getStringAttribute ("display") != "none");
            parentDrawable.addChildComponent (drawable);
        }
    }
5

Thanks, will take a look!

BTW, I'd really recommend that you get into the habit of using the "if (Object* x = ..." pattern to check for nullptr, e.g.


        forEachXmlChildElement (*xml, e)
        {
            if (Drawable* drawable = parseSubElement (xml.getChild (e)))

            {
                drawable->setVisible (e->getStringAttribute ("display") != "none");
                parentDrawable.addChildComponent (drawable);

            }
        }

We push this pattern really hard in our internal coding standards, because it makes it impossible to dereference a null pointer.

6

Ok.. I don't have an example file to test, but presumably this simple change would work?

 

    static void setDrawableID (Drawable& d, const XmlPath& xml)
    {
        String compID (xml->getStringAttribute ("id"));
        d.setName (compID);
        d.setComponentID (compID);

        if (xml->getStringAttribute ("display") == "none")
            d.setVisible (false);
    }
7

that could do, but.. out of curiosity, why not leave it in parseSubElements()?

8

Just seemed more sensible to put it in that method (which I'd probably choose to rename as something like "addCommonAttributes" if this change was made)

9

fair enough, although I would name it setCommonAttributes(). After all, they all are set..() operations of aspects that are already there :)

Anyway, I'm ok with the change, I can't see anything wrong with it