Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #403 from cdk/patch/delocalisedbonds
Looks good to me. (Chemists are a conservative lot :)
  • Loading branch information
egonw committed Dec 29, 2017
2 parents b571b88 + dec3405 commit dd8823a
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 52 deletions.
Expand Up @@ -164,8 +164,23 @@ public boolean visible(IAtom atom, List<IBond> bonds, RendererModel model) {
private static boolean isFourValent(IAtom atom, List<IBond> bonds) {
Integer valence = atom.getImplicitHydrogenCount();
if (valence == null) return true;
for (final IBond bond : bonds) {
valence += bond.getOrder().numeric();
if (atom.isAromatic()) {
boolean hasUnsetArom = false;
for (final IBond bond : bonds) {
if (bond.getOrder() == IBond.Order.UNSET && bond.isAromatic()) {
hasUnsetArom = true;
valence++;
} else {
valence += bond.getOrder().numeric();
}
}
// valence nudge, we're only dealing with neutral carbons here
// and if
if (hasUnsetArom)
valence++;
} else {
for (final IBond bond : bonds)
valence += bond.getOrder().numeric();
}
return valence == 4;
}
Expand Down
Expand Up @@ -220,13 +220,35 @@ public void alwaysDisplayCharges() {

a1.setPoint2d(new Point2d(0, 0));
a2.setPoint2d(new Point2d(0.5, -0.5));
a2.setPoint2d(new Point2d(1, 0));
a3.setPoint2d(new Point2d(1, 0));

IBond bond1 = new Bond(a1, a2, IBond.Order.DOUBLE);
IBond bond2 = new Bond(a2, a3, IBond.Order.SINGLE);

assertTrue(SymbolVisibility.iupacRecommendationsWithoutTerminalCarbon()
.visible(a1, Arrays.asList(bond1, bond2), new RendererModel()));
.visible(a1, Collections.singletonList(bond1), new RendererModel()));
}

@Test
public void delocalisedCarbons() {
IAtom a1 = new Atom("CH");
IAtom a2 = new Atom("CH");
IAtom a3 = new Atom("CH");

a1.setPoint2d(new Point2d(0, 0));
a2.setPoint2d(new Point2d(0.5, -0.5));
a3.setPoint2d(new Point2d(1, 0));

IBond bond1 = new Bond(a1, a2, IBond.Order.UNSET);
IBond bond2 = new Bond(a2, a3, IBond.Order.UNSET);
bond1.setIsAromatic(true);
bond2.setIsAromatic(true);
a1.setIsAromatic(true);
a2.setIsAromatic(true);
a3.setIsAromatic(true);

assertFalse(SymbolVisibility.iupacRecommendationsWithoutTerminalCarbon()
.visible(a2, Arrays.asList(bond1, bond2), new RendererModel()));
}

}
Expand Up @@ -26,7 +26,6 @@
package org.openscience.cdk.renderer.generators.standard;

import com.google.common.primitives.Ints;

import org.openscience.cdk.config.Elements;
import org.openscience.cdk.graph.Cycles;
import org.openscience.cdk.interfaces.IAtom;
Expand All @@ -53,7 +52,6 @@
import javax.vecmath.Point2d;
import javax.vecmath.Tuple2d;
import javax.vecmath.Vector2d;

import java.awt.Color;
import java.awt.Font;
import java.util.ArrayList;
Expand All @@ -65,6 +63,7 @@
import java.util.Map;

import static org.openscience.cdk.interfaces.IBond.Order.SINGLE;
import static org.openscience.cdk.interfaces.IBond.Order.UNSET;
import static org.openscience.cdk.interfaces.IBond.Stereo.NONE;
import static org.openscience.cdk.renderer.generators.BasicSceneGenerator.BondLength;
import static org.openscience.cdk.renderer.generators.standard.StandardGenerator.BondSeparation;
Expand Down Expand Up @@ -123,8 +122,9 @@ final class StandardBondGenerator {
private final Color foreground, annotationColor;
private final boolean fancyBoldWedges, fancyHashedWedges;
private final double annotationDistance, annotationScale;
private final Font font;
private final ElementGroup annotations;
private final Font font;
private final ElementGroup annotations;
private final boolean forceDelocalised;

/**
* Create a new standard bond generator for the provided structure (container) with the laid out
Expand Down Expand Up @@ -163,6 +163,7 @@ private StandardBondGenerator(IAtomContainer container, AtomSymbol[] symbols, Re
* (parameters.get(BondLength.class) / scale);
this.annotationScale = (1 / scale) * parameters.get(StandardGenerator.AnnotationFontScale.class);
this.annotationColor = parameters.get(StandardGenerator.AnnotationColor.class);
this.forceDelocalised = parameters.get(StandardGenerator.ForceDelocalisedBondDisplay.class);
this.font = font;

// foreground is based on the carbon color
Expand Down Expand Up @@ -212,17 +213,25 @@ IRenderingElement generate(IBond bond) {

switch (order) {
case SINGLE:
elem = generateSingleBond(bond, atom1, atom2);
if (bond.isAromatic() && forceDelocalised)
elem = generateDoubleBond(bond, true);
else
elem = generateSingleBond(bond, atom1, atom2);
break;
case DOUBLE:
elem = generateDoubleBond(bond);
elem = generateDoubleBond(bond,
bond.isAromatic() && forceDelocalised);
break;
case TRIPLE:
elem = generateTripleBond(bond, atom1, atom2);
break;
default:
// bond orders > 3 not supported
elem = generateDashedBond(atom1, atom2);
if (bond.isAromatic() && order == UNSET) {
elem = generateDoubleBond(bond, true);
} else {
// bond orders > 3 not supported
elem = generateDashedBond(atom1, atom2);
}
break;
}

Expand Down Expand Up @@ -599,9 +608,10 @@ IRenderingElement generateWavyBond(final IAtom from, final IAtom to) {
* Generates a double bond rendering element by deciding how best to display it.
*
* @param bond the bond to render
* @param dashed the second line should be dashed
* @return rendering element
*/
private IRenderingElement generateDoubleBond(IBond bond) {
private IRenderingElement generateDoubleBond(IBond bond, boolean dashed) {

final boolean cyclic = ringMap.containsKey(bond);

Expand Down Expand Up @@ -629,31 +639,37 @@ private IRenderingElement generateDoubleBond(IBond bond) {
atom2Bonds.remove(bond);

if (cyclic) {
// get the winding relative to the ring
final int wind1 = winding(atom1Bonds.get(0), bond);
final int wind2 = winding(bond, atom2Bonds.get(0));
if (wind1 > 0 && !hasDisplayedSymbol(atom1)) {
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bonds.get(0), atom2Bonds);
} else if (wind2 > 0 && !hasDisplayedSymbol(atom2)) {
return generateOffsetDoubleBond(bond, atom2, atom1, atom2Bonds.get(0), atom1Bonds);
} else if (!hasDisplayedSymbol(atom1)) {
// special case, offset line is drawn on the opposite side
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bonds.get(0), atom2Bonds, true);
} else if (!hasDisplayedSymbol(atom2)) {
// special case, offset line is drawn on the opposite side
return generateOffsetDoubleBond(bond, atom2, atom1, atom2Bonds.get(0), atom1Bonds, true);
if (wind1 > 0) {
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bonds.get(0), atom2Bonds, dashed);
} else if (wind2 > 0) {
return generateOffsetDoubleBond(bond, atom2, atom1, atom2Bonds.get(0), atom1Bonds, dashed);
} else {
return generateCenteredDoubleBond(bond, atom1, atom2, atom1Bonds, atom2Bonds);
// special case, offset line is drawn on the opposite side for
// when concave in macro cycle
//
// ---
// a --- b
// / \
// -- x x --
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bonds.get(0), atom2Bonds, true, dashed);
}
} else if (atom1Bonds.size() == 1 && !hasDisplayedSymbol(atom1) && (!hasDisplayedSymbol(atom2) || atom2Bonds.isEmpty())) {
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bonds.get(0), atom2Bonds);
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bonds.get(0), atom2Bonds, dashed);
} else if (atom2Bonds.size() == 1 && !hasDisplayedSymbol(atom2) && (!hasDisplayedSymbol(atom1) || atom1Bonds.isEmpty())) {
return generateOffsetDoubleBond(bond, atom2, atom1, atom2Bonds.get(0), atom1Bonds);
return generateOffsetDoubleBond(bond, atom2, atom1, atom2Bonds.get(0), atom1Bonds, dashed);
} else if (specialOffsetBondNextToWedge(atom1, atom1Bonds) && !hasDisplayedSymbol(atom1)) {
return generateOffsetDoubleBond(bond, atom1, atom2, selectPlainSingleBond(atom1Bonds), atom2Bonds);
return generateOffsetDoubleBond(bond, atom1, atom2, selectPlainSingleBond(atom1Bonds), atom2Bonds, dashed);
} else if (specialOffsetBondNextToWedge(atom2, atom2Bonds) && !hasDisplayedSymbol(atom2)) {
return generateOffsetDoubleBond(bond, atom2, atom1, selectPlainSingleBond(atom2Bonds), atom1Bonds);
return generateOffsetDoubleBond(bond, atom2, atom1, selectPlainSingleBond(atom2Bonds), atom1Bonds, dashed);
} else {
return generateCenteredDoubleBond(bond, atom1, atom2, atom1Bonds, atom2Bonds);
if (dashed) {
return generateDashedBond(atom1, atom2);
} else {
return generateCenteredDoubleBond(bond, atom1, atom2, atom1Bonds, atom2Bonds);
}
}
}

Expand Down Expand Up @@ -735,8 +751,8 @@ private boolean atWideEndOfWedge(final IAtom atom, final IBond bond) {
* @return the rendered bond element
*/
private IRenderingElement generateOffsetDoubleBond(IBond bond, IAtom atom1, IAtom atom2, IBond atom1Bond,
List<IBond> atom2Bonds) {
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bond, atom2Bonds, false);
List<IBond> atom2Bonds, boolean dashed) {
return generateOffsetDoubleBond(bond, atom1, atom2, atom1Bond, atom2Bonds, false, dashed);
}

/**
Expand All @@ -753,14 +769,15 @@ private IRenderingElement generateOffsetDoubleBond(IBond bond, IAtom atom1, IAto
* @return the rendered bond element
*/
private IRenderingElement generateOffsetDoubleBond(IBond bond, IAtom atom1, IAtom atom2, IBond atom1Bond,
List<IBond> atom2Bonds, boolean invert) {
List<IBond> atom2Bonds, boolean invert, boolean dashed) {

assert !hasDisplayedSymbol(atom1);
assert atom1Bond != null;

final Point2d atom1Point = atom1.getPoint2d();
final Point2d atom2Point = atom2.getPoint2d();

final Point2d atom1BackOffPoint = backOffPoint(atom1, atom2);
final Point2d atom2BackOffPoint = backOffPoint(atom2, atom1);

final Vector2d unit = newUnitVector(atom1Point, atom2Point);
Expand All @@ -783,15 +800,19 @@ private IRenderingElement generateOffsetDoubleBond(IBond bond, IAtom atom1, IAto

// the offset line isn't drawn the full length and is backed off more depending on the
// angle of adjacent bonds, see GR-1.10 in the IUPAC recommendations
double atom1Offset = adjacentLength(sum(reference, unit), perpendicular, separation);
double atom1Offset = 0;
double atom2Offset = 0;

// reference bond may be on the other side (invert specified) - the offset needs negating
if (dashed || !hasDisplayedSymbol(atom1)) {
atom1Offset = adjacentLength(sum(reference, unit), perpendicular, separation);
}

// reference bond may be on the other side (invert specified) - the offset needs negating
if (reference.dot(perpendicular) < 0) atom1Offset = -atom1Offset;

// the second atom may have zero or more bonds which we can use to get the offset
// we find the one which is closest to the perpendicular vector
if (!atom2Bonds.isEmpty() && !hasDisplayedSymbol(atom2)) {
if (!atom2Bonds.isEmpty() && (dashed || !hasDisplayedSymbol(atom2))) {
Vector2d closest = getNearestVector(perpendicular, atom2, atom2Bonds);
atom2Offset = adjacentLength(sum(closest, negate(unit)), perpendicular, separation);

Expand All @@ -806,9 +827,19 @@ private IRenderingElement generateOffsetDoubleBond(IBond bond, IAtom atom1, IAto

final ElementGroup group = new ElementGroup();

group.add(newLineElement(atom1Point, atom2BackOffPoint));
group.add(newLineElement(sum(sum(atom1Point, scale(perpendicular, separation)), scale(unit, atom1Offset)),
sum(sum(atom2BackOffPoint, scale(perpendicular, separation)), scale(unit, -atom2Offset))));
group.add(newLineElement(atom1BackOffPoint, atom2BackOffPoint));
if (dashed) {
Point2d beg = new Point2d(sum(atom1Point, scale(perpendicular, separation)));
Point2d end = new Point2d(sum(atom2Point, scale(perpendicular, separation)));
group.add(generateDashedBond(beg, end,
atom1Offset,
beg.distance(end) - atom2Offset));
} else {
Point2d beg = new Point2d(sum(atom1BackOffPoint, scale(perpendicular, separation)));
Point2d end = new Point2d(sum(atom2BackOffPoint, scale(perpendicular, separation)));
group.add(newLineElement(sum(beg, scale(unit, atom1Offset)),
sum(end, scale(unit, -atom2Offset))));
}

// add annotation label on the opposite side
String label = StandardGenerator.getAnnotationLabel(bond);
Expand Down Expand Up @@ -1102,26 +1133,20 @@ private void addAnnotation(IAtom atom1, IAtom atom2, String label, Vector2d perp
/**
* Generates a rendering element for displaying an 'unknown' bond type.
*
* @param from drawn from this atom
* @param to drawn to this atom
* @param fromPoint drawn from this point
* @param toPoint drawn to this point
* @param start only start drawing dashes after this point
* @param end stop drawing dashes after this point
* @return rendering of unknown bond
*/
IRenderingElement generateDashedBond(IAtom from, IAtom to) {

final Point2d fromPoint = from.getPoint2d();
final Point2d toPoint = to.getPoint2d();
IRenderingElement generateDashedBond(Point2d fromPoint, Point2d toPoint, double start, double end) {

final Vector2d unit = newUnitVector(fromPoint, toPoint);

final int nDashes = parameters.get(StandardGenerator.DashSection.class);

final double step = fromPoint.distance(toPoint) / ((3 * nDashes) - 2);

final double start = hasDisplayedSymbol(from) ? fromPoint.distance(backOffPoint(from, to))
: Double.NEGATIVE_INFINITY;
final double end = hasDisplayedSymbol(to) ? fromPoint.distance(backOffPoint(to, from))
: Double.POSITIVE_INFINITY;

ElementGroup group = new ElementGroup();

double distance = 0;
Expand Down Expand Up @@ -1151,6 +1176,18 @@ else if (distance > start && distance < end) {
return group;
}

IRenderingElement generateDashedBond(IAtom from, IAtom to) {
final Point2d fromPoint = from.getPoint2d();
final Point2d toPoint = to.getPoint2d();

final double start = hasDisplayedSymbol(from) ? fromPoint.distance(backOffPoint(from, to))
: Double.NEGATIVE_INFINITY;
final double end = hasDisplayedSymbol(to) ? fromPoint.distance(backOffPoint(to, from))
: Double.POSITIVE_INFINITY;

return generateDashedBond(fromPoint, toPoint, start, end);
}

/**
* Create a new line element between two points. The line has the specified stroke and
* foreground color.
Expand Down
Expand Up @@ -55,7 +55,6 @@
import java.awt.geom.Ellipse2D;
import java.awt.geom.Point2D;
import java.awt.geom.Rectangle2D;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -164,7 +163,8 @@ public static enum HighlightStyle {
fancyHashedWedges = new FancyHashedWedges(), highlighting = new Highlighting(),
glowWidth = new OuterGlowWidth(), annCol = new AnnotationColor(), annDist = new AnnotationDistance(),
annFontSize = new AnnotationFontScale(), sgroupBracketDepth = new SgroupBracketDepth(),
sgroupFontScale = new SgroupFontScale(), omitMajorIsotopes = new OmitMajorIsotopes();
sgroupFontScale = new SgroupFontScale(), omitMajorIsotopes = new OmitMajorIsotopes(),
forceDonuts = new ForceDelocalisedBondDisplay();

/**
* Create a new standard generator that utilises the specified font to display atom symbols.
Expand Down Expand Up @@ -559,7 +559,7 @@ public static IRenderingElement embedText(Font font, String text, Color color, d
public List<IGeneratorParameter<?>> getParameters() {
return Arrays.asList(atomColor, visibility, strokeRatio, separationRatio, wedgeRatio, marginRatio,
hatchSections, dashSections, waveSections, fancyBoldWedges, fancyHashedWedges, highlighting, glowWidth,
annCol, annDist, annFontSize, sgroupBracketDepth, sgroupFontScale, omitMajorIsotopes);
annCol, annDist, annFontSize, sgroupBracketDepth, sgroupFontScale, omitMajorIsotopes, forceDonuts);
}

static String getAnnotationLabel(IChemObject chemObject) {
Expand Down Expand Up @@ -1086,4 +1086,24 @@ public Boolean getDefault() {
return false;
}
}

/**
* Indicate delocalised/aromatic bonds should always be rendered, even when
* there is a valid Kekule structure. Delocalised bonds will either be
* rendered as a dashed bond to the side or as a circle/donut/life buoy
* inside small rings. This depiction is used by default when a bond does
* not have an order assigned (e.g. null/unset). Turning this option on
* means all delocalised bonds will be rendered this way.
* <br>
* <b>As recommended by IUPAC, their usage is discouraged and the Kekule
* representation is more clear.</b>
*/
public static final class ForceDelocalisedBondDisplay extends AbstractGeneratorParameter<Boolean> {

/**{@inheritDoc} */
@Override
public Boolean getDefault() {
return false;
}
}
}

0 comments on commit dd8823a

Please sign in to comment.