Bugzilla – Full Text Bug Listing
The patch is not well formated (inverse patch). To facilitate the review process, I wanted to create a gerrit, with you as author but it's not possible: "error: The author does not have a Gerrit account. All authors must either be a commiter on the project, or have a current CLA on file. Please see http://wiki.eclipse.org/CLA" Is it possible for you to create this gerrit commit? It's not mandatory, but it simplifies the "patch" application to test it. The problem of your solution is that the memory footprint can be strongly impacted (one image cache by SVG by zoom level). Created attachment 246315 [details]
Correctly formatted patch
Related to this, it seems that GMF Tooling has a figure here: http://git.eclipse.org/c/gmf-tooling/org.eclipse.gmf-tooling.git/tree/plugins/org.eclipse.gmf.runtime.lite.svg/src/org/eclipse/gmf/runtime/lite/svg/SVGFigure.java which, from a quick reading of the code (I have not tested it) re-renders the SVG itself on each Draw2D paintFigure() call. This would avoid having to deal with a cache, at the cost of runtime performance. Only actual tests and measures on concrete cases could tell us which approach is best. It may be possible to test this without touching Sirius itself by using the existing extension points to provide a custom edit part which uses this specific figure implementation. currently working on it I've restarted work on this in the last few days, and have some preliminary results (not published anywhere yet). Unfortunately a real, complete solution which takes all the trade-offs into account (rendering quality, rendering time, memory consumption) is more impacting and risky that what we can push this late in the cycle. We may push a fix for an SWT resource leak detected while working on this code, but the actual improvement of SVG rendering will have to wait. New Gerrit change created: https://git.eclipse.org/r/64731 New Gerrit change created: https://git.eclipse.org/r/64733 New Gerrit change created: https://git.eclipse.org/r/66132 New Gerrit change created: https://git.eclipse.org/r/66131 Gerrit change https://git.eclipse.org/r/66132 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=bb031978234dcdfb85512d67640b1e063870a9cb Gerrit change https://git.eclipse.org/r/66131 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=3406ee32a1f643604bdcf998aee00a16f2690fda New Gerrit change created: https://git.eclipse.org/r/66317 New Gerrit change created: https://git.eclipse.org/r/66316 New Gerrit change created: https://git.eclipse.org/r/66315 Gerrit change https://git.eclipse.org/r/66316 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=84c5183ef0e3296da08828a2b1ed48db7d05ca26 Gerrit change https://git.eclipse.org/r/66315 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=208418a8db5681c36c52800da01163464e589409 Gerrit change https://git.eclipse.org/r/66317 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=a37147db68ae3003c9d379b2411dad96244d9d12 New Gerrit change created: https://git.eclipse.org/r/66906 New Gerrit change created: https://git.eclipse.org/r/66905 New Gerrit change created: https://git.eclipse.org/r/66904 New Gerrit change created: https://git.eclipse.org/r/66903 New Gerrit change created: https://git.eclipse.org/r/66928 New Gerrit change created: https://git.eclipse.org/r/66927 Gerrit change https://git.eclipse.org/r/66928 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=aec781c3deb92d7659512ffdcc8b3dc5d97030e1 Gerrit change https://git.eclipse.org/r/66927 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=47e7573e73d4a130b74f3d1e03ed51a905e7d119 Gerrit change https://git.eclipse.org/r/66906 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=a4ce04159734e26350ff87408932216500e0ddb0 Gerrit change https://git.eclipse.org/r/66905 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=c008499e5956922de4d7091e890222b62eb4f979 Gerrit change https://git.eclipse.org/r/66904 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=36ccc7b9f424913c5197af9148d24b9acbf80c30 Gerrit change https://git.eclipse.org/r/66903 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=ffd657406aa9f5d2fbfa67e6b422d1e0e1e15052 New Gerrit change created: https://git.eclipse.org/r/67063 New Gerrit change created: https://git.eclipse.org/r/67062 New Gerrit change created: https://git.eclipse.org/r/67061 New Gerrit change created: https://git.eclipse.org/r/67066 New Gerrit change created: https://git.eclipse.org/r/67069 New Gerrit change created: https://git.eclipse.org/r/67068 New Gerrit change created: https://git.eclipse.org/r/67067 Gerrit change https://git.eclipse.org/r/67068 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=08dcd2af0d5d69d65a505278b14439656dc19a55 Gerrit change https://git.eclipse.org/r/67069 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=69ab78cbbeee444e3592898637d983272f15cf63 Gerrit change https://git.eclipse.org/r/67067 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=89aaf8ac55fb02d2e0007a4a26a42f10d2706c45 Gerrit change https://git.eclipse.org/r/67066 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=a7ecf8f13b7ecd7d4b17ef288a89965dc4c35657 Gerrit change https://git.eclipse.org/r/67063 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=026e070ad198948a5d776c3e631d71ae0ac63b07 Gerrit change https://git.eclipse.org/r/67062 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=c530ac267dcf8e7779fbc06e4c710fa397d488b7 Gerrit change https://git.eclipse.org/r/67061 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=c024e70ab53d41e7c8c2f4665999c6cad3be0d74 New Gerrit change created: https://git.eclipse.org/r/68151 New Gerrit change created: https://git.eclipse.org/r/68150 New Gerrit change created: https://git.eclipse.org/r/68149 Gerrit change https://git.eclipse.org/r/68150 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=64c89ccdd9352a18cf481a964c6c4a6b5f1b4deb Gerrit change https://git.eclipse.org/r/68151 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=4c5e96f4b45919ccbba25eafdf56f9077fe1bbde Gerrit change https://git.eclipse.org/r/68149 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=8e89137623cfab73e00a871685a977b2443ddd57 New Gerrit change created: https://git.eclipse.org/r/70349 New Gerrit change created: https://git.eclipse.org/r/70348 New Gerrit change created: https://git.eclipse.org/r/70350 New Gerrit change created: https://git.eclipse.org/r/70484 Gerrit change https://git.eclipse.org/r/70350 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=5f957f75c7fb8c8a632b0a90047096eee4975dc2 Gerrit change https://git.eclipse.org/r/70484 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=7b763b843f91c6c591ae91653eb429d5917665f2 Gerrit change https://git.eclipse.org/r/70348 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=7a5b79b6415e87b1bffc071440bf2ce2f679c3b6 Gerrit change https://git.eclipse.org/r/70349 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=e7db12d5b124a33ec7cc9bf986b39d923ecae829 Fixed. There are some issues with Batik mis-handling some SVG files, but there are not introduced by this change, they are just more easily triggered once you use more SVGs. Not sure we can do anything about it though, except try to understand better what kind of SVG features make Batik crash and document it. A new system property has been added, org.eclipse.sirius.diagram.ui.svg.maxCacheSizeMB, to configure the maximum size (in MB) to use to cache pre-rendered SVGs. The current default value is 50, which means 50MB. The default value and the configuration mechanism itself may change before the final release. Available in Sirius 4.0.0. |
Created attachment 246207 [details] Project and patch On the diagram, at zoom > 100%, the rendering of a SVG file is equivalent to a PNG file. I'm working on eclipse Kepler. To reproduce this bug, I used the get-started project, with the current head of git repository (e4ee76). Attachments : * man.svg : the svg to copy in the 'icons' folder of the plug-in 'org.eclipse.sirius.sample.basicfamily.design' ; * basicfamily.odesign : the design modified to take into account the SVG ; * SvgProject : a basic project to show the problem ; * SVG.patch : a proposition of correction. What happens ? In the 'paintFigure' method of the 'AbstractCachedSVGFigure' class (package org.eclipse.sirius.diagram.ui.tools.api.figure), the image to display is retrieve from cached images. But, the 'getClientArea()' method returns always the same value (zoom is not taken into account) : the image is always the same. By considering the scale (getContextKey and getCachedImage), the image is good. By changing the drawImage to call on graphics, the image is correctly positionned on the diagram (the former one did the scaling, so the image had a good size, but a bad 'resolution'). For me, this is a hack, and I have the feeling that getClientArea shall return a Rectangle with scale taken into account. Am I wrong ? Note : When exporting diagram as SVG file, the SVG images are drawn like PNG. It seems that the method in charge of copying the image (CopyToImageUtil.copyToImage(DiagramEditPart, IPath, ImageFileFormat, IProgressMonitor)) uses the 'paintFigure' method instead of using the SVG file. But this is GMF code.