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

SVG: Always set SVG dimentions #1171

Closed
wants to merge 1 commit into from

Conversation

utopiabound
Copy link
Contributor

  • What does this PR do?

This always sets a value for SVG size which accounts for SVG files that do not have a viewport.

Fixes #1168

  • Screenshots (if applicable)

Without Patch (showing SVG's without viewport elements):
screen

With Patch (Working as well as 1.1.1 did with the same SVGs):
with-patch

This always sets a value for SVG size which accounts for SVG files
that do not have a viewport.

Fixes fvwmorg#1168

Signed-off-by: Nathaniel Clark <[email protected]>
@ThomasAdam
Copy link
Member

Hi @utopiabound

Not sure about this. This was my original approach to handling this, and it broke SVG rendering for some versions of librsvg.

I'm inclined to just leave things as-is, and allow people to correctly "update" their SVG icons the way they're probably supposed to.

@ThomasAdam ThomasAdam closed this Feb 14, 2025
@utopiabound
Copy link
Contributor Author

Hi @utopiabound

Not sure about this. This was my original approach to handling this, and it broke SVG rendering for some versions of librsvg.

Do you recall with version of librsvg? This code is only for version 2.52.0+. This is working for me on 2.59.2.

I'm inclined to just leave things as-is, and allow people to correctly "update" their SVG icons the way they're probably supposed to.

There are a lot of upstream icons that don't have viewbox defined, I do not believe it's a required field in the standard, and it just leaves holes in menus if FVWM doesn't even try to render them. This patch does the right thing if that's available but then falls back to the previous behavior which was working.

@7185
Copy link

7185 commented Feb 21, 2025

Thank you for this patch.
viewBox is not mandatory on SVG files, it's perfectly fine to write <svg width="64" height="64" ...> for a 64 px (default unit) squared viewport even though it's not recommended. These values can be overwritten if using something like Icon file.svg:16x16. Scaling would be different without a viewBox but it should still render something valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip:changelog Issue/PR should skip CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SVG icons no longer work after updating to FVWM3 1.1.2
3 participants