Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Patch/rxnlayout #357

Merged
merged 3 commits into from Aug 23, 2017
Merged

Patch/rxnlayout #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
284fed4
Resolves odd placement of some rings when one of the atoms is placed.
johnmay Aug 22, 2017
bdb7f0d
Less aggressive alignment of reaction parts.
johnmay Aug 22, 2017
a0e61af
Fix problem with placing positional variation, rather than skipping t…
johnmay Aug 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion tool/sdg/src/main/java/org/openscience/cdk/layout/RingPlacer.java
View file
Open in desktop
Expand Up @@ -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);
Expand Down
82 changes: 66 additions & 16 deletions tool/sdg/src/main/java/org/openscience/cdk/layout/StructureDiagramGenerator.java
View file
Open in desktop
Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -621,7 +647,6 @@ private void seedLayout() throws CDKException {
if (rset.getFlag(CDKConstants.ISPLACED)) {
ringPlacer.placeRingSubstituents(rset, bondLength);
} else {

List<IRing> placed = new ArrayList<>();
List<IRing> unplaced = new ArrayList<>();

Expand Down Expand Up @@ -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<Sgroup> sgroups = mol.getProperty(CDKConstants.CTAB_SGROUPS);
if (sgroups == null)
Expand All @@ -2275,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();
Expand All @@ -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());

Expand All @@ -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);
Expand Down