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

Fix issue with relative units in symbols #832

Merged
merged 6 commits into from
Oct 14, 2024

Conversation

LaurenzV
Copy link
Contributor

Fixes #829

I'm not a 100% sure about this fix, so some scrutiny would be appreciated. With that said, I think there were two things going on:

  • When resolving relative sizes (like percentages), we only use the width/height of the viewbox, but I think when using a use node, we should use the width/height of that as the reference, if available.
  • We did not set the use_size attribute for use nodes linked to symbols.

Let me know what you think.

Given the following SVG:

<svg version="1.1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">
    <title>With size on `use` and relative units</title>
    <symbol id="image1">
        <image width="100%" height="100%"
               href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAMgAAADICAYAAACtWK6eAAAFKUlEQVR4Xu3VsRWDQAwFQa5/6nWKGzAONv1DrkCj28d5Pvdz+QgQ+ClwBOJlEHgXEIjXQeCPgEA8DwIC8QYINAF/kOZmakRAICOHtmYTEEhzMzUiIJCRQ1uzCQikuZkaERDIyKGt2QQE0txMjQgIZOTQ1mwCAmlupkYEBDJyaGs2AYE0N1MjAgIZObQ1m4BAmpupEQGBjBzamk1AIM3N1IiAQEYObc0mIJDmZmpEQCAjh7ZmExBIczM1IiCQkUNbswkIpLmZGhEQyMihrdkEBNLcTI0ICGTk0NZsAgJpbqZGBAQycmhrNgGBNDdTIwICGTm0NZuAQJqbqREBgYwc2ppNQCDNzdSIgEBGDm3NJiCQ5mZqREAgI4e2ZhMQSHMzNSIgkJFDW7MJCKS5mRoREMjIoa3ZBATS3EyNCAhk5NDWbAICaW6mRgQEMnJoazYBgTQ3UyMCAhk5tDWbgECam6kRAYGMHNqaTUAgzc3UiIBARg5tzSYgkOZmakRAICOHtmYTEEhzMzUiIJCRQ1uzCQikuZkaERDIyKGt2QQE0txMjQgIZOTQ1mwCAmlupkYEBDJyaGs2AYE0N1MjAgIZObQ1m4BAmpupEQGBjBzamk1AIM3N1IiAQEYObc0mIJDmZmpEQCAjh7ZmExBIczM1IiCQkUNbswkIpLmZGhEQyMihrdkEBNLcTI0ICGTk0NZsAgJpbqZGBAQycmhrNgGBNDdTIwICGTm0NZuAQJqbqREBgYwc2ppNQCDNzdSIgEBGDm3NJiCQ5mZqREAgI4e2ZhMQSHMzNSIgkJFDW7MJCKS5mRoREMjIoa3ZBATS3EyNCAhk5NDWbAICaW6mRgQEMnJoazYBgTQ3UyMCAhk5tDWbgECam6kRAYGMHNqaTUAgzc3UiIBARg5tzSYgkOZmakRAICOHtmYTEEhzMzUiIJCRQ1uzCQikuZkaERDIyKGt2QQE0txMjQgIZOTQ1mwCAmlupkYEBDJyaGs2AYE0N1MjAgIZObQ1m4BAmpupEQGBjBzamk1AIM3N1IiAQEYObc0mIJDmZmpEQCAjh7ZmExBIczM1IiCQkUNbswkIpLmZGhEQyMihrdkEBNLcTI0ICGTk0NZsAgJpbqZGBAQycmhrNgGBNDdTIwICGTm0NZuAQJqbqREBgYwc2ppNQCDNzdSIgEBGDm3NJiCQ5mZqREAgI4e2ZhMQSHMzNSIgkJFDW7MJCKS5mRoREMjIoa3ZBATS3EyNCAhk5NDWbAICaW6mRgQEMnJoazYBgTQ3UyMCAhk5tDWbgECam6kRAYGMHNqaTUAgzc3UiIBARg5tzSYgkOZmakRAICOHtmYTEEhzMzUiIJCRQ1uzCQikuZkaERDIyKGt2QQE0txMjQgIZOTQ1mwCAmlupkYEBDJyaGs2AYE0N1MjAgIZObQ1m4BAmpupEQGBjBzamk1AIM3N1IiAQEYObc0mIJDmZmpEQCAjh7ZmExBIczM1IiCQkUNbswkIpLmZGhEQyMihrdkEBNLcTI0ICGTk0NZsAgJpbqZGBAQycmhrNgGBNDdTIwICGTm0NZuAQJqbqREBgYwc2ppNQCDNzdSIgEBGDm3NJiCQ5mZqREAgI4e2ZhMQSHMzNSIgkJFDW7MJCKS5mRoREMjIoa3ZBATS3EyNCAhk5NDWbAICaW6mRgQEMnJoazYBgTQ3UyMCAhk5tDWbgECam6kRAYGMHNqaTUAgzc3UiIBARg5tzSYgkOZmakRAICOHtmYTEEhzMzUi8AVw2rznM39f5AAAAABJRU5ErkJggg=="></image>
    </symbol>

    <g transform="translate(25 50)">
        <use href="#image1" width="150" height="100"></use>
    </g>

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

Before:
test

After, which also matches Chrome:
test

@RazrFalcon
Copy link
Collaborator

A yet another symbol bug? What a surprise. One the of the worst SVG features.

As for a solution, hard to say. As long as it doesn't break existing tests it should be fine, but it feels like a hack. No idea how it should be handled properly.

@RazrFalcon
Copy link
Collaborator

RazrFalcon commented Oct 14, 2024

Can we simply override the viewbox size like we do in convert_svg? Then we would not need any changes to the units code. Or does it break something?

@LaurenzV
Copy link
Contributor Author

Can we simply override the viewbox size like we do in convert_svg? Then we would not need any changes to the units code. Or does it break something?

I guess it should work, let me try.

@LaurenzV
Copy link
Contributor Author

Hmm, I'm not sure though, because a view box conceptually has a x/y coordinate, too, does it really make sense to just replace the width/height of the view box in this case?

@RazrFalcon
Copy link
Collaborator

I feel like it would a more straight-forward approach since we already doing the same with svg.

@LaurenzV
Copy link
Contributor Author

Better now?

@RazrFalcon RazrFalcon merged commit 6298c1d into linebender:master Oct 14, 2024
3 checks passed
@RazrFalcon
Copy link
Collaborator

All good. Thanks!

@LaurenzV LaurenzV deleted the symbol-issue branch October 14, 2024 13:29
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.

Weird black bar in rendered SVG
2 participants