Fix problem with placing positional variation, rather than skipping t…

…o the next one we initially pre sort the bonds in the order we prefer.
cdk · Aug 22, 2017 · e443f46 · e443f46
1 parent 0298914
commit e443f46
Showing 1 changed file with 32 additions and 7 deletions.
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
@@ -2281,7 +2281,18 @@ private int safeUnbox(Integer x) {
         return x == null ? 0 : x;
     }
 
-    private void placePositionalVariation(IAtomContainer mol) {
+    private int getPositionalRingBondPref(IBond bond, IAtomContainer mol) {
+        int begRingBonds = numRingBonds(mol, bond.getBegin());
+        int endRingBonds = numRingBonds(mol, bond.getEnd());
+        if (begRingBonds == 2 && endRingBonds == 2)
+            return 0;
+        if ((begRingBonds > 2 && endRingBonds == 2) ||
+            (begRingBonds == 2 && endRingBonds > 2))
+            return 1;
+        return 2;
+    }
+
+    private void placePositionalVariation(final IAtomContainer mol) {
 
         final List<Sgroup> sgroups = mol.getProperty(CDKConstants.CTAB_SGROUPS);
         if (sgroups == null)
@@ -2300,19 +2311,36 @@ private void placePositionalVariation(IAtomContainer mol) {
             idxs.put(atom, idxs.size());
 
         for (Map.Entry<Set<IAtom>,Collection<IAtom>> e : mapping.asMap().entrySet()) {
-            Set<IBond> bonds = new LinkedHashSet<>();
+            List<IBond> bonds = new ArrayList<>();
 
             IAtomContainer shared = mol.getBuilder().newInstance(IAtomContainer.class);
             for (IAtom atom : e.getKey())
                 shared.addAtom(atom);
             Point2d center = GeometryUtil.get2DCenter(shared);
 
             for (IBond bond : mol.bonds()) {
-                if (e.getKey().contains(bond.getBegin()) && e.getKey().contains(bond.getEnd())) {
+                if (e.getKey().contains(bond.getBegin()) &&
+                    e.getKey().contains(bond.getEnd())) {
                     bonds.add(bond);
                 }
             }
 
+            Collections.sort(bonds, new Comparator<IBond>() {
+                @Override
+                public int compare(IBond a, IBond b) {
+                    int atype = getPositionalRingBondPref(a, mol);
+                    int btype = getPositionalRingBondPref(b, mol);
+                    if (atype != btype)
+                        return Integer.compare(atype, btype);
+                    int aord  = a.getOrder().numeric();
+                    int bord  = b.getOrder().numeric();
+                    if (aord > 0 && bord > 0) {
+                        return Integer.compare(aord, bord);
+                    }
+                    return 0;
+                }
+            });
+
             if (bonds.size() >= e.getValue().size()) {
 
                 Iterator<IAtom> begIter = e.getValue().iterator();
@@ -2323,9 +2351,6 @@ private void placePositionalVariation(IAtomContainer mol) {
                     final IBond bond = bndIter.next();
                     final IAtom atom = begIter.next();
 
-                    if (numRingBonds(mol, bond.getBegin()) > 2 && numRingBonds(mol, bond.getEnd()) > 2)
-                        continue;
-
                     final Point2d newBegP = new Point2d(bond.getBegin().getPoint2d());
                     final Point2d newEndP = new Point2d(bond.getEnd().getPoint2d());
 
@@ -2351,7 +2376,7 @@ private void placePositionalVariation(IAtomContainer mol) {
                     newBegP.sub(bndXVec);
                     newEndP.sub(bndVec);
                     bndXVec.normalize();
-                    bndXVec.scale(3*bndStep);
+                    bndXVec.scale(4*bndStep);
                     newEndP.add(bndXVec);
 
                     int atomIdx = idxs.get(atom);