Add support for the transform-origin attribute.

[LaurenzV]

See here.

Fixes #429.
Blocked by linebender/svgtypes#18.

I think it should be correct since it's a relatively simple feature, and I also added a bunch of tests. The only thing I didn't consider was the z position, since I've never even seen something like that in this library. Is this something we just don't support yet?

[RazrFalcon]

How transform-origin interact with transform? Can you add a test when both are used?

The only thing I didn't consider was the z position

There is no concept of z-axis in SVG 1 and therefore in resvg. We can ignore it for now. Not sure if we would ever need it.

Also, you have missed tests/structure/transform-origin/on-image.png and haven't updated tests file/list.

Otherwise looks good. But I'm not sure if we handled all possible transform occurrences? Technically, node.attribute(AId::Transform) should not be present anywhere in the code anymore, right? But I guess you already did that.

What about gradientTransform and patternTransform? Are they affected?

transform-origin on text just works? How is that?
Add a test for text on path with transform-origin. See resolve_text_flow in usvg-parser/src/text.rs

[LaurenzV]

How transform-origin interact with transform? Can you add a test when both are used?

Isn't that what most of the tests already do? Or do you mean if there is an additional transform on the parent?

Also, you have missed tests/structure/transform-origin/on-image.png and haven't updated tests file/list.

Ah yeah sorry, I keep forgetting to update it. 😅

Technically, node.attribute(AId::Transform) should not be present anywhere in the code anymore, right? But I guess you already did that.

Yes, I searched for all occurrences. Initially, I had the check for the AId::Transform in the resolve_transform_origin to avoid duplication, but the thing is that for clip paths the logic of getting the transforms is a bit different, so I changed it so that the transform needs to be passed to the resolve_transform_origin explicitly. But I will think about it again.

What about gradientTransform and patternTransform? Are they affected?

True, I didn't consider those. 😅 I guess it's not as straight-forward as I thought. Will try to look into what should happen here and add tests.

transform-origin on text just works? How is that?

Seems like it, but I'll check again why it works.

Add a test for text on path with transform-origin. See resolve_text_flow in usvg-parser/src/text.rs

Will do that.

[RazrFalcon]

Isn't that what most of the tests already do?

Oh, I see. I haven't looked into the files itself. What if we have transform-origin and no transform?

I guess it's not as straight-forward as I thought.

SVG tricked you again. It likes to do it.

[LaurenzV]

transform-origin on text just works? How is that?

The reason it works for texts (and images) as well is that transforms are resolved in the convert_element method, which will be called for any of paths/images/texts:

https://github.com/RazrFalcon/resvg/blob/23d689d92c851704fd3ace16f5b8484371df41a6/crates/usvg-parser/src/converter.rs#L338-L403

[LaurenzV]

Regarding pattern and gradient transforms, I mean it isn't mentioned anywhere explicitly how this should be handled, but given that here it mentions:

SVG 2 defines the transform, patternTransform, gradientTransform attributes as presentation attributes, represented by the CSS transform property [SVG2].

It does feel like pattern and gradient transforms should be treated equivalently with normal transforms, so I will add support for those as well.

[LaurenzV]

So... I'm having a bit of trouble testing the transform-origin on gradients/patterns with objectBoundingBox, and it seems to be because I don't understand how transforms work in that context... Consider the following SVG:

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">

    <radialGradient id="rg1" spreadMethod="pad" gradientTransform="translate(0.5 0.5)">
        <stop offset="0" stop-color="black"/>
        <stop offset="0.3" stop-color="green"/>
    </radialGradient>

    <rect id="rect1" x="20" y="20" width="160" height="160" fill="url(#rg1)"/>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">

As I would expect, the center of the gradient appears in the bottom right corner, because we translate it by 0.5 0.5 in object bounding box units. However, if I try the exact same on patterns:

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">
    <pattern id="patt1" patternUnits="objectBoundingBox" patternContentUnits="objectBoundingBox" patternTransform="translate(0.5 0.5)" width="1" height="1">
        <rect id="rect1" x="0" y="0" width="0.5" height="0.5" fill="grey"/>
        <rect id="rect2" x="0.5" y="0.5" width="0.5" height="0.5" fill="green"/>
    </pattern>
    <rect id="rect3" x="20" y="20" width="160" height="160"
          fill="url(#patt1)" stroke="darkblue"/>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

It barely gets shifted at all, i.e. it seems that the transform is applied in user space units. What am I missing here? From what I can tell they should behave exactly the same:
https://www.w3.org/TR/SVG11/pservers.html#PatternElementPatternTransformAttribute
https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementGradientTransformAttribute

[LaurenzV]

I implemented the changes you proposed, but I'm still confused about the gradient/pattern thing, and depending on what you think I might have to make some more changes to it.

[RazrFalcon]

Honestly, no idea what's wrong with patterns. Looks at prepare_pattern_pixmap in resvg. It's a convoluted mess.
For the sake of the test I would suggest using userSpaceOnUse, aka:

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">
    <pattern id="patt1" width="80" height="80" patternUnits="userSpaceOnUse" 
        patternContentUnits="userSpaceOnUse" patternTransform="translate(20)">
        <rect id="rect1" x="0" y="0" width="40" height="40" fill="grey"/>
        <rect id="rect2" x="40" y="40" width="40" height="40" fill="green"/>
    </pattern>
    <rect id="rect3" x="20" y="20" width="160" height="160"
          fill="url(#patt1)" stroke="darkblue"/>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

[RazrFalcon]

As for on-gradient-object-bounding-box.svg test, are you sure it's correct? For one, transform-origin="center" should be no-op to begin with, but resvg pushes gradient to the center. No other SVG library does this. Are you sure this is an expected output? Also, prefer translate to scale. It would be easier to reason about.

Based on my experiments, transform-origin should not affect gradientTransform and patternTransform at all. Or all other implementations do not support it. Either way we should ignore it as well for now.
What SVG library/app supports transform-origin to begin with? Which one are you using as a reference?

And we do not need dedicated objectBoundingBox test as well then. All we have to test for is that transform-origin doesn't actually affect gradientTransform.
As well as that transform-origin has no effect without transform in general.

Also, I cannot find it in the spec, but MDN says that:

Note: The default value of transform-origin is 0 0 for all SVG elements except for root elements and elements that are a direct child of a foreignObject, and whose transform-origin is 50% 50%, like other CSS elements.

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

Are you using 50% 50% right now or 0 0?

Once again, the hardest part about SVG is to actually figure out what you're suppose to do. Not the implementation itself.
And I have to basically implement this feature myself to have any meaningful input. SVG is too complicated.
For now, just make sure resvg produces the same output as Chrome.

[LaurenzV]

As for on-gradient-object-bounding-box.svg test, are you sure it's correct? No other SVG library does this. Are you sure this is an expected output?

This is the only one I'm not sure about, precisely because of the fact that I don't quite understand how transform is supposed to interact with the gradientUnits parameter and because of the fact that Chrome renders it differently. But in this case, I will revert the change I made to make it consistent with Chrome as well as with the way how we handle patterns.

For one, transform-origin="center" should be no-op to begin with, but resvg pushes gradient to the center.

Not sure what you mean, why would this be a no-op? Consider the following SVG:

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">
    <title>on `gradient` with `gradientUnits` `userSpaceOnUse`</title>

    <radialGradient id="rg1" spreadMethod="pad" gradientUnits="userSpaceOnUse" gradientTransform="scale(1.5)">
        <stop offset="0" stop-color="black"/>
        <stop offset="0.3" stop-color="green"/>
    </radialGradient>
    <rect id="rect1" x="20" y="20" width="160" height="160" fill="url(#rg1)"/>
</svg>

This is how Chrome renders it:

Now if you add a transform-origin = "center", it will look like this. I will basically scale it from the center instead of scaling it from the top left:

Based on my experiments, transform-origin should not affect gradientTransform and patternTransform at all. Or all other implementations do not support it. Either way we should ignore it as well for now.
What SVG library/app supports transform-origin to begin with? Which one are you using as a reference?

As mentioned above, Google Chrome supports it. All of our reference images (except the gradient-object-bounding-box one, which I will fix later) match the output of Google Chrome. But it does seem like Inkscape, Firefox and Safari don't handle this case. I can't tell for sure who is correct, but my understanding of the specification is that gradientTransform and patternTransform should be treated equally to normal transforms, so it would make sense.

Also, prefer translate to scale. It would be easier to reason about.

That's the problem though, if you just use translations, the transform-origin will have no effect at all. Let's say my transform is translate(10 10) and my transform-origin is (100 100). What will essentially happen is that we calculate the transformation translate(100 100) translate(10 10) translate(-100 -100), which is just the same as doing translate(10 10), so it will have no effect. However, if we have the transform scale(2), then translate(100 100) scale(2) translate(-100 -100) will not have the same effect as just doing scale(2). As can be seen in the pictures above.

All we have to test for is that transform-origin doesn't actually affect gradientTransform.

I will remove the objectBoundingBox tests.

As well as that transform-origin has no effect without transform in general.

I added a test no-transform.svg for that.

Are you using 50% 50% right now or 0 0?

We are using 0 0. This basically just means that if the transform-origin is not defined at all, all transformations happen from the "top-left" corner, which is what happens by default, even before this PR. However, if I understand this part correctly, then if the transform is in the root element, then it should happen in the center, which currently doesn't seem to be happening. For example:

<svg id="svg1" transform="rotate(45)" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">
    <title>Simple case</title>

    <!-- should be covered -->
    <path id="path1" d="M 20 20 L 20 180 L 180 180 L 180 20" fill="red"/>

    <rect id="rect1" x="20" y="20" width="160" height="160" fill="green"/>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

Output in Chrome/Firefox:

Output in resvg:

I will fix this as well.

[RazrFalcon]

Oh boy...

That's the problem though, if you just use translations, the transform-origin will have no effect at all.

I see now. I was testing only translations and was confused that it had no effect.

But in this case, I will revert the change I made to make it consistent with Chrome as well as with the way how we handle patterns.

Yes, treat Chrome as a reference implementation unless you're 100% sure it's wrong, which is rare.

Not sure what you mean, why would this be a no-op?

My bad again. I thought transform-origin="center" is the default value. But it's default in CSS, not SVG.

I will remove the objectBoundingBox tests.

No need to. If objectBoundingBox does affect the output than we have to test for it as well. Preferably have two tests: one for objectBoundingBox and one for userSpaceOnUse.

However, if I understand this part correctly, then if the transform is in the root element, then it should happen in the center, which currently doesn't seem to be happening.

The root element is special. It affects browsers, but not resvg. This is because browsers embed SVG into the page content, but resvg renders SVG as a stand-alone image. So it's hard to tell what is the correct output here.
Also note that when an SVG has no explicit width/height (excluding viewBox), browsers will center such SVG. But resvg doesn't do this, because we do not have a concept of a parent viewport.

Overall, while I'm still a bit confused, I would suggest having as many tests as possible and try to match Chrome output. Except the root element. This case can be ignored for now and/or implemented as a separate patch later.

As for how objectBoundingBox works... Well, I still have no clear understanding either. It's a very confusing SVG feature.
In short, we're creating a transform like matrix(width, 0, 0, height, x, y) using object's bounding box.
So it's element transform + bbox transform + gradient transform. In that order (afaik). Remember that transforms order is important.
Therefore I would assume that transform-origin should be applied to gradientTransform during rendering and not parsing. Because during parsing we don't know object's bbox.
Aka element transform + bbox transform + transform-origin + gradient transform - transform-origin ?

The spec does kinda mention this:

For the SVG pattern element, the reference box gets defined by the patternUnits attribute [SVG2].
For the SVG linearGradient and radialGradient elements, the reference box gets defined by the gradientUnits attribute [SVG2].
For the SVG clipPath element, the reference box gets defined by the clipPathUnits attribute [CSS-MASKING].

Therefore we're technically correct. But it also mentions:

Uses the nearest SVG viewport as reference box.

Which is not always the image size, but we do handle it already via State::view_box in convert_length. Hm...

It feels like resolve_transform_origin can be used only for userSpaceOnUse.
Does usage of only userSpaceOnUse in resolve_transform_origin fixes gradients?
If not, we could try to preserve transform-origin for gradients and patterns and apply it only during rendering. Aka add BaseGradient::transform_origin field. Maybe it would work.
Otherwise I would have to pull this patch and figure it out myself...

No more ideas for now.

PS: Not gonna lie, I've spend an absurd amount of time figuring out even the current transforms code. It's really really hard. So do no blame yourself. It suppose to be that confusing.

PSS: No need for spreadMethod="pad" in tests, since this is the default value.

[LaurenzV]

No need to. If objectBoundingBox does affect the output than we have to test for it as well. Preferably have two tests: one for objectBoundingBox and one for userSpaceOnUse.

Okay, I'll keep that then.

Does usage of only userSpaceOnUse in resolve_transform_origin fixes gradients?

If by fix you mean that it matches the output of Chrome, then yes. So I guess it's best to go back to that behaviour.

[LaurenzV]

Alright, I changed the gradient ObjectBoundingBox from "center" to "0.5 0.5", so that you can still see it.

I double-checked, all tests match the output of Google Chrome now.

[LaurenzV]

Done.

I also applied two clippy lints which I forgot in the PR for text-decoration (first one is using Some instead of |n| Some(n) and the second one was a parameter which I forgot to delete, which was only used because we recursively passed it on to the same function, but it was never actually used). Hope that's okay. If not, I'll reverse the clippy lint one.

[RazrFalcon]

Thanks!