From 91a92e4835bfc9a5eff76eb6f0af5159d9d6714f Mon Sep 17 00:00:00 2001 From: John May Date: Wed, 5 Oct 2016 12:15:35 +0100 Subject: [PATCH] Tweaks to disconnected component placement to try and avoid overlaps. --- .../cdk/layout/StructureDiagramGenerator.java | 162 +++++++++++++++++- .../layout/StructureDiagramGeneratorTest.java | 25 +++ 2 files changed, 185 insertions(+), 2 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 index af096d951a9..14a8bcc8f05 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 @@ -27,6 +27,7 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; import org.openscience.cdk.CDKConstants; +import org.openscience.cdk.config.Elements; import org.openscience.cdk.exception.CDKException; import org.openscience.cdk.geometry.GeometryUtil; import org.openscience.cdk.graph.ConnectedComponents; @@ -54,9 +55,9 @@ import org.openscience.cdk.tools.ILoggingTool; import org.openscience.cdk.tools.LoggingToolFactory; import org.openscience.cdk.tools.manipulator.AtomContainerManipulator; -import org.openscience.cdk.tools.manipulator.AtomContainerSetManipulator; import org.openscience.cdk.tools.manipulator.ReactionManipulator; import org.openscience.cdk.tools.manipulator.RingSetManipulator; +import uk.ac.ebi.beam.Element; import javax.vecmath.Point2d; import javax.vecmath.Vector2d; @@ -907,6 +908,88 @@ private static int countAlignedBonds(IAtomContainer mol) { return count; } + private final double adjustForHydrogen(IAtom atom, IAtomContainer mol) { + Integer hcnt = atom.getImplicitHydrogenCount(); + if (hcnt == null || hcnt == 0) + return 0; + List bonds = mol.getConnectedBondsList(atom); + + int pos = 0; // right + + // isolated atoms, HCl vs NH4+ etc + if (bonds.isEmpty()) { + Elements elem = Elements.ofNumber(atom.getAtomicNumber()); + // see HydrogenPosition for canonical list + switch (elem) { + case Oxygen: + case Sulfur: + case Selenium: + case Tellurium: + case Fluorine: + case Chlorine: + case Bromine: + case Iodine: + pos = -1; // left + break; + default: + pos = +1; // right + break; + } + } else if (bonds.size() == 1) { + IAtom other = bonds.get(0).getConnectedAtom(atom); + double deltaX = atom.getPoint2d().x - other.getPoint2d().x; + if (Math.abs(deltaX) > 0.05) + pos = (int) Math.signum(deltaX); + } + return pos * (bondLength/2); + } + + /** + * Similar to the method {@link GeometryUtil#getMinMax(IAtomContainer)} but considers + * heteroatoms with hydrogens. + * + * @param mol molecule + * @return the min/max x and y bounds + */ + private final double[] getAprxBounds(IAtomContainer mol) { + double maxX = -Double.MAX_VALUE; + double maxY = -Double.MAX_VALUE; + double minX = Double.MAX_VALUE; + double minY = Double.MAX_VALUE; + IAtom[] boundedAtoms = new IAtom[4]; + for (int i = 0; i < mol.getAtomCount(); i++) { + IAtom atom = mol.getAtom(i); + if (atom.getPoint2d() != null) { + if (atom.getPoint2d().x < minX) { + minX = atom.getPoint2d().x; + boundedAtoms[0] = atom; + } + if (atom.getPoint2d().y < minY) { + minY = atom.getPoint2d().y; + boundedAtoms[1] = atom; + } + if (atom.getPoint2d().x > maxX) { + maxX = atom.getPoint2d().x; + boundedAtoms[2] = atom; + } + if (atom.getPoint2d().y > maxY) { + maxY = atom.getPoint2d().y; + boundedAtoms[3] = atom; + } + } + } + double[] minmax = new double[4]; + minmax[0] = minX; + minmax[1] = minY; + minmax[2] = maxX; + minmax[3] = maxY; + double minXAdjust = adjustForHydrogen(boundedAtoms[0], mol); + double maxXAdjust = adjustForHydrogen(boundedAtoms[1], mol); + if (minXAdjust < 0) minmax[0] += minXAdjust; + if (maxXAdjust > 0) minmax[1] += maxXAdjust; + return minmax; + } + private void generateFragmentCoordinates(IAtomContainer mol, List frags) throws CDKException { final List ionicBonds = makeIonicBonds(frags); @@ -934,7 +1017,8 @@ private void generateFragmentCoordinates(IAtomContainer mol, List ionicBonds, IAtomContainer fragment) { + + final IChemObjectBuilder bldr = fragment.getBuilder(); + + if (ionicBonds.isEmpty()) + return; + + IAtomContainer newfrag = bldr.newInstance(IAtomContainer.class); + IAtom[] atoms = new IAtom[fragment.getAtomCount()]; + for (int i = 0; i < atoms.length; i++) + atoms[i] = fragment.getAtom(i); + newfrag.setAtoms(atoms); + + for (IBond bond : fragment.bonds()) { + if (!ionicBonds.contains(bond)) { + newfrag.addBond(bond); + } else { + Integer numBegIonic = bond.getAtom(0).getProperty("ionicDegree"); + Integer numEndIonic = bond.getAtom(1).getProperty("ionicDegree"); + if (numBegIonic == null) numBegIonic = 0; + if (numEndIonic == null) numEndIonic = 0; + numBegIonic++; + numEndIonic++; + bond.getAtom(0).setProperty("ionicDegree", numBegIonic); + bond.getAtom(1).setProperty("ionicDegree", numEndIonic); + } + } + + if (newfrag.getBondCount() == fragment.getBondCount()) + return; + + IAtomContainerSet subfragments = ConnectivityChecker.partitionIntoMolecules(newfrag); + Map atomToFrag = new HashMap<>(); + + // index atom->fragment + for (IAtomContainer subfragment : subfragments.atomContainers()) + for (IAtom atom : subfragment.atoms()) + atomToFrag.put(atom, subfragment); + + for (IBond bond : ionicBonds) { + IAtom beg = bond.getAtom(0); + IAtom end = bond.getAtom(1); + + // select which bond to stretch from + Integer numBegIonic = bond.getAtom(0).getProperty("ionicDegree"); + Integer numEndIonic = bond.getAtom(1).getProperty("ionicDegree"); + if (numBegIonic == null || numEndIonic == null) + continue; + if (numBegIonic > numEndIonic) { + IAtom tmp = beg; + beg = end; + end = tmp; + } else if (numBegIonic == numEndIonic && numBegIonic > 1) { + // can't stretch these + continue; + } + + IAtomContainer begFrag = atomToFrag.get(beg); + IAtomContainer endFrags = bldr.newInstance(IAtomContainer.class); + if (begFrag == null) + continue; + for (IAtomContainer mol : subfragments.atomContainers()) { + if (mol != begFrag) + endFrags.add(mol); + } + double dx = end.getPoint2d().x - beg.getPoint2d().x; + double dy = end.getPoint2d().y - beg.getPoint2d().y; + Vector2d bondVec = new Vector2d(dx, dy); + bondVec.normalize(); + bondVec.scale(bondLength/2); // 1.5 bond length + GeometryUtil.translate2D(endFrags, bondVec); + } + } + /** * Property to cache the charge of a fragment. */ diff --git a/tool/sdg/src/test/java/org/openscience/cdk/layout/StructureDiagramGeneratorTest.java b/tool/sdg/src/test/java/org/openscience/cdk/layout/StructureDiagramGeneratorTest.java index f8e9bf5183e..40ca3aa68c9 100644 --- a/tool/sdg/src/test/java/org/openscience/cdk/layout/StructureDiagramGeneratorTest.java +++ b/tool/sdg/src/test/java/org/openscience/cdk/layout/StructureDiagramGeneratorTest.java @@ -67,6 +67,7 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.closeTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.lessThan; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -1215,4 +1216,28 @@ public void disconnectedMultigroupPlacement() throws Exception { 0.01); } } + + /** + * These molecules are laid out 'H2N=NH2.H2N=NH2', ensure we give them more space than + * usual (bond length) + */ + @Test + public void dihydroazine() throws Exception { + SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance()); + IAtomContainer mol = smipar.parseSmiles("N=N.N=N"); + layout(mol); + assertThat(mol.getAtom(2).getPoint2d().x - mol.getAtom(1).getPoint2d().x, + is(greaterThan(SDG.getBondLength()))); + + } + + @Test + public void NH4OH() throws Exception { + SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance()); + IAtomContainer mol = smipar.parseSmiles("[NH4+].[OH-]"); + layout(mol); + assertThat(mol.getAtom(1).getPoint2d().x - mol.getAtom(0).getPoint2d().x, + is(greaterThan(SDG.getBondLength()))); + + } }