- Expose multicolor highlights to MinimalLib (#7787)

2026-06-03 21:44:30 +08:00 · 2024-09-03 21:03:40 +02:00
parent d8e3eab165
commit e4f4644a89
6 changed files with 210 additions and 33 deletions
--- a/Code/GraphMol/MolDraw2D/DrawMolMCHCircleAndLine.cpp
+++ b/Code/GraphMol/MolDraw2D/DrawMolMCHCircleAndLine.cpp
@@ -203,6 +203,7 @@ void DrawMolMCHCircleAndLine::calcSymbolEllipse(unsigned int atomIdx,
                                                Point2D &centre,
                                                double &xradius,
                                                double &yradius) const {
+  PRECONDITION(atomIdx < atCds_.size() && atomIdx < atomLabels_.size(), "")
  centre = atCds_[atomIdx];
  getAtomRadius(atomIdx, xradius, yradius);
  if (drawOptions_.atomHighlightsAreCircles || !atomLabels_[atomIdx] ||
--- a/Code/GraphMol/MolDraw2D/catch_tests.cpp
+++ b/Code/GraphMol/MolDraw2D/catch_tests.cpp
@@ -9984,3 +9984,50 @@ TEST_CASE("Github 7739 - Bad multi-coloured wedge") {
    check_file_hash(fileStem + "5.svg");
  }
 }
+
+TEST_CASE("idx out of bounds should not cause a segfault") {
+  auto m = "C"_smiles;
+  {
+    MolDraw2DSVG drawer(300, 300, -1, -1, NO_FREETYPE);
+    std::map<int, std::vector<DrawColour>> atomCols{{2, {DrawColour(0, 0, 0)}}};
+    std::map<int, std::vector<DrawColour>> bondCols;
+    std::map<int, double> atomRads{{2, 1.0}};
+    std::map<int, int> bondMults;
+    REQUIRE_THROWS_AS(
+        drawer.drawMoleculeWithHighlights(*m, "nocrash", atomCols, bondCols,
+                                          atomRads, bondMults),
+        Invar::Invariant);
+  }
+  {
+    MolDraw2DSVG drawer(300, 300, -1, -1, NO_FREETYPE);
+    std::map<int, std::vector<DrawColour>> atomCols;
+    std::map<int, std::vector<DrawColour>> bondCols{{2, {DrawColour(0, 0, 0)}}};
+    std::map<int, double> atomRads;
+    std::map<int, int> bondMults;
+    REQUIRE_THROWS_AS(
+        drawer.drawMoleculeWithHighlights(*m, "nocrash", atomCols, bondCols,
+                                          atomRads, bondMults),
+        Invar::Invariant);
+  }
+  {
+    MolDraw2DSVG drawer(300, 300, -1, -1, NO_FREETYPE);
+    std::map<int, std::vector<DrawColour>> atomCols{{0, {DrawColour(0, 0, 0)}}};
+    std::map<int, std::vector<DrawColour>> bondCols;
+    std::map<int, double> atomRads{{2, 1.0}};
+    std::map<int, int> bondMults;
+    REQUIRE_NOTHROW(
+        drawer.drawMoleculeWithHighlights(*m, "nocrash", atomCols, bondCols,
+                                          atomRads, bondMults));
+  }
+  {
+    MolDraw2DSVG drawer(300, 300, -1, -1, NO_FREETYPE);
+    std::map<int, std::vector<DrawColour>> atomCols;
+    std::map<int, std::vector<DrawColour>> bondCols{{0, {DrawColour(0, 0, 0)}}};
+    std::map<int, double> atomRads;
+    std::map<int, int> bondMults{{2, 10}};
+    REQUIRE_THROWS_AS(
+        drawer.drawMoleculeWithHighlights(*m, "nocrash", atomCols, bondCols,
+                                          atomRads, bondMults),
+        Invar::Invariant);
+  }
+}
--- a/Code/MinimalLib/cffi_test.c
+++ b/Code/MinimalLib/cffi_test.c
@@ -2295,7 +2295,8 @@ void test_assign_chiral_tags_from_mol_parity() {
  char *mpkl;
  size_t mpkl_size;
  char *smiles;
-  const char *artemisininCTAB = "68827\n\
+  const char *artemisininCTAB =
+      "68827\n\
  -OEChem-03262404452D\n\
 \n\
 20 23  0     1  0  0  0  0  0999 V2000\n\
@@ -2350,10 +2351,13 @@ M  END\n\
  assert(!strcmp(smiles, "CC1CCC2C(C)C(=O)OC3OC4(C)CCC1C32OO4"));
  free(smiles);
  free(mpkl);
-  mpkl = get_mol(artemisininCTAB, &mpkl_size, "{\"assignChiralTypesFromMolParity\":true}");
+  mpkl = get_mol(artemisininCTAB, &mpkl_size,
+                 "{\"assignChiralTypesFromMolParity\":true}");
  assert(mpkl);
  smiles = get_smiles(mpkl, mpkl_size, NULL);
-  assert(!strcmp(smiles, "C[C@@H]1CC[C@H]2[C@@H](C)C(=O)O[C@@H]3O[C@@]4(C)CC[C@@H]1[C@]32OO4"));
+  assert(!strcmp(
+      smiles,
+      "C[C@@H]1CC[C@H]2[C@@H](C)C(=O)O[C@@H]3O[C@@]4(C)CC[C@@H]1[C@]32OO4"));
  free(smiles);
  free(mpkl);
 }
@@ -2663,9 +2667,9 @@ M  END\n\
  char *mpkl;
  char *mpkl_copy;
  size_t mpkl_size;
-  const char *mb_rdkit_wedging;
-  const char *mb_rdkit_wedging_post_orig;
-  const char *mb_orig_wedging;
+  char *mb_rdkit_wedging;
+  char *mb_rdkit_wedging_post_orig;
+  char *mb_orig_wedging;
  mpkl = get_mol(mb, &mpkl_size, "");
  assert(mpkl && mpkl_size);
  mpkl_copy = malloc(mpkl_size);
@@ -2687,6 +2691,36 @@ M  END\n\
  free(mpkl_copy);
 }

+void test_multi_highlights() {
+  printf("--------------------------\n");
+  printf("  test_multi_highlights\n");
+  const char *smi =
+      "[H]c1cc2c(-c3ccnc(Nc4ccc(F)c(F)c4)n3)c(-c3cccc(C(F)(F)F)c3)nn2nc1C";
+  const char *details =
+      "{\"width\":250,\"height\":200,\"highlightAtomMultipleColors\":{\"15\":[[0.941,0.894,0.259]],\"17\":[[0,0.62,0.451]],\"21\":[[0.902,0.624,0]],\"22\":[[0.902,0.624,0]],\"23\":[[0.902,0.624,0]],\"24\":[[0.902,0.624,0]],\"25\":[[0.902,0.624,0]],\"26\":[[0.902,0.624,0]],\"27\":[[0.902,0.624,0]],\"28\":[[0.902,0.624,0]],\"29\":[[0.902,0.624,0]],\"30\":[[0.902,0.624,0]],\"35\":[[0.337,0.706,0.914]]},\"highlightBondMultipleColors\":{\"14\":[[0.941,0.894,0.259]],\"16\":[[0,0.62,0.451]],\"20\":[[0.902,0.624,0]],\"21\":[[0.902,0.624,0]],\"22\":[[0.902,0.624,0]],\"23\":[[0.902,0.624,0]],\"24\":[[0.902,0.624,0]],\"25\":[[0.902,0.624,0]],\"26\":[[0.902,0.624,0]],\"27\":[[0.902,0.624,0]],\"28\":[[0.902,0.624,0]],\"29\":[[0.902,0.624,0]],\"34\":[[0.337,0.706,0.914]],\"38\":[[0.902,0.624,0]]},\"highlightAtomRadii\":{\"15\":0.4,\"17\":0.4,\"21\":0.4,\"22\":0.4,\"23\":0.4,\"24\":0.4,\"25\":0.4,\"26\":0.4,\"27\":0.4,\"28\":0.4,\"29\":0.4,\"30\":0.4,\"35\":0.4},\"highlightLineWidthMultipliers\":{\"14\":2,\"16\":2,\"20\":2,\"21\":2,\"22\":2,\"23\":2,\"24\":2,\"25\":2,\"26\":2,\"27\":2,\"28\":2,\"29\":2,\"34\":2,\"38\":2}}";
+  const char *colors[] = {"#009E73", "#55B4E9", "#E69F00", "#EFE342", NULL};
+  char *mpkl;
+  char *svg_with_details;
+  char *svg_without_details;
+  size_t mpkl_size;
+  int i;
+  mpkl = get_mol(smi, &mpkl_size, "{\"removeHs\":false}");
+  assert(mpkl && mpkl_size);
+  svg_with_details = get_svg(mpkl, mpkl_size, details);
+  assert(strstr(svg_with_details, "ellipse"));
+  for (i = 0; colors[i]; ++i) {
+    assert(strstr(svg_with_details, colors[i]));
+  }
+  svg_without_details = get_svg(mpkl, mpkl_size, "");
+  assert(!strstr(svg_without_details, "ellipse"));
+  for (i = 0; colors[i]; ++i) {
+    assert(!strstr(svg_without_details, colors[i]));
+  }
+  free(svg_with_details);
+  free(svg_without_details);
+  free(mpkl);
+}
+
 int main() {
  enable_logging();
  char *vers = version();
@@ -2721,5 +2755,6 @@ int main() {
  test_smiles_smarts_params();
  test_wedged_bond_atropisomer();
  test_get_molblock_use_molblock_wedging();
+  test_multi_highlights();
  return 0;
 }
--- a/Code/MinimalLib/common.h
+++ b/Code/MinimalLib/common.h
@@ -354,6 +354,41 @@ std::string parse_rgba_array(const rj::Value &val, DrawColour &color,
  return "";
 }

+std::string parse_highlight_multi_colors(
+    const rj::Document &doc, std::map<int, std::vector<DrawColour>> &colorMap,
+    const std::string &keyName, bool &haveMultiColors) {
+  const auto it = doc.FindMember(keyName.c_str());
+  haveMultiColors = (it != doc.MemberEnd());
+  if (!haveMultiColors) {
+    return "";
+  }
+  if (!it->value.IsObject()) {
+    return "JSON contains '" + keyName + "' field, but it is not an object";
+  }
+  for (const auto &entry : it->value.GetObject()) {
+    int idx = std::atoi(entry.name.GetString());
+    auto &drawColorVect = colorMap[idx];
+    if (entry.value.IsArray()) {
+      const auto &arr = entry.value.GetArray();
+      if (std::all_of(arr.begin(), arr.end(),
+                      [](const auto &item) { return item.IsArray(); })) {
+        for (const auto &item : arr) {
+          DrawColour color;
+          auto problems = parse_rgba_array(item, color, keyName);
+          if (!problems.empty()) {
+            return problems;
+          }
+          drawColorVect.push_back(std::move(color));
+        }
+        continue;
+      }
+    }
+    return "JSON contains '" + keyName +
+           "' field, but it is not an array of R,G,B[,A] arrays";
+  }
+  return "";
+}
+
 std::string parse_highlight_colors(const rj::Document &doc,
                                   std::map<int, DrawColour> &colorMap,
                                   const std::string &keyName) {
@@ -441,19 +476,48 @@ std::string process_mol_details(const std::string &details,
  if (!problems.empty()) {
    return problems;
  }
-
-  problems = parse_highlight_colors(doc, molDrawingDetails.atomMap,
-                                    "highlightAtomColors");
+  bool haveAtomMultiColors;
+  problems = parse_highlight_multi_colors(doc, molDrawingDetails.atomMultiMap,
+                                          "highlightAtomMultipleColors",
+                                          haveAtomMultiColors);
  if (!problems.empty()) {
    return problems;
  }
-
-  problems = parse_highlight_colors(doc, molDrawingDetails.bondMap,
-                                    "highlightBondColors");
+  bool haveBondMultiColors;
+  problems = parse_highlight_multi_colors(doc, molDrawingDetails.bondMultiMap,
+                                          "highlightBondMultipleColors",
+                                          haveBondMultiColors);
  if (!problems.empty()) {
    return problems;
  }
-
+  if (haveAtomMultiColors || haveBondMultiColors) {
+    const auto lineWidthMultiplierMapit =
+        doc.FindMember("highlightLineWidthMultipliers");
+    if (lineWidthMultiplierMapit != doc.MemberEnd()) {
+      if (!lineWidthMultiplierMapit->value.IsObject()) {
+        return "JSON contains 'highlightLineWidthMultipliers' field, but it is not an object";
+      }
+      for (const auto &entry : lineWidthMultiplierMapit->value.GetObject()) {
+        if (!entry.value.IsInt()) {
+          return "JSON contains 'highlightLineWidthMultipliers' field, but the multipliers"
+                 "are not ints";
+        }
+        int idx = std::atoi(entry.name.GetString());
+        molDrawingDetails.lineWidthMultiplierMap[idx] = entry.value.GetInt();
+      }
+    }
+  } else {
+    problems = parse_highlight_colors(doc, molDrawingDetails.atomMap,
+                                      "highlightAtomColors");
+    if (!problems.empty()) {
+      return problems;
+    }
+    problems = parse_highlight_colors(doc, molDrawingDetails.bondMap,
+                                      "highlightBondColors");
+    if (!problems.empty()) {
+      return problems;
+    }
+  }
  const auto radiiMapit = doc.FindMember("highlightAtomRadii");
  if (radiiMapit != doc.MemberEnd()) {
    if (!radiiMapit->value.IsObject()) {
--- a/Code/MinimalLib/common_defs.h
+++ b/Code/MinimalLib/common_defs.h
@@ -39,7 +39,10 @@ struct DrawingDetails {
 struct MolDrawingDetails : public DrawingDetails {
  std::map<int, DrawColour> atomMap;
  std::map<int, DrawColour> bondMap;
+  std::map<int, std::vector<DrawColour>> atomMultiMap;
+  std::map<int, std::vector<DrawColour>> bondMultiMap;
  std::map<int, double> radiiMap;
+  std::map<int, int> lineWidthMultiplierMap;
 };

 struct RxnDrawingDetails : public DrawingDetails {
@@ -87,28 +90,41 @@ class DrawerFromDetails {
    }
    initDrawer(molDrawingDetails);
    const ROMol *molPtr = &mol;
-    std::unique_ptr<ROMol> origWedgingMol;
-    if (molDrawingDetails.useMolBlockWedging) {
-      origWedgingMol.reset(new ROMol(mol));
-      Chirality::reapplyMolBlockWedging(*origWedgingMol);
-      molPtr = origWedgingMol.get();
-      drawer().drawOptions().useMolBlockWedging = false;
+    std::unique_ptr<RWMol> drawnMol;
+    bool haveMultiMap = (!molDrawingDetails.atomMultiMap.empty() ||
+                         !molDrawingDetails.bondMultiMap.empty());
+    if (molDrawingDetails.useMolBlockWedging || haveMultiMap) {
+      drawnMol.reset(new RWMol(mol));
+      molPtr = static_cast<ROMol *>(drawnMol.get());
+      if (molDrawingDetails.useMolBlockWedging) {
+        Chirality::reapplyMolBlockWedging(*drawnMol);
+        drawer().drawOptions().useMolBlockWedging = false;
+      }
    }
    drawer().setOffset(molDrawingDetails.offsetx, molDrawingDetails.offsety);
-
-    MolDraw2DUtils::prepareAndDrawMolecule(
-        drawer(), *molPtr, molDrawingDetails.legend, &molDrawingDetails.atomIds,
-        &molDrawingDetails.bondIds,
-        molDrawingDetails.atomMap.empty() ? nullptr
-                                          : &molDrawingDetails.atomMap,
-        molDrawingDetails.bondMap.empty() ? nullptr
-                                          : &molDrawingDetails.bondMap,
-        molDrawingDetails.radiiMap.empty() ? nullptr
-                                           : &molDrawingDetails.radiiMap,
-        -1, molDrawingDetails.kekulize, molDrawingDetails.addChiralHs,
-        molDrawingDetails.wedgeBonds, molDrawingDetails.forceCoords,
-        molDrawingDetails.wavyBonds);
-
+    if (!haveMultiMap) {
+      MolDraw2DUtils::prepareAndDrawMolecule(
+          drawer(), *molPtr, molDrawingDetails.legend,
+          &molDrawingDetails.atomIds, &molDrawingDetails.bondIds,
+          molDrawingDetails.atomMap.empty() ? nullptr
+                                            : &molDrawingDetails.atomMap,
+          molDrawingDetails.bondMap.empty() ? nullptr
+                                            : &molDrawingDetails.bondMap,
+          molDrawingDetails.radiiMap.empty() ? nullptr
+                                             : &molDrawingDetails.radiiMap,
+          -1, molDrawingDetails.kekulize, molDrawingDetails.addChiralHs,
+          molDrawingDetails.wedgeBonds, molDrawingDetails.forceCoords,
+          molDrawingDetails.wavyBonds);
+    } else {
+      MolDraw2DUtils::prepareMolForDrawing(
+          *drawnMol, molDrawingDetails.kekulize, molDrawingDetails.addChiralHs,
+          molDrawingDetails.wedgeBonds, molDrawingDetails.forceCoords,
+          molDrawingDetails.wavyBonds);
+      drawer().drawMoleculeWithHighlights(
+          *drawnMol, molDrawingDetails.legend, molDrawingDetails.atomMultiMap,
+          molDrawingDetails.bondMultiMap, molDrawingDetails.radiiMap,
+          molDrawingDetails.lineWidthMultiplierMap);
+    }
    return finalizeDrawing();
  }
  std::string draw_rxn(const ChemicalReaction &rxn) {
--- a/Code/MinimalLib/tests/tests.js
+++ b/Code/MinimalLib/tests/tests.js
@@ -3176,6 +3176,19 @@ function test_get_mol_copy() {
    }
 }

+function test_multi_highlights() {
+    const mol = RDKitModule.get_mol('[H]c1cc2c(-c3ccnc(Nc4ccc(F)c(F)c4)n3)c(-c3cccc(C(F)(F)F)c3)nn2nc1C', JSON.stringify({removeHs: false}));
+    const details = '{"width":250,"height":200,"highlightAtomMultipleColors":{"15":[[0.941,0.894,0.259]],"17":[[0,0.62,0.451]],"21":[[0.902,0.624,0]],"22":[[0.902,0.624,0]],"23":[[0.902,0.624,0]],"24":[[0.902,0.624,0]],"25":[[0.902,0.624,0]],"26":[[0.902,0.624,0]],"27":[[0.902,0.624,0]],"28":[[0.902,0.624,0]],"29":[[0.902,0.624,0]],"30":[[0.902,0.624,0]],"35":[[0.337,0.706,0.914]]},"highlightBondMultipleColors":{"14":[[0.941,0.894,0.259]],"16":[[0,0.62,0.451]],"20":[[0.902,0.624,0]],"21":[[0.902,0.624,0]],"22":[[0.902,0.624,0]],"23":[[0.902,0.624,0]],"24":[[0.902,0.624,0]],"25":[[0.902,0.624,0]],"26":[[0.902,0.624,0]],"27":[[0.902,0.624,0]],"28":[[0.902,0.624,0]],"29":[[0.902,0.624,0]],"34":[[0.337,0.706,0.914]],"38":[[0.902,0.624,0]]},"highlightAtomRadii":{"15":0.4,"17":0.4,"21":0.4,"22":0.4,"23":0.4,"24":0.4,"25":0.4,"26":0.4,"27":0.4,"28":0.4,"29":0.4,"30":0.4,"35":0.4},"highlightLineWidthMultipliers":{"14":2,"16":2,"20":2,"21":2,"22":2,"23":2,"24":2,"25":2,"26":2,"27":2,"28":2,"29":2,"34":2,"38":2}}';
+    const svgWithDetails = mol.get_svg_with_highlights(details);
+    assert(svgWithDetails.includes('ellipse'));
+    const COLORS = ['#009E73', '#55B4E9', '#E69F00', '#EFE342'];
+    assert(COLORS.every((color) => svgWithDetails.includes(color)));
+    const svgWithOutDetails = mol.get_svg_with_highlights('');
+    assert(!svgWithOutDetails.includes('ellipse'));
+    assert(!COLORS.some((color) => svgWithOutDetails.includes(color)));
+    mol.delete();
+}
+
 initRDKitModule().then(function(instance) {
    var done = {};
    const waitAllTestsFinished = () => {
@@ -3257,6 +3270,7 @@ initRDKitModule().then(function(instance) {
    test_assign_chiral_tags_from_mol_parity();
    test_make_dummies_queries();
    test_get_mol_copy();
+    test_multi_highlights();
    waitAllTestsFinished().then(() =>
        console.log("Tests finished successfully")
    );