Correct handling of partially placed rings.

cdk · Jun 26, 2016 · 8675e1b · 8675e1b
1 parent 2d98f18
commit 8675e1b
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 24 deletions.
diff --git a/tool/sdg/src/main/java/org/openscience/cdk/layout/RingPlacer.java b/tool/sdg/src/main/java/org/openscience/cdk/layout/RingPlacer.java
@@ -505,6 +505,33 @@ public void placeFusedRing(IRing ring,
         atomPlacer.populatePolygonCorners(atomsToDraw, ringCenter, startAngle, addAngle, radius);
     }
 
+    /**
+     * Completes the layout of a partiallyed laid out ring.
+     *
+     * @param rset ring set
+     * @param ring the ring to complete
+     * @param bondLength the bond length
+     */
+    boolean completePartiallyPlacedRing(IRingSet rset, IRing ring, double bondLength) {
+        if (ring.getFlag(CDKConstants.ISPLACED))
+            return true;
+        IRing partiallyPlacedRing = molecule.getBuilder().newInstance(IRing.class);
+        for (IAtom atom : ring.atoms())
+            if (atom.getPoint2d() != null)
+                atom.setFlag(CDKConstants.ISPLACED, true);
+        AtomPlacer.copyPlaced(partiallyPlacedRing, ring);
+
+        if (partiallyPlacedRing.getAtomCount() > 0 && partiallyPlacedRing.getAtomCount() < ring.getAtomCount()) {
+            placeConnectedRings(rset, partiallyPlacedRing, RingPlacer.FUSED, bondLength);
+            placeConnectedRings(rset, partiallyPlacedRing, RingPlacer.BRIDGED, bondLength);
+            placeConnectedRings(rset, partiallyPlacedRing, RingPlacer.SPIRO, bondLength);
+            ring.setFlag(CDKConstants.ISPLACED, true);
+            return true;
+        } else {
+            return false;
+        }
+    }
+
     /**
      * Get the middle of two provide points.
      *

diff --git a/tool/sdg/src/main/java/org/openscience/cdk/layout/StructureDiagramGenerator.java b/tool/sdg/src/main/java/org/openscience/cdk/layout/StructureDiagramGenerator.java
@@ -581,17 +581,27 @@ private void seedLayout() throws CDKException {
                         ringPlacer.placeRingSubstituents(rset, bondLength);
                     } else {
 
-                        List<IRing> placed   = new ArrayList<>();
+                        List<IRing> placed = new ArrayList<>();
                         List<IRing> unplaced = new ArrayList<>();
 
                         for (IAtomContainer ring : rset.atomContainers()) {
                             if (ring.getFlag(CDKConstants.ISPLACED))
-                                placed.add((IRing)ring);
+                                placed.add((IRing) ring);
                             else
                                 unplaced.add((IRing) ring);
                         }
 
-                        while (!unplaced.isEmpty()) {
+                        // partially laid out rings
+                        if (placed.isEmpty()) {
+                            for (IRing ring : unplaced) {
+                                if (ringPlacer.completePartiallyPlacedRing(rset, ring, bondLength))
+                                    placed.add(ring);
+                            }
+                            unplaced.removeAll(placed);
+                        }
+
+                        while (!unplaced.isEmpty() && !placed.isEmpty()) {
+
                             for (IAtomContainer ring : placed) {
                                 ringPlacer.placeConnectedRings(rset, (IRing) ring, RingPlacer.FUSED, bondLength);
                                 ringPlacer.placeConnectedRings(rset, (IRing) ring, RingPlacer.BRIDGED, bondLength);
@@ -606,8 +616,11 @@ private void seedLayout() throws CDKException {
                                     placed.add(ring);
                                 }
                             }
-                            if (placed.isEmpty())
-                                break;
+                        }
+
+                        if (allPlaced(rset)) {
+                            rset.setFlag(CDKConstants.ISPLACED, true);
+                            ringPlacer.placeRingSubstituents(rset, bondLength);
                         }
                     }
                 }
@@ -1596,28 +1609,11 @@ private void layoutCyclicParts() throws CDKException {
         } else {
             logger.debug("...no bond found");
 
-            IRing partiallyPlacedRing = molecule.getBuilder().newInstance(IRing.class);
-
             // partially laid out ring system
             if (ringSystems != null) {
                 for (IRingSet ringset : ringSystems) {
-                    for (IAtomContainer ring : ringset.atomContainers()) {
-                        if (!ring.getFlag(CDKConstants.ISPLACED)) {
-
-                            partiallyPlacedRing.removeAllElements();
-                            for (IAtom atom : ring.atoms())
-                                if (atom.getPoint2d() != null)
-                                    atom.setFlag(CDKConstants.ISPLACED, true);
-                            AtomPlacer.copyPlaced(partiallyPlacedRing, ring);
-
-                            if (partiallyPlacedRing.getAtomCount() > 0 && partiallyPlacedRing.getAtomCount() < ring.getAtomCount()) {
-                                ringPlacer.placeConnectedRings(ringset, partiallyPlacedRing, RingPlacer.FUSED, bondLength);
-                                ringPlacer.placeConnectedRings(ringset, partiallyPlacedRing, RingPlacer.BRIDGED, bondLength);
-                                ringPlacer.placeConnectedRings(ringset, partiallyPlacedRing, RingPlacer.SPIRO, bondLength);
-                            }
-                        }
-                    }
-
+                    for (IAtomContainer ring : ringset.atomContainers())
+                        ringPlacer.completePartiallyPlacedRing(ringset, (IRing) ring, bondLength);
                     if (allPlaced(ringset))
                         ringPlacer.placeRingSubstituents(ringset, bondLength);
                 }