fskpf/svg2roughjs

Styling may be wrong on referenced elements

fskpf opened this issue · 5 comments

fskpf commented

Repro

<svg xmlns="http://www.w3.org/2000/svg" 
  xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 300 300" width="300" height="300">
  <g stroke-width="38" stroke="#000">
    <g id="b" transform="translate(150 150)">
      <path id="a" fill="#ffb13b" d="M-84.1-15.9a22.4 22.4 0 100 31.8H84a22.4 22.4 0 100-31.8z"/>
    </g>
  </g>
  <use xlink:href="#b"/>
</svg>

Currently, when a reference element is found (here it is <use xlink:href="#b"/>) we obtain the element with the respective id. When this element is drawn, we traverse the parent hierarchy of this element to determine its style config.

However, when the element is referenced, the parent hierarchy is another as the hierarchy it is defined in. Thus, the referenced elements always get the style configuration of the hierarchy where they have been defined.

ygra commented

In that case we must use attributes and styles of the use element and can only use the attributes and styles of the referenced element as fallback, I think. Since use basically establishes a hidden DOM that does not interact with the rest in any way. But perhaps we should consult the spec for that ;)

fskpf commented

This is quite nasty, particularly with nested use elements. I've implemented a nested useElementCtx stack that is considered when parsing the style config (see the use-element-styling branch).

This seems to almost work for the original test-svg, e.g.

<svg xmlns="http://www.w3.org/2000/svg" 
  xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 300 300" width="300" height="300">
  <g stroke-width="38" stroke="#000">
    <g id="b" transform="translate(150 150)">
      <path id="a" fill="#ffb13b" d="M-84.1-15.9a22.4 22.4 0 100 31.8H84a22.4 22.4 0 100-31.8z"/>
      <use xlink:href="#a" transform="rotate(45)"/>
      <use xlink:href="#a" transform="rotate(90)"/>
      <use xlink:href="#a" transform="rotate(135)"/>
    </g>
  </g>
  <g stroke-width="5" stroke="red">
    <use xlink:href="#b"/>
  </g>
</svg>

The problem is now the stroke-width scaling (removing it makes the above example work):

      // If we have a transform and an explicit stroke, include the scaling factor
      if (svgTransform && stroke !== 'none') {
        // For lack of a better option here, just use the mean of x and y scaling factors
        const factor = (svgTransform.matrix.a + svgTransform.matrix.d) / 2
        strokeWidth *= factor
      }

Why do we need this again and why is it bound to the x/y of the transform?

ygra commented

It's bound to M11/M22, i.e. the scaling factors, to at least include some semblance of stroke scaling which would otherwise be lost. It's an approximation, though, and seemingly goes wrong with rotation.

Yeah, that scaling code is broken, for a 45° rotation we get a scaling of 0.7, although we should of course still have a scaling of 1. But there's not really a way of figuring that out, is there?

We could perhaps use the determinant instead;

    // For lack of a better option here, use the determinant
    const m = svgTransform.matrix
    const det = m.a * m.d - m.c * m.b
    const factor = Math.sqrt(det)
    strokeWidth *= factor
fskpf commented

The determinante!!! 🤓 This looks much better! Played around with some test files and it definitely works better for rotations than the previous solution, e.g.

<svg xmlns="http://www.w3.org/2000/svg" width="300" height="800" version="1.1">
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(20, 20)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(100, 20) rotate(45)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(200, 20) rotate(90)"></rect>
  <!-- scaled up -->
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(20, 80) scale(2)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(100, 80) rotate(45) scale(2)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(200, 80) rotate(90) scale(2)"></rect>
  <!-- scaled down -->
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(20, 250) scale(0.5)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(100, 250) rotate(45) scale(0.5)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(200, 250) rotate(90) scale(0.5)"></rect>
  <!-- scaled up non-uniform -->
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(20, 350) scale(2, 3)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(100, 350) rotate(45) scale(2, 3)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(200, 350) rotate(90) scale(2, 3)"></rect>
  <!-- scaled down non-uniform -->
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(20, 550) scale(0.5, 0.2)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(100, 550) rotate(45) scale(0.5, 0.2)"></rect>
  <rect width="30" height="50" stroke="red" stroke-width="5" transform="translate(200, 550) rotate(90) scale(0.5, 0.2)"></rect>
</svg>
ygra commented

Yeah, figured that out on Friday, only ;)

Given the constraints of our current rendering model, we probably can't do much better unless we tesselate and render the strokes ourselves. Non-uniform scales won't work, but at least uniform scaling is now independent of rotation.