fritzing / fritzing-app

Fritzing desktop application
http://fritzing.org
Other
4.04k stars 833 forks source link

SVG speed optimization #3589

Open KjellMorgenstern opened 4 years ago

KjellMorgenstern commented 4 years ago

When profiling Fritzing, svg file parsing and loading show up quite prominent ( 8% of CPU time), and would be therefore a good place to start optimization.

At some point SVG is parsed to a tree, modified, then serialized again, then the string is modified, then deserialized into a DOM again.

KjellMorgenstern commented 4 years ago

I am also looking into resvg / usvg instead of QtSvg. We would get a much more svg standard compliant rendering (reduces issues with inkscape and illustrator created files). Also, the idea of having a high level svg and micro level svg (usvg) is quite tempting, in fact Fritzing does simplify the svg in many use cases, but I don't think the abstraction is very clean.

KjellMorgenstern commented 4 years ago

See also https://github.com/fritzing/fritzing-app/tree/svg_optimize branch for a quick attempt to optimize. . I don't mind using #3587 instead, but it must be made sure there are no regressions, and that the optimization is worth the extra code lines.

DrItanium commented 4 years ago

You went in a different direction :D. I decided to persist the svg dom as long as possible. I did not even notice that removeXMLEntities was accepting a QString by value... Good call on the rvalue reference.

DrItanium commented 4 years ago

How about both fixes? You've identified another issue that I completely missed. I only changed the code to persist the SVGDom across function boundaries. How the fixing code works is exactly the same. I did the analysis of how those functions worked and found it safe to change in how it was done.

If I use your fixes by itself I can see a slight speed up but the adafruit parts bin still takes some time to load. When I use my fixes by itself it is even faster. I think having both your fixes and mine may actually be the best way moving forward. Doing a search for removeXMLEntities in the code shows that there are many places where that function is used. Combining both I think makes the most sense.

DrItanium commented 4 years ago

The two branches combined makes part bin loading and searching so fast in Debug!

failiz commented 3 years ago

I would really love to see both fixes merged. It takes around 15 minutes to start a debugging session in QtCreator in my computer. I managed to reduce it to one/two minutes by removing most of the core parts and leaving just the minimum ones, but it is still a bit annoying. In any case, I guess that we should start to think in replacing the QtSvg library as it has been depreciated. https://wiki.qt.io/Qt_Modules_Maturity_Level. I am not an expert and maybe it is not feasible, but could itr be possible to use QWebEngine or Qt WebKit? https://stackoverflow.com/questions/60901562/qtsvg-ignores-units

KjellMorgenstern commented 3 years ago

The the speed improvement in debug mode... it takes only seconds on my typical development environment. Qt Creator is slower in debug mode, but not in the range of minutes. Do you have valgrind running or something like that?

Replacing QtSVG with Webkit doesn't make much sense to me. Only a different lightweight TinySVG library could be a candidate. I evaluated a different svg parser beginning of this year, namely looked at resvg and its usvg (micro svg layer) approach. There is more to it than just QtSvg replacement. Extending beyond the TinySVG standard would cause quite some problems, and will not free Fritzing from the custom tweaks done when reading such files. Many of the SVG extensions rely more or less on Chromium, people hack whole games in CSS (which is one of those extensions) Imagine breaking that down into a gerber file again.

failiz commented 3 years ago

Nope, no valgring or similar running. Just the default options in QtCreator. No idea why is so slow then.