Fix layout of reaction components in reaction drawing. (#9302)

* Fix layout of reaction components in reaction drawing.

* <sigh>

---------

Co-authored-by: Greg Landrum <greg.landrum@gmail.com>
This commit is contained in:
David Cosgrove
2026-05-27 17:23:17 +01:00
committed by GitHub
parent ce08c344e8
commit 71e7775d35
2 changed files with 73 additions and 37 deletions

View File

@@ -1116,11 +1116,13 @@ void DrawMol::calculateScale() {
partitionForLegend();
if (xRange_ > 1e-4 || yRange_ > 1e-4) {
double widthForScale = molWidth_ > 0 ? double(molWidth_) : double(drawWidth_);
double widthForScale =
molWidth_ > 0 ? double(molWidth_) : double(drawWidth_);
const bool sideLegendHoriz =
!legend_.empty() &&
(drawOptions_.legendPosition == MolDrawOptions::LegendPosition::Left ||
drawOptions_.legendPosition == MolDrawOptions::LegendPosition::Right) &&
drawOptions_.legendPosition ==
MolDrawOptions::LegendPosition::Right) &&
!drawOptions_.legendVerticalText;
if (sideLegendHoriz && molWidth_ > 0) {
// Keep a minimum absolute side gap of 8 px so labels do not crowd the
@@ -1132,8 +1134,7 @@ void DrawMol::calculateScale() {
widthForScale = double(molWidth_) - g;
}
}
newScale =
std::min(widthForScale / xRange_, double(molHeight_) / yRange_);
newScale = std::min(widthForScale / xRange_, double(molHeight_) / yRange_);
double fix_scale = newScale;
// after all that, use the fixed scale unless it's too big, in which case
// scale the drawing down to fit.
@@ -1402,6 +1403,7 @@ void DrawMol::shrinkToFit(bool withPadding) {
} else {
legendHeight_ = 0;
molHeight_ = height_;
molWidth_ = drawWidth_;
drawHeight_ = height_ * (1 - 2 * padding);
}
}
@@ -1801,24 +1803,23 @@ std::vector<std::string> split_legend_bits(const std::string &legend,
return legend_bits;
}
boost::split(legend_bits, legend, boost::is_any_of("\n"));
legend_bits.erase(std::remove_if(legend_bits.begin(), legend_bits.end(),
[](const std::string &s) {
return s.empty();
}),
legend_bits.end());
legend_bits.erase(
std::remove_if(legend_bits.begin(), legend_bits.end(),
[](const std::string &s) { return s.empty(); }),
legend_bits.end());
return legend_bits;
}
void place_bottom_legend(const std::vector<std::string> &legend_bits,
double relFontScale, const DrawColour &legendColour,
DrawText &textDrawer, double baseX, double baseY,
double drawWidth, double drawHeight,
std::vector<std::unique_ptr<DrawAnnotation>> &legends) {
void place_bottom_legend(
const std::vector<std::string> &legend_bits, double relFontScale,
const DrawColour &legendColour, DrawText &textDrawer, double baseX,
double baseY, double drawWidth, double drawHeight,
std::vector<std::unique_ptr<DrawAnnotation>> &legends) {
Point2D loc(baseX + drawWidth / 2.0, baseY + drawHeight);
for (const auto &bit : legend_bits) {
legends.emplace_back(new DrawAnnotation(bit, TextAlignType::MIDDLE, "legend",
relFontScale, loc, legendColour,
textDrawer));
legends.emplace_back(new DrawAnnotation(bit, TextAlignType::MIDDLE,
"legend", relFontScale, loc,
legendColour, textDrawer));
}
double xmin, xmax, ymin, ymax;
double lastAbove = 0.0;
@@ -1847,7 +1848,8 @@ void place_top_legend(const std::vector<std::string> &legend_bits,
for (size_t i = 0; i < legend_bits.size(); ++i) {
double height, width;
DrawAnnotation da(legend_bits[i], TextAlignType::MIDDLE, "legend",
relFontScale, Point2D(0.0, 0.0), legendColour, textDrawer);
relFontScale, Point2D(0.0, 0.0), legendColour,
textDrawer);
da.getDimensions(width, height);
total_h += height;
if (i + 1 < legend_bits.size()) {
@@ -1859,13 +1861,14 @@ void place_top_legend(const std::vector<std::string> &legend_bits,
for (size_t i = 0; i < legend_bits.size(); ++i) {
double height, width;
DrawAnnotation da(legend_bits[i], TextAlignType::MIDDLE, "legend",
relFontScale, Point2D(0.0, 0.0), legendColour, textDrawer);
relFontScale, Point2D(0.0, 0.0), legendColour,
textDrawer);
da.getDimensions(width, height);
y += height / 2.0;
Point2D loc(baseX + drawWidth / 2.0, y);
legends.emplace_back(new DrawAnnotation(legend_bits[i], TextAlignType::MIDDLE,
"legend", relFontScale, loc,
legendColour, textDrawer));
legends.emplace_back(
new DrawAnnotation(legend_bits[i], TextAlignType::MIDDLE, "legend",
relFontScale, loc, legendColour, textDrawer));
y += height / 2.0;
if (i + 1 < legend_bits.size()) {
y += gap;
@@ -1876,8 +1879,9 @@ void place_top_legend(const std::vector<std::string> &legend_bits,
void place_side_legend(const std::vector<std::string> &legend_bits,
double relFontScale, const DrawColour &legendColour,
DrawText &textDrawer, bool vertText, bool is_left,
double baseX, double baseY, int legendWidth, int molWidth,
double drawWidth, double drawHeight, double marginPadding,
double baseX, double baseY, int legendWidth,
int molWidth, double drawWidth, double drawHeight,
double marginPadding,
std::vector<std::unique_ptr<DrawAnnotation>> &legends) {
if (legend_bits.empty()) {
return;
@@ -1925,9 +1929,9 @@ void place_side_legend(const std::vector<std::string> &legend_bits,
for (size_t i = 0; i < legend_bits.size(); ++i) {
y += max_h / 2.0;
Point2D loc(stripCentreX, y);
legends.emplace_back(new DrawAnnotation(legend_bits[i], TextAlignType::MIDDLE,
"legend", relFontScale, loc,
legendColour, textDrawer));
legends.emplace_back(
new DrawAnnotation(legend_bits[i], TextAlignType::MIDDLE, "legend",
relFontScale, loc, legendColour, textDrawer));
y += max_h / 2.0;
if (i + 1 < legend_bits.size()) {
y += gap;
@@ -1955,9 +1959,9 @@ void place_side_legend(const std::vector<std::string> &legend_bits,
da.getDimensions(width, height);
y += height / 2.0;
Point2D loc(stripCentreX, y);
legends.emplace_back(new DrawAnnotation(legend_bits[i], TextAlignType::MIDDLE,
"legend", relFontScale, loc,
legendColour, textDrawer));
legends.emplace_back(
new DrawAnnotation(legend_bits[i], TextAlignType::MIDDLE, "legend",
relFontScale, loc, legendColour, textDrawer));
y += height / 2.0;
if (i + 1 < legend_bits.size()) {
y += gap;
@@ -2007,7 +2011,8 @@ void DrawMol::extractLegend() {
(drawOptions_.legendPosition == MolDrawOptions::LegendPosition::Left ||
drawOptions_.legendPosition == MolDrawOptions::LegendPosition::Right) &&
!vertText;
if (total_width > maxWidth && maxWidth > 0 && !flexiCanvasX_ && !sideHorizontal) {
if (total_width > maxWidth && maxWidth > 0 && !flexiCanvasX_ &&
!sideHorizontal) {
relFontScale *= maxWidth / total_width;
calc_legend_dims(legend_bits, relFontScale, drawOptions_.legendColour,
textDrawer_, total_width, total_height);
@@ -2064,9 +2069,8 @@ void DrawMol::extractLegend() {
}
const double gap = calc_line_gap(textDrawer_, vertText, relFontScale);
const size_t n = legend_bits.size();
const double span =
static_cast<double>(n) * max_h +
(n > 1 ? static_cast<double>(n - 1) * gap : 0.0);
const double span = static_cast<double>(n) * max_h +
(n > 1 ? static_cast<double>(n - 1) * gap : 0.0);
if (span <= avail || span <= 0.0) {
break;
}
@@ -2098,9 +2102,9 @@ void DrawMol::extractLegend() {
break;
case MolDrawOptions::LegendPosition::Right:
place_side_legend(legend_bits, relFontScale, drawOptions_.legendColour,
textDrawer_, vertText, false, baseX, baseY, legendWidth_,
molWidth_, drawWidth_, drawHeight_, marginPadding_,
legends_);
textDrawer_, vertText, false, baseX, baseY,
legendWidth_, molWidth_, drawWidth_, drawHeight_,
marginPadding_, legends_);
break;
}
}

View File

@@ -375,6 +375,7 @@ const std::map<std::string, std::hash_result_t> SVG_HASHES = {
{"testDrawingExtentsInclude_allButHighlights.svg", 1604243819U},
{"testDrawingExtentsIncludeWithHighlights_allButHighlights.svg",
436783789U},
{"test_Github9301_1.svg", 3573122884U},
{"test_Github9280_1.0.svg", 1658116840U},
{"test_Github9280_2.0.svg", 1805554327U},
{"test_Github9280_0.3.svg", 893100468U},
@@ -11366,6 +11367,37 @@ M END
}
}
TEST_CASE("Github9301 - reaction layout regression") {
std::unique_ptr<ChemicalReaction> rxn(RxnSmartsToChemicalReaction(
"[CH3:1][C:2](=[O:3])[OH:4].[CH3:5][NH2:6]>CC(O)C.[Pt]>[CH3:1][C:2](=[O:3])[NH:6][CH3:5].[OH2:4]"));
MolDraw2DSVG drawer(450, 200, 450, 200, NO_FREETYPE);
drawer.drawReaction(*rxn);
drawer.finishDrawing();
std::ofstream outs("test_Github9301_1.svg");
auto txt = drawer.getDrawingText();
outs << txt;
outs.close();
const static std::regex atom0(
"<text x='(\\d+\\.\\d+)' y='(\\d+\\.\\d+)' class='atom-0'.* >C</text>");
std::ptrdiff_t const match_count(
std::distance(std::sregex_iterator(txt.begin(), txt.end(), atom0),
std::sregex_iterator()));
CHECK(match_count == 3);
auto match_begin = std::sregex_iterator(txt.begin(), txt.end(), atom0);
std::smatch match = *match_begin;
CHECK_THAT(stod(match[1]), Catch::Matchers::WithinAbs(40.2, 0.1));
CHECK_THAT(stod(match[2]), Catch::Matchers::WithinAbs(125.8, 0.1));
++match_begin;
match = *match_begin;
CHECK_THAT(stod(match[1]), Catch::Matchers::WithinAbs(138.3, 0.1));
CHECK_THAT(stod(match[2]), Catch::Matchers::WithinAbs(104.5, 0.1));
++match_begin;
match = *match_begin;
CHECK_THAT(stod(match[1]), Catch::Matchers::WithinAbs(355.9, 0.1));
CHECK_THAT(stod(match[2]), Catch::Matchers::WithinAbs(80.0, 0.1));
check_file_hash("test_Github9301_1.svg");
}
TEST_CASE("Github 9280 - font scaling bug") {
auto mol = "CC(C)Oc1ccc(N2CCc3nccc(C(=O)Nc4ccccn4)c3C2)nc1"_smiles;
{