RFR: 6689: Export (and print?) the flame graph

[Alex Macdonald]

On Thu, 20 Feb 2020 22:23:10 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

> This implements [JMC-6689: Export (and print?) the flame graph](https://bugs.openjdk.java.net/browse/JMC-6689)
> 
> Two actions were added to Flame View's menu bar, each for
> - saving the current flame graph to the local file system as a `.jpg` or `.png` file
> - sending the current flame graph to a printer (or to a `.pdf` file)
> 
> Please let me know your thoughts. Thank you very much!

Neat! I've been trying this out and the functionality looks good to me. I do have a few nits that I've added as comments.

When the chart is exported as a jpg, the background gets filled in black. IMO it should be white like how it looks in JMC, but maybe that's just my preference. However, because the opacity for the rectangles is `< 1`, and the background alters the colours of the labels. I left a suggestion in-line that the canvas background could be filled in white using `fillRect`, and here's a quick comparison of the resulting jpgs.

![black-vs-white](https://user-images.githubusercontent.com/10425301/75704350-394e6200-5c87-11ea-8947-f1ced349e285.jpg)

application/org.openjdk.jmc.flightrecorder.flameview/src/main/resources/page.template line 116:

> 115: 					"	white-space: nowrap;" +
> 116: 					"   text-overflow: ellipsis;" +
> 117: 					"	overflow: hidden;" +

the space before the `text-overflow` looks to be indented with spaces, rather than a tab like the other lines

application/org.openjdk.jmc.flightrecorder.flameview/src/main/resources/page.template line 140:

> 139: 					canvas.setAttribute("height", height);
> 140: 
> 141: 					canvas.getContext('2d').drawImage(image, 0, 0);

Just a suggestion, but if the canvas is filled in here then the resulting jpg image will have a white background. But it means the .png will also have a background instead of containing just the graph on a transparent background.. so if you're going this route then maybe the background should only fill if the type is jpeg.

Something like:
canvas.getContext('2d').fillStyle = 'white';
canvas.getContext('2d').fillRect(0, 0, canvas.width, canvas.height);

application/org.openjdk.jmc.flightrecorder.flameview/src/main/java/org/openjdk/jmc/flightrecorder/flameview/views/FlameGraphView.java line 391:

> 390: 			// FIXME: FileDialog filterIndex returns -1 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=546256)
> 391: 			if (fd.getFileName().endsWith(".jpg") || fd.getFileName().endsWith(".jpeg")) { //$NON-NLS-1$ //$NON-NLS-2$
> 392: 				type = "image/jpeg"; //$NON-NLS-1$

Small nit, (and the users would have to change it manually themselves), but this would make extensions written in capitals to throw an exception. Something like `image.JPG`.

-------------



PR: https://git.openjdk.java.net/jmc/pull/56