diff --git a/Code/GraphMol/MolDraw2D/DrawMol.cpp b/Code/GraphMol/MolDraw2D/DrawMol.cpp index 82824583c..72d473873 100644 --- a/Code/GraphMol/MolDraw2D/DrawMol.cpp +++ b/Code/GraphMol/MolDraw2D/DrawMol.cpp @@ -2042,7 +2042,8 @@ void DrawMol::makeBondHighlightLines(double lineWidth, double scale) { // These effects can be seen in bond_highlights_8.svg produced // by catch_tests.cpp. DrawColour col = getHighlightBondColour( - bond->getIdx(), drawOptions_, highlightBonds_, highlightBondMap_); + bond, drawOptions_, highlightBonds_, highlightBondMap_, + highlightAtoms_, highlightAtomMap_); std::vector thisHighNbrs; std::vector nbrHighNbrs; auto nbr = drawMol_->getAtomWithIdx(nbrIdx); @@ -3213,7 +3214,8 @@ DrawColour DrawMol::getColour(int atom_idx) const { nbr->getIdx()) != highlightBonds_.end() || highlightBondMap_.find(nbr->getIdx()) != highlightBondMap_.end()) { DrawColour hc = getHighlightBondColour( - nbr->getIdx(), drawOptions_, highlightBonds_, highlightBondMap_); + nbr, drawOptions_, highlightBonds_, highlightBondMap_, + highlightAtoms_, highlightAtomMap_); if (!highCol) { highCol.reset(new DrawColour(hc)); } else { @@ -3322,15 +3324,36 @@ DrawColour getColourByAtomicNum(int atomic_num, // **************************************************************************** DrawColour getHighlightBondColour( - int bondIdx, const MolDrawOptions &drawOptions, + const Bond *bond, const MolDrawOptions &drawOptions, const std::vector &highlightBonds, - const std::map &highlightBondMap) { + const std::map &highlightBondMap, + const std::vector &highlightAtoms, + const std::map &highlightAtomMap) { + PRECONDITION(bond, "no bond provided"); + RDUNUSED_PARAM(highlightAtoms); + DrawColour col(0.0, 0.0, 0.0); - if (std::find(highlightBonds.begin(), highlightBonds.end(), bondIdx) != + if (std::find(highlightBonds.begin(), highlightBonds.end(), bond->getIdx()) != highlightBonds.end()) { col = drawOptions.highlightColour; - if (highlightBondMap.find(bondIdx) != highlightBondMap.end()) { - col = highlightBondMap.find(bondIdx)->second; + if (highlightBondMap.find(bond->getIdx()) != highlightBondMap.end()) { + col = highlightBondMap.find(bond->getIdx())->second; + } else { + // the highlight color of the bond is not explicitly provided. What about + // the highlight colors of the begin/end atoms? Ideally these will both be + // the same, but we want to set the coloring even if that's not the + // case, so we'll use: + // - begin atom color if that is set + // - end atom color if that is set + // - the default highlight color otherwise + if (highlightAtomMap.find(bond->getBeginAtomIdx()) != + highlightAtomMap.end()) { + col = highlightAtomMap.find(bond->getBeginAtomIdx())->second; + + } else if (highlightAtomMap.find(bond->getEndAtomIdx()) != + highlightAtomMap.end()) { + col = highlightAtomMap.find(bond->getEndAtomIdx())->second; + } } } return col; diff --git a/Code/GraphMol/MolDraw2D/DrawMol.h b/Code/GraphMol/MolDraw2D/DrawMol.h index 35765ed61..af6451a05 100644 --- a/Code/GraphMol/MolDraw2D/DrawMol.h +++ b/Code/GraphMol/MolDraw2D/DrawMol.h @@ -309,9 +309,11 @@ std::string getAtomListText(const Atom &atom); DrawColour getColourByAtomicNum(int atomicNum, const MolDrawOptions &drawOptions); DrawColour getHighlightBondColour( - int bondIdx, const MolDrawOptions &drawOptions, + const Bond *bond, const MolDrawOptions &drawOptions, const std::vector &highlightBonds, - const std::map &highlightBondMap); + const std::map &highlightBondMap, + const std::vector &highlightAtoms, + const std::map &highlightAtomMap); double getHighlightBondWidth( const MolDrawOptions &drawOptions, int bond_idx, const std::map *highlight_linewidth_multipliers); diff --git a/Code/GraphMol/MolDraw2D/Wrap/testMolDraw2D.py b/Code/GraphMol/MolDraw2D/Wrap/testMolDraw2D.py index ace424d91..23156c6da 100644 --- a/Code/GraphMol/MolDraw2D/Wrap/testMolDraw2D.py +++ b/Code/GraphMol/MolDraw2D/Wrap/testMolDraw2D.py @@ -186,7 +186,7 @@ M END""") drawer.FinishDrawing() svg = drawer.GetDrawingText() # 4 molecules, 6 bonds each: - re_str = r"path class='bond-\d+ atom-\d+ atom-\d+' d='M \d+.\d+,\d+.\d+ L \d+.\d+,\d+.\d+ L \d+.\d+,\d+.\d+ L \d+.\d+,\d+.\d+ Z' style='fill:#FF7F7F;" + re_str = r"path class='bond-\d+ atom-\d+ atom-\d+' d='M \d+.\d+,\d+.\d+ L \d+.\d+,\d+.\d+ L \d+.\d+,\d+.\d+ L \d+.\d+,\d+.\d+ Z' style='fill:" patt = re.compile(re_str) self.assertEqual(len(patt.findall(svg)), 24) # 4 molecules, one atom each: diff --git a/Code/GraphMol/MolDraw2D/catch_tests.cpp b/Code/GraphMol/MolDraw2D/catch_tests.cpp index 3f190dfff..bbcdbe368 100644 --- a/Code/GraphMol/MolDraw2D/catch_tests.cpp +++ b/Code/GraphMol/MolDraw2D/catch_tests.cpp @@ -5992,4 +5992,87 @@ M END check_file_hash(nameBase + ".svg"); } } -} \ No newline at end of file +} + +TEST_CASE( + "Github5704: set bond highlight color when atom highlight color changes") { + std::string nameBase = "test_github5704"; + + auto m = + "CCCO |(-1.97961,-0.1365,;-0.599379,0.450827,;0.599379,-0.450827,;1.97961,0.1365,)|"_smiles; + REQUIRE(m); + std::regex redline(R"RE( aids{0, 1, 2}; + drawer.drawMolecule(*m, "red bond highlight", &aids); + drawer.finishDrawing(); + std::string text = drawer.getDrawingText(); + + std::smatch rematch; + CHECK(std::regex_search(text, rematch, redline)); + CHECK(!std::regex_search(text, rematch, blueline)); + + std::ofstream outs(nameBase + "_1.svg"); + outs << text; + outs.flush(); + outs.close(); + } + + SECTION("both ends specified") { + MolDraw2DSVG drawer(300, 300, 300, 300, NO_FREETYPE); + std::vector aids{0, 1, 2}; + std::map acolors{ + {0, {.3, .3, 1}}, {1, {.3, .3, 1}}, {2, {.3, .3, 1}}}; + drawer.drawMolecule(*m, "blue bond highlight", &aids, &acolors); + drawer.finishDrawing(); + std::string text = drawer.getDrawingText(); + + std::smatch rematch; + CHECK(!std::regex_search(text, rematch, redline)); + CHECK(std::regex_search(text, rematch, blueline)); + + std::ofstream outs(nameBase + "_2.svg"); + outs << text; + outs.flush(); + outs.close(); + } + + SECTION("color just on begin") { + MolDraw2DSVG drawer(300, 300, 300, 300, NO_FREETYPE); + std::vector aids{0, 1}; + std::map acolors{{0, {.3, .3, 1}}}; + drawer.drawMolecule(*m, "blue bond highlight", &aids, &acolors); + drawer.finishDrawing(); + std::string text = drawer.getDrawingText(); + + std::smatch rematch; + CHECK(!std::regex_search(text, rematch, redline)); + CHECK(std::regex_search(text, rematch, blueline)); + + std::ofstream outs(nameBase + "_3.svg"); + outs << text; + outs.flush(); + outs.close(); + } + SECTION("color just on end") { + MolDraw2DSVG drawer(300, 300, 300, 300, NO_FREETYPE); + std::vector aids{0, 1}; + std::map acolors{{1, {.3, .3, 1}}}; + drawer.drawMolecule(*m, "blue bond highlight", &aids, &acolors); + drawer.finishDrawing(); + std::string text = drawer.getDrawingText(); + + std::smatch rematch; + + CHECK(!std::regex_search(text, rematch, redline)); + CHECK(std::regex_search(text, rematch, blueline)); + + std::ofstream outs(nameBase + "_4.svg"); + outs << text; + outs.flush(); + outs.close(); + } +} diff --git a/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/Chemv2Tests.java b/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/Chemv2Tests.java index 9e8795849..87381ccea 100644 --- a/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/Chemv2Tests.java +++ b/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/Chemv2Tests.java @@ -467,7 +467,9 @@ public class Chemv2Tests extends GraphMolTest { assertTrue(svg.indexOf("fill:#FF00FF;") > -1); assertTrue(svg.indexOf("fill:#00FFFF;") > -1); // default line color: - assertTrue(svg.indexOf("stroke:#FF7F7F;") > -1); + assertTrue(svg.indexOf("stroke:#FF7F7F;") == -1); + assertTrue(svg.indexOf("stroke:#FFFF00;") > -1); + assertTrue(svg.indexOf("stroke:#FF00FF;") > -1); } @Test