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 index 795fec04c8f..ebe1fcf331e 100644 --- a/tool/sdg/src/main/java/org/openscience/cdk/layout/RingPlacer.java +++ b/tool/sdg/src/main/java/org/openscience/cdk/layout/RingPlacer.java @@ -522,7 +522,8 @@ boolean completePartiallyPlacedRing(IRingSet rset, IRing ring, double bondLength atom.setFlag(CDKConstants.ISPLACED, true); AtomPlacer.copyPlaced(partiallyPlacedRing, ring); - if (partiallyPlacedRing.getAtomCount() > 0 && partiallyPlacedRing.getAtomCount() < ring.getAtomCount()) { + if (partiallyPlacedRing.getAtomCount() > 1 && + partiallyPlacedRing.getAtomCount() < ring.getAtomCount()) { placeConnectedRings(rset, partiallyPlacedRing, RingPlacer.FUSED, bondLength); placeConnectedRings(rset, partiallyPlacedRing, RingPlacer.BRIDGED, bondLength); placeConnectedRings(rset, partiallyPlacedRing, RingPlacer.SPIRO, bondLength); 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 index 3ae39d890fa..a9e489f6c22 100644 --- a/tool/sdg/src/main/java/org/openscience/cdk/layout/StructureDiagramGenerator.java +++ b/tool/sdg/src/main/java/org/openscience/cdk/layout/StructureDiagramGenerator.java @@ -249,25 +249,51 @@ public final void generateCoordinates(final IReaction reaction) throws CDKExcept } } + boolean aggresive = false; + if (!afix.isEmpty()) { - for (IBond bond : mol.bonds()) { - if (afix.containsKey(bond.getBegin()) && afix.containsKey(bond.getEnd())) { - // only fix acyclic bonds if the source atoms were also acyclic - if (!bond.isInRing()) { + if (aggresive) { + for (IBond bond : mol.bonds()) { + if (afix.containsKey(bond.getBegin()) && afix.containsKey(bond.getEnd())) { + // only fix acyclic bonds if the source atoms were also acyclic + if (!bond.isInRing()) { + IAtom srcBeg = afix.get(bond.getBegin()); + IAtom srcEnd = afix.get(bond.getEnd()); + for (IAtomContainer product : reaction.getProducts().atomContainers()) { + IBond srcBond = product.getBond(srcBeg, srcEnd); + if (srcBond != null) { + if (!srcBond.isInRing()) + bfix.add(bond); // safe to add + break; + } + } + } else { + bfix.add(bond); + } + } + } + } else { + for (IBond bond : mol.bonds()) { + if (afix.containsKey(bond.getBegin()) && afix.containsKey(bond.getEnd())) { + // only fix bonds that match their ring membership status IAtom srcBeg = afix.get(bond.getBegin()); IAtom srcEnd = afix.get(bond.getEnd()); for (IAtomContainer product : reaction.getProducts().atomContainers()) { IBond srcBond = product.getBond(srcBeg, srcEnd); if (srcBond != null) { - if (!srcBond.isInRing()) - bfix.add(bond); // safe to add + if (srcBond.isInRing() == bond.isInRing()) + bfix.add(bond); break; } } - } else { - bfix.add(bond); } } + // XXX: can do better + afix.clear(); + for (IBond bond : bfix) { + afix.put(bond.getBegin(), null); + afix.put(bond.getEnd(), null); + } } } @@ -621,7 +647,6 @@ private void seedLayout() throws CDKException { if (rset.getFlag(CDKConstants.ISPLACED)) { ringPlacer.placeRingSubstituents(rset, bondLength); } else { - List placed = new ArrayList<>(); List unplaced = new ArrayList<>(); @@ -2256,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 sgroups = mol.getProperty(CDKConstants.CTAB_SGROUPS); if (sgroups == null) @@ -2275,7 +2311,7 @@ private void placePositionalVariation(IAtomContainer mol) { idxs.put(atom, idxs.size()); for (Map.Entry,Collection> e : mapping.asMap().entrySet()) { - Set bonds = new LinkedHashSet<>(); + List bonds = new ArrayList<>(); IAtomContainer shared = mol.getBuilder().newInstance(IAtomContainer.class); for (IAtom atom : e.getKey()) @@ -2283,11 +2319,28 @@ private void placePositionalVariation(IAtomContainer mol) { 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() { + @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 begIter = e.getValue().iterator(); @@ -2298,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()); @@ -2326,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);