From e6b549da369bca5987608949056cdbbff97b7c64 Mon Sep 17 00:00:00 2001 From: David Cosgrove Date: Wed, 27 May 2026 05:39:28 +0100 Subject: [PATCH] Github9280 (#9300) * Fix setFontScale for maximum and minimum font sizes. * Fix other test. Add hashcodes. --- Code/GraphMol/MolDraw2D/DrawText.cpp | 6 +- Code/GraphMol/MolDraw2D/catch_tests.cpp | 91 ++++++++++++++++++++----- 2 files changed, 77 insertions(+), 20 deletions(-) diff --git a/Code/GraphMol/MolDraw2D/DrawText.cpp b/Code/GraphMol/MolDraw2D/DrawText.cpp index 116f4ab0f..33f51bfac 100644 --- a/Code/GraphMol/MolDraw2D/DrawText.cpp +++ b/Code/GraphMol/MolDraw2D/DrawText.cpp @@ -78,13 +78,11 @@ bool DrawText::setFontScale(double new_scale, bool ignoreLimits) { } font_scale_ = new_scale; double nfs = fontSize(); - if (max_font_size_ > 0.0 && - nfs * (baseFontSize() / DEFAULT_FONT_SCALE) > max_font_size_) { + if (max_font_size_ > 0.0 && nfs > max_font_size_) { font_scale_ = max_font_size_ / baseFontSize(); return false; } - if (min_font_size_ > 0.0 && - nfs * (baseFontSize() / DEFAULT_FONT_SCALE) < min_font_size_) { + if (min_font_size_ > 0.0 && nfs < min_font_size_) { font_scale_ = min_font_size_ / baseFontSize(); return false; } diff --git a/Code/GraphMol/MolDraw2D/catch_tests.cpp b/Code/GraphMol/MolDraw2D/catch_tests.cpp index 2976c1f10..de7069ac1 100644 --- a/Code/GraphMol/MolDraw2D/catch_tests.cpp +++ b/Code/GraphMol/MolDraw2D/catch_tests.cpp @@ -374,7 +374,11 @@ const std::map SVG_HASHES = { {"testDrawingExtentsIncludeWithHighlights_default.svg", 1595689626U}, {"testDrawingExtentsInclude_allButHighlights.svg", 1604243819U}, {"testDrawingExtentsIncludeWithHighlights_allButHighlights.svg", - 436783789U}}; + 436783789U}, + {"test_Github9280_1.0.svg", 1658116840U}, + {"test_Github9280_2.0.svg", 1805554327U}, + {"test_Github9280_0.3.svg", 893100468U}, + {"test_Github9280_0.2.svg", 770838895U}}; // These PNG hashes aren't completely reliable due to floating point cruft, // but they can still reduce the number of drawings that need visual @@ -4250,7 +4254,7 @@ TEST_CASE("changing baseFontSize") { drawer.drawOptions().baseFontSize = 0.9; drawer.drawMolecule(*mol1); drawer.finishDrawing(); - CHECK_THAT(drawer.fontSize(), Catch::Matchers::WithinAbs(5.5, 0.2)); + CHECK_THAT(drawer.fontSize(), Catch::Matchers::WithinAbs(6.0, 0.2)); auto text = drawer.getDrawingText(); std::ofstream outs("testBaseFontSize.1b.svg"); outs << text; @@ -4868,8 +4872,9 @@ TEST_CASE("legend position Top Left Right and vertical text", "[drawing]") { y = std::stod(match[2].str()); return true; } - std::regex pathRgx("class='legend' d='M (-?[0-9]+\\.?[0-9]*) " - "(-?[0-9]+\\.?[0-9]*)"); + std::regex pathRgx( + "class='legend' d='M (-?[0-9]+\\.?[0-9]*) " + "(-?[0-9]+\\.?[0-9]*)"); if (std::regex_search(text, match, pathRgx) && match.size() == 3) { x = std::stod(match[1].str()); y = std::stod(match[2].str()); @@ -4888,8 +4893,9 @@ TEST_CASE("legend position Top Left Right and vertical text", "[drawing]") { if (!coords.empty()) { return coords; } - std::regex pathRgx("class='legend' d='M (-?[0-9]+\\.?[0-9]*) " - "(-?[0-9]+\\.?[0-9]*)"); + std::regex pathRgx( + "class='legend' d='M (-?[0-9]+\\.?[0-9]*) " + "(-?[0-9]+\\.?[0-9]*)"); for (auto it = std::sregex_iterator(text.begin(), text.end(), pathRgx); it != std::sregex_iterator(); ++it) { coords.emplace_back(std::stod((*it)[1].str()), std::stod((*it)[2].str())); @@ -4902,8 +4908,7 @@ TEST_CASE("legend position Top Left Right and vertical text", "[drawing]") { double right_x = 0.0, right_y = 0.0; SECTION("Top") { MolDraw2DSVG drawer(200, 200, -1, -1, NO_FREETYPE); - drawer.drawOptions().legendPosition = - MolDrawOptions::LegendPosition::Top; + drawer.drawOptions().legendPosition = MolDrawOptions::LegendPosition::Top; MolDraw2DUtils::prepareAndDrawMolecule(drawer, *m1, legend); drawer.finishDrawing(); auto text = drawer.getDrawingText(); @@ -4917,8 +4922,7 @@ TEST_CASE("legend position Top Left Right and vertical text", "[drawing]") { } SECTION("Left with vertical text") { MolDraw2DSVG drawer(200, 200, -1, -1, NO_FREETYPE); - drawer.drawOptions().legendPosition = - MolDrawOptions::LegendPosition::Left; + drawer.drawOptions().legendPosition = MolDrawOptions::LegendPosition::Left; drawer.drawOptions().legendVerticalText = true; MolDraw2DUtils::prepareAndDrawMolecule(drawer, *m1, legend); drawer.finishDrawing(); @@ -4935,8 +4939,7 @@ TEST_CASE("legend position Top Left Right and vertical text", "[drawing]") { } SECTION("Left horizontal") { MolDraw2DSVG drawer(200, 200, -1, -1, NO_FREETYPE); - drawer.drawOptions().legendPosition = - MolDrawOptions::LegendPosition::Left; + drawer.drawOptions().legendPosition = MolDrawOptions::LegendPosition::Left; drawer.drawOptions().legendVerticalText = false; MolDraw2DUtils::prepareAndDrawMolecule(drawer, *m1, legend); drawer.finishDrawing(); @@ -4950,8 +4953,7 @@ TEST_CASE("legend position Top Left Right and vertical text", "[drawing]") { } SECTION("Right horizontal") { MolDraw2DSVG drawer(200, 200, -1, -1, NO_FREETYPE); - drawer.drawOptions().legendPosition = - MolDrawOptions::LegendPosition::Right; + drawer.drawOptions().legendPosition = MolDrawOptions::LegendPosition::Right; drawer.drawOptions().legendVerticalText = false; MolDraw2DUtils::prepareAndDrawMolecule(drawer, *m1, legend); drawer.finishDrawing(); @@ -4980,8 +4982,7 @@ TEST_CASE("legend position Top Left Right and vertical text", "[drawing]") { SECTION("Long vertical side legend fits panel height") { const std::string longName(48, 'M'); MolDraw2DSVG drawer(160, 90, -1, -1, NO_FREETYPE); - drawer.drawOptions().legendPosition = - MolDrawOptions::LegendPosition::Left; + drawer.drawOptions().legendPosition = MolDrawOptions::LegendPosition::Left; drawer.drawOptions().legendVerticalText = true; drawer.drawOptions().legendFraction = 0.22f; MolDraw2DUtils::prepareAndDrawMolecule(drawer, *m1, longName); @@ -11359,3 +11360,61 @@ M END CHECK(checkCoords(referenceCoords, highlightCoords)); } } + +TEST_CASE("Github 9280 - font scaling bug") { + auto mol = "CC(C)Oc1ccc(N2CCc3nccc(C(=O)Nc4ccccn4)c3C2)nc1"_smiles; + { + MolDraw2DSVG drawer(358, 290, -1, -1, NO_FREETYPE); + drawer.drawOptions().baseFontSize = 1.0; + drawer.drawMolecule(*mol); + drawer.finishDrawing(); + auto text = drawer.getDrawingText(); + std::ofstream ofs("test_Github9280_1.0.svg"); + ofs << text; + ofs.close(); + // With the bug, it snapped to maximum font size, 40 pixels. + CHECK(text.find("font-size:40px") == std::string::npos); + CHECK(text.find("font-size:24px") != std::string::npos); + check_file_hash("test_Github9280_1.0.svg"); + } + { + // Check it still maxes out at 40 - font size would be 50 without. + MolDraw2DSVG drawer(358, 290, -1, -1, NO_FREETYPE); + drawer.drawOptions().baseFontSize = 2.0; + drawer.drawMolecule(*mol); + drawer.finishDrawing(); + auto text = drawer.getDrawingText(); + std::ofstream ofs("test_Github9280_2.0.svg"); + ofs << text; + ofs.close(); + CHECK(text.find("font-size:40px") != std::string::npos); + check_file_hash("test_Github9280_2.0.svg"); + } + { + MolDraw2DSVG drawer(358, 290, -1, -1, NO_FREETYPE); + drawer.drawOptions().baseFontSize = 0.3; + drawer.drawMolecule(*mol); + drawer.finishDrawing(); + auto text = drawer.getDrawingText(); + std::ofstream ofs("test_Github9280_0.3.svg"); + ofs << text; + ofs.close(); + // With the bug, it snapped to minimum font size, 6 pixels. + CHECK(text.find("font-size:6px") == std::string::npos); + CHECK(text.find("font-size:7px") != std::string::npos); + check_file_hash("test_Github9280_0.3.svg"); + } + { + MolDraw2DSVG drawer(358, 290, -1, -1, NO_FREETYPE); + drawer.drawOptions().baseFontSize = 0.2; + drawer.drawMolecule(*mol); + drawer.finishDrawing(); + auto text = drawer.getDrawingText(); + std::ofstream ofs("test_Github9280_0.2.svg"); + ofs << text; + ofs.close(); + // This should be the minimum font size + CHECK(text.find("font-size:6px") != std::string::npos); + check_file_hash("test_Github9280_0.2.svg"); + } +} \ No newline at end of file