Update svgo.yml and disable problematic arcto optimization #4
Conversation
waldyrious
force-pushed
the
re-optimize-flags
branch
from
6c1024b
to
f3076f5
Compare
Thank you once again for your contributions! 👍The reason I was hesitant to update to newer versions of svgo is that it was causing the SVG files' thumbnails to display incorrectly. I am using Windows 10 and tibold's svg-see (formerly svg-explorer-extension). Here is how these icons re-optimized with svgo 1.3.2 look to me in explorer: And here is how the base branch icons look: There is already a bug report on the svg-see issues page from back in January regarding this, and the OP appears to have linked the cause of the bug to way the zeros are combined that you mentioned in #3. |
|
So as of right now I'm not sure what the best course of action is. The latest release of svg-explorer-extension is only 9 days old, and that's what I'm currently using, but even with that I still have this issue. Maybe @User756675678's regex hotfix was never applied... Any suggestions? |
|
Well, that certainly seems to be a bug in svg-see 🤔. The files display correctly in all the platforms I tried: chrome, firefox, safari, inkscape, macOS preview, vscode markdown preview. We could certainly require an older version of svgo, but that would make things more cumbersome for contributors only to accommodate a bug in a program that is no doubt useful, but not required for this project. That said, I'm happy to hold off this PR for a while if you think there's a good chance svg-see will integrate the fix sometime soon 👍. |
|
(As a side note, happy to see from the screenshot that you have some uncommitted/unpushed local changes! Looking forward to see them in the repo :)) |
I'll admit that originally I hadn't updated svgo for lazy/selfish reasons. But then... after you brought this further to my attention I found that issue on svg-see where the OP and project owner seem to agree that it isn't just a bug in svg-see, it's a bug inherited from qt, which across its entire codebase I think it's safe to estimate it's used by thousands of applications. I'm normally all for optimising filesize to the very last byte, but in this instance it seems to come at the cost of breaking the appearance of the icons in some apps. Quality over quantity, right?
You are right about updating svgo to the latest version. That would both benefit contributors as well as filesizes.
Until qt is able to correctly render the svgs is there possibly an option we can add to the svgo.yml to disable the |
|
Unless of course it's already fixed in qt, in which case you're right that it must be an svg-see bug. |
It would, IMO, but I'm afraid there's no such option — at least from my reading of the commit that introduced the change.
I'm not aware of an easy way to test that. It's been too long since I worked with Qt. That said, although there are indeed many applications built with Qt, I wouldn't assume it would affect that many downstream users, considering that all the other rendering engines I tested (including major browsers, which I'd expect would be the main platform where these images would be consumed) seem to cope with these just fine. Maybe another idea I was planning on proposing in a separate issue could provide an alternative solution to this: what do you think of keeping both human-readable SVG files in a |
|
Glad to hear you're working on a gallery page yourself! Would you mind moving your comment to #5 so we could discuss there and avoid polluting this thread? |
Hmm, I see, that's rather disappointing. 🤔
When you say human-readable SVG do you mean the direct output of Inkscape/Illustrator without any minification/optimisation applied by svgo? Are there other benefits to only working on the src directory? I personally chose to have my src/dist folders be one in the same so that I knew what I was seeing within Inkscape is what the end user would be seeing. |
Not exactly :) I meant something closer to Inkscape's "plain svg" output, with some cleanup from svgo on top, but not as aggressive. For example, keeping the pretty printing, relevant IDs for paths and groups, and keeping actual geometric shapes rather than converting everything to paths.
Well, call me old-fashioned, but I actually enjoy hand-editing (and hand-optimizing) SVG files. Having them be human-readable would allow quick fixes to be made much more confidently and conveniently (maybe even by editing the file in the GitHub interface!), thus attracting contributors. Also, since they're text files anyway, changes such as those I did in #1 and #2 would be understandable from the git diff, and git blame also gets more useful as not all changes edit the same very long line. 😄
I understand, but I have been following svgo for long enough to trust that they only make non-destructive changes to the files (bugs in renderers notwithstanding). |
|
Update: apparently there is a way to disable this optimization! A comment in svgo#1137 shows how this can be configured. I can edit |
Oh that's great, nice find! As for the question you asked in #3, I'm not entirely sure why I decided to set options in svgo.yml that are already the svgo default. Perhaps as a precaution for the defaults ever changing between svgo versions (seems unlikely), or perhaps so contributors who don't know the svgo defaults off by heart can still see at a glance what svgo is set to do. I'm happy for you to remove the duplicates, but maybe we could add some sort of comment to the top of svgo.yml # Default options: https://github.com/svg/svgo#what-it-can-do |
Sure, will do. If the goal was to document svgo's default behavior, it would make sense to add all the options, not a subset as we currently have — but that would be even harder to keep track of, as new options are added to svgo or their default status is (for some reason) changed. |
waldyrious
force-pushed
the
re-optimize-flags
branch
from
f3076f5
to
01fa4bf
Compare
|
Alright, I've rebased the branch and done the changes discussed above. Turns out if we disable that particular optimization, the current output matches the one produced by the latest svgo, so there's no longer a huge diff touching all flag files! However, the flag of Cabo Verde still had the optimizations which I did as part of #1 and #2, so I manually reverted the |
waldyrious
changed the title
- Remove plugins that are already the default in svgo, and link to docs - Move the `removeDimensions` plugin to be next to `removeViewBox` - Add configuration to the `convertPathData` and `mergePaths` plugins to avoid an issue with over-aggressive optimization for arcto commands introduced in svgo v1.3.0. Also: - Add missing `--config` option to the svgo command in README - Remove over-aggressive optimization from the cv.svg flag
waldyrious
force-pushed
the
re-optimize-flags
branch
from
01fa4bf
to
69e1f8d
Compare
|
Fantastic work as always, Waldir. Merged! 😃 |
|
@HatScripts I'd still like to pursue the src/dist split proposed above. Is that something you'd be willing to consider? I can open an issue to track that. |
Recent versions of svgo gained the ability to compress svg files further, in particular by removing spaces in some sections of path data.
See commit svg/svgo@258fecf (first included in release v1.3.0 of svgo) for more details.