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

Optimize and/or improve some SVG icons #77318

Merged
merged 1 commit into from
May 22, 2023

Conversation

MewPurPur
Copy link
Contributor

@MewPurPur MewPurPur commented May 21, 2023

These manual optimizations save 8.3 kB. Unlike svgcleaner, which never affects the content, I slightly changed some icons. But it's usually either unnoticeable or an improvement.

Initially I wanted to check if we're using any ellipses where we should be using circles, but I ended up doing a little bit more. Overview on the icons changed:

  • BezierHandles icons: Tweaked them a little, replaced ellipses with circles.
  • CylinderMesh: Used an ellipse and a simple path composed of two segments and an arc, instead of a complex path with dozens of controls. Did this inside a standard 16x16 viewbox.
  • CylinderShape: Remade it inside a standard 16x16 viewbox.
  • GPUParticlesAttractor icons: Tweaked the ellipses, removed some unnecessary transforms.
  • ToolMove, ToolRotate, ToolScale: Removed some unnecessary points, merged some arcs, split out the point in the middle as its own circle.
  • KeyTrack icons: They are basically like the tool icons, but more scuffed and with an unused ellipse inside the SVG. Applied the aforementioned changes and removed the ellipse.
  • PointMesh: Used circles instead of ellipses.

@MewPurPur MewPurPur requested a review from a team as a code owner May 21, 2023 20:35
@Calinou Calinou added this to the 4.x milestone May 21, 2023
@capnm
Copy link
Contributor

capnm commented May 22, 2023

I ran all the icons through a ThorVG development version. It may help you make it cleaner (and make the svg parser a bit faster:). Where I've looked, the unsupported elements are unused junk added by the SVG software...

thorvg-svg-parse-test.txt.gz

@MewPurPur
Copy link
Contributor Author

@capnm Thanks, although I personally would need a bit more handholding in order to be able to make use of what you've uploaded. How can I interpret the text file?

@capnm
Copy link
Contributor

capnm commented May 22, 2023

E.g. the image GuiRadioCheckedDisabled.svg:

<svg enable-background="new 0 0 16 16" height="16" viewBox="0 0 16 16"
                           width="16" xmlns="http://www.w3.org/2000/svg">
<path d="m15 8c0 3.866-3.134 7-7 7s-7-3.134-7-7 3.134-7 7-7 7 3.134 7 7" fill="#808080"/>
<path d="m12 8c0 2.209-1.791 4-4 4s-4-1.791-4-4 1.791-4 4-4 4 1.791 4 4" fill="#b3b3b3"/>
</svg>

[L] SVG (../src/loaders/svg/tvgXmlParser.cpp 363): Unsupported attributes used
[Elements type: Svg][Id : NO_ID][Attribute: enable-background][Value: new 0 0 16 16]

→ the attribute enable-background="new 0 0 16 16" is ignored and can be deleted.

[L] COMMON (../src/lib/tvgCanvas.cpp 61): Draw S. ---- Canvas(0x55811bee6500)
[L] SW_ENGINE (../src/lib/sw_engine/tvgSwRaster.cpp 1500): Unpremultiply [Size: 16 x 16]

Here you can check the calculated resulting image size.

[L] COMMON (../src/lib/tvgCanvas.cpp 63): Draw E. ---- Canvas(0x55811bee6500)
Generated PNG file: /intranet2/godot-build/godot-git/editor/icons/GuiRadioCheckedDisabled.png

The same file name as the *.svg file.
I would also delete xmlns="hxxp://www.w3.org/2000/svg" because it isn't needed in Godot context:

<svg height="16" viewBox="0 0 16 16" width="16">
<path d="m15 8c0 3.866-3.134 7-7 7s-7-3.134-7-7 3.134-7 7-7 7 3.134 7 7" fill="#808080"/>
<path d="m12 8c0 2.209-1.791 4-4 4s-4-1.791-4-4 1.791-4 4-4 4 1.791 4 4" fill="#b3b3b3"/>
</svg>

this would be then the equivalent and optimal version.

@MewPurPur
Copy link
Contributor Author

The xmlns thing is used in all of our svg's, so I'm not so sure about removing it. The rest I will look into for further PRs if no one does so in the meantime.

@capnm
Copy link
Contributor

capnm commented May 22, 2023

The xmlns thing is used in all of our svg's, so I'm not so sure about removing it. The rest I will look into for further PRs if no one does so in the meantime.

It is answered here for html5 as Optional for inlined <svg> also IMO here in the Godot user interface context (~ html5 inlined), it is standard compliant to remove it.

@YuriSizov YuriSizov modified the milestones: 4.x, 4.1 May 22, 2023
@capnm
Copy link
Contributor

capnm commented May 22, 2023

An another example taken from your current PR change:

diff --git a/editor/icons/BezierHandlesLinear.svg b/editor/icons/BezierHandlesLinear.svg
index 2667779dcbff..c3dd0297ed48 100644
--- a/editor/icons/BezierHandlesLinear.svg
+++ b/editor/icons/BezierHandlesLinear.svg
@@ -1 +1 @@
-<svg height="16" viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg"><path d="m8.2711868 4.7796612-6.3728828 8.7118648z" fill="none" stroke="#87b1d7" stroke-miterlimit="4.9" stroke-width=".5"/><ellipse cx="1.898304" cy="13.491526" fill="#e0e0e0" rx="1.267586" ry="1.199789"/><path d="m14.237288 13.491526-5.9661012-8.7118648" fill="none" stroke="#87b1d7" stroke-miterlimit="4.9" stroke-width=".5"/><path d="m5.6316733 8.3879317 2.6395135-3.6082705 2.4416832 3.5654122" fill="none" stroke="#61b2ff" stroke-width="1.5"/><g fill="#e0e0e0"><ellipse cx="14.237288" cy="13.491526" rx="1.267586" ry="1.199789"/><ellipse cx="8.271187" cy="4.779661" rx="1.267586" ry="1.199789"/><path d="m5.0847454 7.9363749a1.2675855 1.1997888 0 0 0 -1.2675781 1.1992188 1.2675855 1.1997888 0 0 0 1.2675781 1.1992183 1.2675855 1.1997888 0 0 0 1.2675781-1.1992183 1.2675855 1.1997888 0 0 0 -1.2675781-1.1992188zm.00195.4238282a.84677333.80148375 0 0 1 .8476593.8007812.84677333.80148375 0 0 1 -.8476562.8007812.84677333.80148375 0 0 1 -.8476562-.8007812.84677333.80148375 0 0 1 .8476562-.8007812z"/><path d="m11.254237 7.9043407a1.2836218 1.231838 0 0 0 -1.2836135 1.2312528 1.2836218 1.231838 0 0 0 1.2836135 1.2312525 1.2836218 1.231838 0 0 0 1.283614-1.2312525 1.2836218 1.231838 0 0 0 -1.283614-1.2312528zm.002.4351497a.85748593.82289328 0 0 1 .858383.8221719.85748593.82289328 0 0 1 -.85838.822172.85748593.82289328 0 0 1 -.858379-.822172.85748593.82289328 0 0 1 .858379-.8221719z"/></g></svg>
+<svg height="16" viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg"><path d="m1.8 13.5 6.2-9 6.2 9" fill="none" stroke="#87b1d7" stroke-width=".5"/><path d="M5.3 8.4 8 4.5l2.7 3.9" fill="none" stroke="#61b3ff" stroke-width="1.5" stroke-miterlimit="1"/><g fill="#e0e0e0"><circle cx="1.8" cy="13.5" r="1.25"/><circle cx="14.2" cy="13.5" r="1.25"/><circle cx="8" cy="4.5" r="1.25"/></g><g fill="none" stroke="#e0e0e0" stroke-width=".5"><circle cx="4.8" cy="9.1" r="1"/><circle cx="11.2" cy="9.1" r="1"/></g></svg>
[L] SVG (../src/loaders/svg/tvgXmlParser.cpp 363): Unsupported attributes used [Elements type: Path][Id : NO_ID][Attribute: stroke-miterlimit][Value: 1]
[L] COMMON (../src/lib/tvgCanvas.cpp 61): Draw S. -------------------------------- Canvas(0x55811bee6500)
[L] SW_ENGINE (../src/lib/sw_engine/tvgSwRaster.cpp 1500): Unpremultiply [Size: 16 x 16]
[L] COMMON (../src/lib/tvgCanvas.cpp 63): Draw E. -------------------------------- Canvas(0x55811bee6500)
Generated PNG file: /intranet2/godot-build/godot-git/editor/icons/BezierHandlesLinear.png

The attribute: stroke-miterlimit is not supported, so you can remove it and compare outside of Godot to be sure it still renders as intended. If it is different, you need to change the design to avoid unsupported svg stuff.

<svg height="16" viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg"><path d="m1.8 13.5 6.2-9 6.2 9" fill="none" stroke="#87b1d7" stroke-width=".5"/><path d="M5.3 8.4 8 4.5l2.7 3.9" fill="none" stroke="#61b3ff" stroke-width="1.5"/><g fill="#e0e0e0"><circle cx="1.8" cy="13.5" r="1.25"/><circle cx="14.2" cy="13.5" r="1.25"/><circle cx="8" cy="4.5" r="1.25"/></g><g fill="none" stroke="#e0e0e0" stroke-width=".5"><circle cx="4.8" cy="9.1" r="1"/><circle cx="11.2" cy="9.1" r="1"/></g></svg>

BezierHandlesLinear1

BezierHandlesLinear2

@MewPurPur
Copy link
Contributor Author

MewPurPur commented May 22, 2023

The miterlimit is useful, why would it be unsupported?? Here, I used it to remove the tiny bump that poked out on top of the point; though I can alternatively just make the segments a little shorter and move the pen.

I'll force push later, pls don't merge this in the meantime

@YuriSizov
Copy link
Contributor

You can change it to a draft if you want to do more changes. But you don't have to, your PR is fine as is.

@MewPurPur
Copy link
Contributor Author

Replaced the miterlimit with a small path tweak, now it's good.

@MewPurPur MewPurPur force-pushed the optimize-svg-ellipses branch from b042051 to eef8c27 Compare May 22, 2023 19:15
@akien-mga akien-mga merged commit 061c6f2 into godotengine:master May 22, 2023
@akien-mga
Copy link
Member

Thanks!

@MewPurPur MewPurPur deleted the optimize-svg-ellipses branch May 22, 2023 21:08
@capnm capnm mentioned this pull request Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants