Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve SVGLoader style support #19146

Merged
merged 5 commits into from
Apr 20, 2020
Merged

Improve SVGLoader style support #19146

merged 5 commits into from
Apr 20, 2020

Conversation

mjurczyk
Copy link
Contributor

@mjurczyk mjurczyk commented Apr 16, 2020

Changes

  • Add support for <style> tag and class / id attributes in SVG files.
  • Add global opacity support for Shapes.
  • Silence console.log for unrecognised tags.

Currently SVGLoader does not work nicely with Adobe Illustrator exported SVGs, in some cases.
It is valid for SVGs to include DOM-like <style> definitions, linked via classes:

Example

<?xml version="1.0" encoding="utf-8"?>
<!-- Generator: Adobe Illustrator 23.0.3, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
<!ENTITY ns_extend "http://ns.adobe.com/Extensibility/1.0/">
<!ENTITY ns_ai "http://ns.adobe.com/AdobeIllustrator/10.0/">
<!ENTITY ns_graphs "http://ns.adobe.com/Graphs/1.0/">
<!ENTITY ns_vars "http://ns.adobe.com/Variables/1.0/">
<!ENTITY ns_imrep "http://ns.adobe.com/ImageReplacement/1.0/">
<!ENTITY ns_sfw "http://ns.adobe.com/SaveForWeb/1.0/">
<!ENTITY ns_custom "http://ns.adobe.com/GenericCustomNamespace/1.0/">
<!ENTITY ns_adobe_xpath "http://ns.adobe.com/XPath/1.0/">
]>
<svg version="1.1" id="Layer_1" xmlns:x="&ns_extend;" xmlns:i="&ns_ai;" xmlns:graph="&ns_graphs;" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px" viewBox="0 0 100 100" style="enable-background:new 0 0 100 100;" xml:space="preserve">
  <style type="text/css">
    .st0{fill:#00ff00;opacity:0.2;}
  </style>
  <switch>
    <foreignObject requiredExtensions="&ns_ai;" x="0" y="0" width="1" height="1">
      <i:pgfRef xlink:href="#adobe_illustrator_pgf"></i:pgfRef>
    </foreignObject>
    <g i:extraneous="self">
      <g>
        <polygon fill="#ff0000" class="st0" points="73.5,64.1 46.76,50.71 46.76,27.31 73.5,40.7"/>
      </g>
    </g>
  </switch>
</svg>

Current vs corrected preview:

preview

Corrected version reads additional styles from stylesheets using CSSStyleSheet. Also, silencing console.log for unrecognised tags does not clutter console with billions of exported third-party tags.

Limitations / Not added in this PR

  • Nested CSS selectors (ex. container child { fill: #ff0000; })
  • Support for all CSS Interfaces. Only type-1, direct stylesheet added, imports / keyframes / media queries not supported.

Browser Support

Yes. CSSRule , CSSStyleSheet + .cssRules

add "opacity" support
@mrdoob mrdoob requested a review from yomboprime April 16, 2020 08:20
@mrdoob mrdoob added this to the r116 milestone Apr 16, 2020
@yomboprime
Copy link
Collaborator

Add support for <style> tag and class / id attributes in SVG files.

This is good code, I approve it. 👍

Add global opacity support for Shapes.

As stated in the code comment, there is a reason this is not good to go. Please revert it.

Silence console.log for unrecognised tags.

I guess this is right. Any thoughts @mrdoob ?

@yomboprime
Copy link
Collaborator

Went through SVG attributes to see if there's anything else that may be missing, added visibility. Rest seems unsupported / unsupportable with ShapeBuffers?

Well, perhaps some user will need to parse and use one of them. So it is good that they are accessible as path.userData.node.style.getAttribute(...)

BTW, a correction to myself:: the style object is in path.userData.style, while the original node style is in path.userData.node.style.

Also, is there a reason why we couldn't add support (in another PR)? There's already TextGeometry around. thinking

It is a complex topic, but perhaps you want to give it a go. For me, it is just easier to convert SVG text to paths in the SVG editor. If you want to contribute more features to the SVGLoader, two things that could be more practical are the window size and viewport or viewBox attributes (they should be implemented in the hierarchical Matrix4 transforms).

@mjurczyk mjurczyk requested a review from mrdoob April 17, 2020 00:51
@mrdoob mrdoob merged commit 2c84ae1 into mrdoob:dev Apr 20, 2020
@mrdoob
Copy link
Owner

mrdoob commented Apr 20, 2020

Thanks!

@mjurczyk mjurczyk deleted the update-svg-loader branch April 20, 2020 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants