RazrFalcon / svgcleaner

svgcleaner could help you to clean up your SVG files from the unnecessary data.
GNU General Public License v2.0
1.63k stars 94 forks source link

Apply transforms to paths #47

Open RazrFalcon opened 8 years ago

RazrFalcon commented 8 years ago

Had been already implemented, but there are a lot of code to port and it's not really helping in size reducing.

mikebronner commented 8 years ago

I was looking for this, and it was a critical feature for using SVG Cleaner for me (it was the main reason I used SVG Cleaner). Should I revert back to v0.6.x for this? (Currently using 0.7.1)

RazrFalcon commented 8 years ago

Well, it doesn't implemented yet and main reason is it will blow up the file size, which is pointless for application with "cleaner" in it.

Basically, I already have a working implementation, but adding it to svgcleaner is more philosophical question. svgcleaner cleans svg files, it doesn't just modified them. So such features should be in a different application.

mikebronner commented 8 years ago

Gotcha ... makes sense ...

Vasilich commented 8 years ago

well, cleaning (baking in) transforms is also kind of cleaning. It is SVG cleaner, not SVG minimizer :) So at leas i vote for implementing it with setting to activate if desired.

mikebronner commented 8 years ago

I agree with @Vasilich view, but of course can respect @RazrFalcon decision on how he wants to develop his app. :)

RazrFalcon commented 8 years ago

@Vasilich

It is SVG cleaner, not SVG minimizer

Well. It is.

There are two problems with this option:

  1. If we will apply all transforms - it will blow up a file size, which is pointless. It's SVG cleaner, not SVG modifier.
  2. Detecting that new path became smaller after transform applying is very hard and very expensive (for CPU).
  3. Before applying a transform we must convert all segments to CurveTo and than back, if possible. And usually not. For example:
    • we can't convert ArcTo to CurveTo and than back, at least I don't know how. And A->C conversion can produce from 1 up to 3 curves, which is already bigger
    • if transform contains rotation or skew - it will break H, V segment conversion, so we have to represent them as LineTo, which is bigger

So basically, it's a very hard task for SVG cleaner and if you want to just apply all transforms - you should use another tool. Which one? I don't know. I've planed to write some kind of svgtoolkit which will support this.

PS: can you explain why you need this? The transform is like a most basic part of an SVG, it can't be unsupported or something.

Vasilich commented 7 years ago

i would prefer to have move and scale transforms applied - it won't enlarge file size but simplify XML code of SVG (if i understand it correct. in that case we don't need to convert everything to CurveTo ?). Other types of transform can really enlarge file size.

RazrFalcon commented 7 years ago

I tried to implement it. Only translate and scale transform are supported. It decreased an average cleaning ration on 0.5%. So it's useless. Mostly because if we have a scale transform - we also have to scale stroke-width, and if it do not exist - we should create it, which enlarges the file size.

So only way is to support only translate transform, which is rare.

I understand that it can simplify the file structure for humans and other parsing software, but since it doesn't make file smaller - it can't be implemented in the svgcleaner.

Vasilich commented 7 years ago

This transform backing-in optimization level depends on the application (i.e. files itself). I can give you around 200 files, where transform is used - in every of these files backing in transform reduces file size from 1 to 6%. One example: BR-Fernsehen.zip

Surely this option should be switchable.

RazrFalcon commented 7 years ago

Implemented. Disabled by default. Can be enabled via --apply-transform-to-paths.

Doesn't convert segments to curves. Only applies translate and/or proportional scale transformations.

Vasilich commented 7 years ago

so move isn't supported? or is it covered by translate?

Will it be supported by SVGCleaner GUI?

RazrFalcon commented 7 years ago

Move == Translate. So yes.

Yes. It's already in the git master.

Vasilich commented 7 years ago

cool. any chance to get compiled version without installing rust compiler? Maybe there are some auto builds?

RazrFalcon commented 7 years ago

What OS use using?

Vasilich commented 7 years ago

Windows (7 x64 if that matters :) )

RazrFalcon commented 7 years ago

svgcleaner_win32_0.8.1.zip

Vasilich commented 7 years ago

Thank you. After some testing i found some files that still have translate transforms. I don't know what the reason is - maybe because translate belongs to group, and inside that group there are some matrix transforms?

WDR.ZIP

RazrFalcon commented 7 years ago

The rules are here: https://github.com/RazrFalcon/svgcleaner/blob/f8b327498cc6af4b9abc0c91001deb87591afc3a/src/task/apply_transforms/mod.rs#L45..L86

Basically, element shouldn't have linked style elements. Like: fill, stroke, filter, clip-path, mask. Because transform attribute is also applied to them and this can't be resolved statically, unless linked element is used only by one element. But this case in not implemented yet.

Vasilich commented 7 years ago

this example `

` won't bake just because it has defined fill? or because it has subnodes with defined fill? how fill with fixed color can avoid translate to be baked?

RazrFalcon commented 7 years ago

This is because paths processing is done before groups ungrouping. You can fix it with --multipass flag.

I will add workaround for this case.

Vasilich commented 7 years ago

thanks for the tip with multipass - that fixed backing transforms for several files with that option turned on. Still there are some files where multipass didn't help: 11.zip would be nice if you could explain why...

RazrFalcon commented 7 years ago

svgcleaner does not ungroup groups with 2 or more children...

The problem is that I have to prove that ungrouping will lead to a smaller file. It can be implemented, but not yet.

Vasilich commented 7 years ago

thank you for explanation. I am eager to see the results of such tests.

RazrFalcon commented 7 years ago

Try this build: svgcleaner_win32_0.8.1.zip

It uses much better ungrouping/transform processing algorithm that works correctly with your files. Multipass is no longer needed.

Also, I think that transform option should be split to translate|scale|all parts. Because only translate can be applied without problems. To apply scale I have to process stroke-width, stroke-dasharray and stroke-dashoffset attributes. Which is pain. Moreover if stroke-width is not set, that mean that it's default and I have to scale it and add to an element. So it will basically replace the transform attribute making transform applying pointless.

Vasilich commented 7 years ago

sorry for late testing yes, with that version some of files from posted 11.zip are cleaned with transform translate (not all though), So that is a clear progress for me 👍 Thank you.

2 questions, not really related to this issue (don't know where to order it to..) 1, If group has fill, then will every element inherit its value if not defined? or if element has no fill, then default (= black) will be taken? I cannot find it in SVG specs. SVGCleaner does not remove fill for elements of the group even if fill value is the same for element of group and group itself. 2, Is it possible to get compiled (for Windows) version of your tool used for testing, SVGRender?

RazrFalcon commented 7 years ago
  1. If element doesn't have a fill - it will be inherited from a parent(or from parent of a parent). If parents doesn't define it - it will fallback to a default value. The problem you describing is a bug. svgcleaner should already do this, but something went wrong.
  2. Only using msys2, which contains prebuild qtwebkit fork. But I didn't tried it.