Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#533 SVGGenerator and friends

Needs_Revision
pending
cdk-1.4.x (181)
cdk-1.4.x
5
2013-09-23
2012-07-31
Ralf Stephan
No

See the pull request at https://github.com/cdk/cdk/pull/27

This implements dedicated SVG output (not via canvas) and is already used in JCP.
Had to adapt a few lines because of diverging code base.
A three-liner usage example is given with the pull request.

Regards,
ralf

Discussion

1 2 > >> (Page 1 of 2)
  • Gilleain,

    this patch acknowledges your work, but does not use your git patches. Are you happy with these patches to just copy in your code, but loosing you as a patch writer in the git history?

     
  • That's perfectly fine by me.

     
  • Oh, wait... let me do some quick patches...

     
  • Ralf Stephan
    Ralf Stephan
    2012-10-06

    documentation patch

     
    Attachments
    t
  • Ralf Stephan
    Ralf Stephan
    2012-10-06

    I have added a patch here, addressing all documentation issues raised by egonw

     
  • OK! Applying the patch did not work out of the box, but I think I got all additions converted into a git patch... see the 427-14x-svg branch.

     
  • Ralf Stephan
    Ralf Stephan
    2012-10-09

    I have added a unit test that checks output with an empty model, also validates it. It takes some time, so I guess it loads the DTD/Schema from the W3C. NOTE: for some reason this work in Eclipse but ant couldn't compile with ant -Dtestclass=rendersvg.SVGGeneratorTest -Dcdk.debugging=false junit-test
    and I can't see why...

    https://github.com/rwst/cdk/commit/d5d4c1bc34fe476aa8ab465728a5bc0a0860b7a6

     
  • John May
    John May
    2012-10-28

    Whats the status on this and what needs reviewing? Is it just Egon's branch?

     
  • Some issues are ironed out, but it could still use a final check of the code...

     
  • John May
    John May
    2012-10-28

    • Milestone: Accepted --> Needs_Review
     
  • John May
    John May
    2012-11-04

    Okay i've had a look - it works and tests pass. Was there any reason why the XML is being assembled manually? StAX might be a bit excessive but even using DOM would be much more flexible. Even better would be to use a SVG library: http://xmlgraphics.apache.org/batik/ (Apache 2.0). This module is not (and should not) be used by another module thus if someone wants to use the SVG rendering I thing it's okay to delegate this to a library.

    This one is more personal preference - but why where the variable names changed? The originals are much better and more readable. PMD might complain but short variables like x,y,z,i,j,k,n are common convention and make the code much more very readable. I would also say xCoord definitely is not favourable as it truncates a word and thus actually less understandable. It could be any of these (words starting with Coord)[http://www.morewords.com/starts-with/coord/.]

    290 - double[] pos = transformPoint(e.x, e.y);
    282 + double[] pos = transformPoint(e.xCoord, e.yCoord);

    499 - double[] p1 = transformPoint(wedge.x1, wedge.y1);
    500 - double[] p2 = transformPoint(wedge.x2, wedge.y2);
    491 + double[] p1 = transformPoint(wedge.firstPointX, wedge.firstPointY);
    492 + double[] p2 = transformPoint(wedge.secondPointX, wedge.secondPointY);

    J

     
    • xCoord may originate from me. Remember, this code is based on earlier code.

      I like xCoord somewhat better than 'x', which may reflect a number of common variables, e.g. in e.x/e.y. Particularly, when multiple coordinates are handled in one blob of code, they really become a pain.

      But, I agree with your 'coord' point. What would help is an indication if these are screen coordinates or world coordinates, though.

       
  • Haven't looked at the code, but I plan to use the SVG rendering server side; in this context manual XML assembling is better from performance point of view (next is StAX and DOM is worst).

     
  • John May
    John May
    2012-11-05

    I've experienced speed difference with huge XML files and only when parsing - I would imagine (I could be wrong) with the size of the files this will be generating there would be no noticeable difference.

    The trouble with manual XML is there is lots to account for - file encoding, escaped characters, new lines (okay easy fix with System.getProperty("line.separator")). If you really are worried about speed then using String.format is not good - checkout this benchmark.

    I'm definitely guilty of lots of these but I would recommend a skim through Java Anti-patterns - Item 7 is Manual XML.

    Anyways - I'll knock up a SVGVisitor this afternoon and do some benchmarking.

     
    Last edit: John May 2012-11-05
    • John May
      John May
      2012-11-05

      Edit: Typos

       
  • John May
    John May
    2012-11-05

    Yep I've seen it in the other visitors. It's fine though I was just being picky :-).

    But, I agree with your 'coord' point. What would help is an indication if these are screen coordinates or world coordinates, though.

    Yep - you can use the point as the context in that case then:
    Point2D screen, world.
    screen.x
    screen.y
    world.x,
    world.y;

    What do you thing of the XML output?

    J

    On 5 Nov 2012, at 11:54, Egon Willighagen egonw@users.sf.net wrote:

    xCoord may originate from me. Remember, this code is based on earlier code.

    I like xCoord somewhat better than 'x', which may reflect a number of common variables, e.g. in e.x/e.y. Particularly, when multiple coordinates are handled in one blob of code, they really become a pain.

    But, I agree with your 'coord' point. What would help is an indication if these are screen coordinates or world coordinates, though.

    patches:533 SVGGenerator and friends

    Status: open Labels: cdk-1.4.x Created: Tue Jul 31, 2012 02:34 PM UTC by Ralf Stephan Last Updated: Mon Nov 05, 2012 09:53 AM UTC Owner: Egon Willighagen

    See the pull request at https://github.com/cdk/cdk/pull/27

    This implements dedicated SVG output (not via canvas) and is already used in JCP.
    Had to adapt a few lines because of diverging code base.
    A three-liner usage example is given with the pull request.

    Regards,
    ralf

    Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/cdk/patches/533/

    To unsubscribe from further messages, please visit https://sourceforge.net/auth/prefs/

     
  • Ralf Stephan
    Ralf Stephan
    2012-11-05

    Sorry for the delay, I was lying flat several days. One reason of manual implementation is stated in https://github.com/JChemPaint/jchempaint/wiki/The-svg-font-problem-and-its-solution

    The other reason is that no library (AFAIK) does optimize the code with USE-directives (sort of macro) and so cannot achieve the terseness (and comprehensibility) of the output. So, this was unavoidable for output quality and compression, speed comes as bonus.

    Of course, I could have simply translated the screen output but, frankly, this property of JCP does not shine, either, and I'll have to do something about it too, at some time.

    As to the long names, I'm only adapting! Egon slapped me already for using short ones!

     
  • John May
    John May
    2012-11-05

    No problem - tis the season to get ill.

    The other reason is that no library (AFAIK) does optimize the code with USE-directives (sort of macro) and so cannot achieve the terseness (and comprehensibility) of the output. So, this was unavoidable for output quality and compression, speed comes as bonus.

    Okay cool. If you want to optimise further StringBuilder > StringBuffer (thread safe - but this isn't shared between threads) and replace the String.format with multiple StringBuilder.appends.

    As to the long names, I'm only adapting! Egon slapped me already for using short ones!

    hehe yep - he said earlier but not sure if it made it to the tracker.

    Of course, I could have simply translated the screen output but, frankly, this property of JCP does not shine, either, and I'll have to do something about it too, at some time.

    I've got fixes for that ;) - meant to right the blog post this morning but didn't have time will try and start now. I found a way of aligning the atom labels correctly.

    Anyways - I've just had a quick look at Batik SVG library and you can actually use it with the AWTVisitor :-). The output looks exactly as though it were a PNG. It might not be compressed and I'm sure it's slower but check out the simplicity: gist - SourceForge code blocks are too ugly...

     
  • Ralf Stephan
    Ralf Stephan
    2012-11-06

    Proposed to be withdrawn temporarily.

    Reason: the code is a JCP hack and needs an amount of work to fit with CDK. Obviously, this should have been done before patch submission. It is however
    unclear if not a unified approach where canvas and SVG shortcomings are fixed
    within the same code would be better.

     
  • John May
    John May
    2012-11-07

    Okay no problem. A unified approach would of course be best but it's more difficult so don't worry. I think having a separate visitor might still be good for case like Nina's where you want to get SVG fast and not worry about how it looks.

    I'll change the status to pending

     
  • John May
    John May
    2012-11-07

    Personally I would add the batik library to JChemPaint (might need to bump up LGPL v3 if you're not already) but this will at least allow you to export SVG right now. The optimisations/unifications could come later.

     
  • John May
    John May
    2012-11-07

    • status: open --> pending
    • branch: --> master
     
  • John May
    John May
    2012-11-07

    • branch: master --> cdk-1.4.x
     
    • Group: Needs_Review --> Needs_Revision
     
1 2 > >> (Page 1 of 2)