Netflix / flamescope

FlameScope is a visualization tool for exploring different time ranges as Flame Graphs.
Apache License 2.0
3.02k stars 169 forks source link

feat: "save as SVG" button #19

Closed doug-skinner closed 5 years ago

doug-skinner commented 6 years ago

This is in response to #2, and adds the minimal amount of code to do so. Unfortunately the way SVGs work I could only style the output image correctly by going through the individual elements and adding the styles that came from the css from the 3-flame-graph/dist/d3-flamegraph.css file.

The flame graph is converted to a downloadable SVG with styling included, makes that the href of a hidden a tag, makes the a tag a download link, and finally clicks the link. The output file is called FlameGraph.svg, and the whole process happens instantly on my laptop.

Let me know if anything seems incorrect, it's my first time contributing to a React app.

spiermar commented 6 years ago

This might help.

doug-skinner commented 6 years ago

The function has been removed out into an action instead of a component, thus allowing the button code to be placed in both FlameGraph and HeatMap. I have also made it so that all applied css is used, however that creates a slight problem by making the file size dramatically larger, roughly 15MB for an average FlameGraph and about 20MB for the example HeatMap.

This was done using the save as SVG method that I had uploaded the other way. While you're link was helpful in showing a different way to tackle the problem, I wasn't sure if you wanted to add the dependencies to the project. If you do, I will go ahead and switch to the other method.

The last point of concern is what the actual format that you want the file to be is, as issue 2 says SVG, but the example uses a PNG and that is what most other articles show when exporting D3 SVGs. Please advise on what method is best.

spiermar commented 6 years ago

@doug-skinner How much of the 15-20Mb is styling vs actual elements? If it's actual elements, not much we can do. We will have to opt for having a separate style for exports or having huge files. Not bit a big fan of either. I don't mind the external dependencies, as long as they are being maintained, but SVGs are better, specially on flame graphs, where search is helpful. If there's no way around the huge file size on dynamic style SVGs, I'm ok with merging a custom style for exports, if they are kept in a separate less/css file or component, making it more "visible" to contributors that the style bring applied is different.

doug-skinner commented 6 years ago

Okay, just to confirm the numbers I ran it both with the two for loops to grab the styling and without, against a flame graph generated from the entire second column of the first example heat map.

I've attached a screenshot showing both generated graphs to below with the "proper" css version on the left, to see the differences.

screen shot 2018-04-10 at 7 12 56 pm

The main difference for the file size is that the cssText variable of each node contains the value of every single css variable that exists for that node, not only the ones that are set. So it includes a bunch of values that are not actually needed because they are the default values for that element type. It then attaches all of the unnecessary data to each and every element. But after scouring the web I am unable to find another way to dynamically get the css values that we are looking for if we want this export code to work without updating the javascript function every time the css for those elements changes.

spiermar commented 6 years ago

Ok, difference is all the useless styling being applied inline. In this case, my suggestion keeping a separate style for the export, on a separate file, and we can improve it later.

doug-skinner commented 6 years ago

Okay, that is something that can be done, however I noticed another caveat of going through with the external css file that is linked in. The SVG doesn't actually put the css file inside of it, instead it relies on the path that you specified.

For instance, if our stylesheet include was: <?xml-stylesheet href="example.css" type="text/css"?>

And also say the file was downloaded to the folder: User/Downloads

When we open the downloaded SVG it looks for the file to be found at: User/Downloads/example.css which won't work, unless we also download the css file.

It looks like saving an SVG with external stylesheets is not an uncommon problem, and the only solutions being suggested online are either the cssText option that bloats the file size or to have D3 style the SVGs inline when they are generated instead of applying classes that would involve re-writing the D3-flamegraph and D3-heatmap libraries, which is not ideal.

Please let me know how you'd like to proceed. It almost seems like this feature, while very nice to have may not be worth the work right now.

spiermar commented 6 years ago

I was looking to have the source CSS in a separate file, so there is no confusion for contributors. The SVG style should be in the SVG file itself, event if it bloats the file a little bit.

brendangregg commented 6 years ago

So the 727KB version was working, just with bad fonts (looking at those screenshots)? One consideration is that it's handy to be able to just email these to people as single files. If it requires a separate css file, then it's tricker to explain to people how to view them. Does it just default to the bad fonts without the css file?

doug-skinner commented 6 years ago

Here's a closer screenshot of the 727KB version:

screen shot 2018-04-12 at 10 20 13 am

So it has a bad font, no padding or outline between rects, and the tooltip code doesn't come through so any smaller boxes that don't show a label due to size aren't ever able to show the labels. Example:

screen shot 2018-04-12 at 10 24 05 am

Also none of the searching comes through, although it's been a couple months since I've used the standalone flame graph tool so I don't remember if the searching was built in or not.

spiermar commented 5 years ago

Closing due to inactivity. Will likely add an export HTML option, with the inlined json data, since it's simpler and could have more features.