stylesheet jsName vs svgName fix

[jhlggit]

Related issue: #XXXX

Description

In the SVG Loader, there's some code to read styles from embedded style sheets, which are held in a collection called stylesheetStyles. This collection was being indexed into using svgName, but the styles in an embedded style sheet are actually identified by the slightly different jsName so, where these two differed, the styles were not being picked up.

The fix is a one-line change, in which stylesheetStyles[ svgName ] is replaced with stylesheetStyles[ jsName ] (two occurrences).

For example, here is a style node from an SVG file:

<style>.highlightBox { fill: #ffa500; fill-opacity: .2; } .highlightBoxReferent { fill: #9ff3db; fill-opacity: .3; }</style>

Elsewhere in the file, there are elements with this attribute
class="highlightBox"

With the old code, these elements were rendered as fully opaque but, with this fix, they are now rendered with the specified opacity of .2.

The likely reason this wasn't noticed before is that not all styles are affected. In the example above, the fill colour was not affected because, for fill colour, the svgName and the jsName are the same (namely, "fill").

SVG file referenced in this example:
LogicEssence1_TimesNewRomanPSMT_TimesNewRomanPSMT.xml

screenshot with old code:

screenshot with fixed code:

[Mugen87]

Not sure this bit is correct. The test SVG style-css-inside-defs.svg in the webgl_loader_svg example now renders incorrectly with this change. The red border becomes too wide.

You can test by comparing Prod with this PR:

https://threejs.org/examples/#webgl_loader_svg
https://rawcdn.githack.com/jhlggit/three.js/StyleSheetNameFixBranch/examples/index.html#webgl_loader_svg

If you open the SVG directly in Chrome or via mac Preview, it renders like in the Prod version.

[jhlggit]

Thank you for the feedback. You're right - the change that fixes my issue does seem to cause a regression with stroke-width - sorry I didn't pick that up. It may take me some time to find a better place to make the change. In the meantime, please excuse my hesitation; this is my first foray into contributing to an open-source project: should I close this pull-request and open a new one when I have a better fix, or is there a way I can make a modification within this pull request - and, in any case, should I also formally log an issue to describe the original problem with fill-opacity?

[Mugen87]

please excuse my hesitation;

Don't worry about it. Let's keep this PR open and update it with your new solution. As long as the PR is open, we need no separate issue. If we don't manage to fix the issue now, we can close the PR and open an issue instead for tracking purposes.

[jhlggit]

OK; I've prepared a new solution. However, it does include a change to the test data file, models/svg/style-css-inside-defs.svg . Specifically, I've added a second star, which overlaps the first, and a fractional fill-opacity, so that you can see easily (from the overlap) whether the fractional opacity is being applied. I suspect that, if I include that change in the pull request, it will automatically fail testing, because, if I understand the doco, some of the testing includes pixel-to-pixel comparisons of the output. So, perhaps I ought not to upload the pull request until I've also updated the test case. Could you advise me on how to do that?

[Mugen87]

You must regenerate the E2E screenshot if you change the visual appearance of an example. However, since the mentioned test file is not loaded when opening webgl_loader_svg, everything should be fine. So you can just update the PR with your changes and the CI shouldn't be affected.