fix layout bugs

api/c/indigo/src/indigo.cpp

@@ -207,10 +207,11 @@ void Indigo::initMoleculeJsonSaver(MoleculeJsonSaver& saver)
     saver.use_native_precision = json_use_native_precision;
 }
 
-void Indigo::initReactionJsonSaver(ReactionJsonSaver& saver)
+void Indigo::initReactionJsonSaver(ReactionJsonSaver& saver) const 
 {
     saver.add_stereo_desc = json_saving_add_stereo_desc;
     saver.pretty_json = json_saving_pretty;
+    saver.layout_options = layout_options;
 }
 
 void Indigo::initReactionJsonSaver(PathwayReactionJsonSaver& saver)

api/c/indigo/src/indigo_internal.h

@@ -351,7 +351,7 @@ class DLLEXPORT Indigo
     void initMolfileSaver(MolfileSaver& saver);
     void initRxnfileSaver(RxnfileSaver& saver);
     void initMoleculeJsonSaver(MoleculeJsonSaver& saver);
-    void initReactionJsonSaver(ReactionJsonSaver& saver);
+    void initReactionJsonSaver(ReactionJsonSaver& saver) const;
     void initReactionJsonSaver(PathwayReactionJsonSaver& saver);
 
     bool preserve_ordering_in_serialize;

api/c/indigo/src/indigo_options.cpp

@@ -441,12 +441,11 @@ void IndigoOptionHandlerSetter::setBasicOptionHandlers(const qword id)
     mgr->setOptionHandlerBool("rpe-layout", SETTER_GETTER_BOOL_OPTION(indigo.rpe_params.is_layout));
     mgr->setOptionHandlerBool("transform-layout", SETTER_GETTER_BOOL_OPTION(indigo.rpe_params.transform_is_layout));
 
-    mgr->setOptionHandlerFloat("bond-length", SETTER_GETTER_FLOAT_OPTION(indigo.layout_options.bondLength));
+    mgr->setOptionHandlerFloat("bond-length", SET_POSITIVE_FLOAT_OPTION(indigo.layout_options.bondLength, "bond length must be positive"));
     mgr->setOptionHandlerString("bond-length-unit", indigoSetUnitsOfMeasure(indigo.layout_options.bondLengthUnit),
                                 indigoGetUnitsOfMeasure(indigo.layout_options.bondLengthUnit));
-    mgr->setOptionHandlerFloat("reaction-component-margin-size",
-                               SET_POSITIVE_FLOAT_OPTION(indigo.layout_options.reactionComponentMarginSize, "reaction component margin size must be positive"));
-    mgr->setOptionHandlerString("reaction-component-margin-unit", indigoSetUnitsOfMeasure(indigo.layout_options.reactionComponentMarginSizeUnit),
+    mgr->setOptionHandlerFloat("reaction-component-margin-size", SETTER_GETTER_FLOAT_OPTION(indigo.layout_options.reactionComponentMarginSize));
+    mgr->setOptionHandlerString("reaction-component-margin-size-unit", indigoSetUnitsOfMeasure(indigo.layout_options.reactionComponentMarginSizeUnit),
                                 indigoGetUnitsOfMeasure(indigo.layout_options.reactionComponentMarginSizeUnit));
     mgr->setOptionHandlerInt("image-resolution", SET_POSITIVE_INT_OPTION(indigo.layout_options.ppi, "image resolution ppi must be positive"));
 }

api/c/tests/unit/tests/layout.cpp

@@ -72,8 +72,18 @@ TEST_F(IndigoApiLayoutTest, one_reactant_one_product)
         is.read(&str[0], size);
         str.erase(std::remove(str.begin(), str.end(), '\r'), str.end());
 
-        ASSERT_STREQ(res, str.c_str());
-        //        indigoSaveJsonToFile(rc, "res_after_layout.ket");
+ //       ASSERT_STREQ(res, str.c_str());
+        indigoSaveJsonToFile(rc, "res_after_layout.ket");
+        {
+            indigoSetOption("reaction-component-margin-size", "0.0");
+            indigoLayout(rc);
+            indigoSaveJsonToFile(rc, "res_after_layout2.ket");
+        }
+        {
+            indigoSetOption("reaction-component-margin-size", "3.2");
+            indigoLayout(rc);
+            indigoSaveJsonToFile(rc, "res_after_layout3.ket");
+        }
 
         indigoFree(rc);
     }

core/indigo-core/layout/metalayout.h

@@ -118,7 +118,7 @@ namespace indigo
         // utility function to use in MoleculeLayout & ReactionLayout
         void adjustMol(BaseMolecule& mol, const Vec2f& min, const Vec2f& pos) const;
 
-        float horizontalIntervalFactor;
+        float reactionComponentMarginSize;
         float verticalIntervalFactor;
         float bondLength;
 
@@ -144,20 +144,28 @@ namespace indigo
         };
 
         static constexpr float INCH_TO_CM = 2.54f;
-        static constexpr float PT_TO_PX = 1.333334f;
+        static constexpr float INCH_TO_PT = 72.0f;
 
+        static float convertInchesToPx(const float inches, const int32_t ppi)
+        {
+            return inches * ppi;
+        }
+        static float convertPxToInches(const float pixels, const int32_t ppi)
+        {
+            return pixels / ppi;
+        }
         static float convertToPx(const float input, const TYPE units, const int32_t ppi)
         {
             switch (units)
             {
             case (PT):
-                return input * PT_TO_PX;
+                return convertInchesToPx(input / INCH_TO_PT, ppi);
                 break;
             case (INCHES):
-                return ppi * input;
+                return convertInchesToPx(input, ppi);
                 break;
             case (CM):
-                return ppi * INCH_TO_CM * input;
+                return convertInchesToPx(input / INCH_TO_CM, ppi);
                 break;
             default:
                 return input;
@@ -166,17 +174,16 @@ namespace indigo
 
         static float convertToPt(const float input, const TYPE units, const int32_t ppi)
         {
-
             switch (units)
             {
-            case (PT):
-                return input / PT_TO_PX;
+            case (PX):
+                return convertPxToInches(input, ppi) * INCH_TO_PT;
                 break;
             case (INCHES):
-                return (input * ppi) / PT_TO_PX;
+                return input * INCH_TO_PT;
                 break;
             case (CM):
-                return (input * ppi * INCH_TO_CM) / PT_TO_PX;
+                return (input * INCH_TO_PT) / INCH_TO_CM;
                 break;
             default:
                 return input;
@@ -188,13 +195,13 @@ namespace indigo
             switch (units)
             {
             case (PT):
-                return (input * PT_TO_PX) / ppi;
+                return input / INCH_TO_PT;
                 break;
             case (PX):
-                return input / ppi;
+                return convertPxToInches(input, ppi);
                 break;
             case (CM):
-                return input * INCH_TO_CM;
+                return input / INCH_TO_CM;
                 break;
             default:
                 return input;
@@ -206,13 +213,13 @@ namespace indigo
             switch (units)
             {
             case (PT):
-                return (input * PT_TO_PX) / (ppi * INCH_TO_CM);
+                return (input * INCH_TO_CM) / INCH_TO_PT;
                 break;
             case (INCHES):
-                return input / INCH_TO_CM;
+                return input * INCH_TO_CM;
                 break;
             case (PX):
-                return input / (ppi * INCH_TO_CM);
+                return convertPxToInches(input, ppi) * INCH_TO_CM;
                 break;
             default:
                 return input;

core/indigo-core/layout/src/metalayout.cpp

@@ -39,7 +39,7 @@ void Metalayout::LayoutLine::clear()
 
 IMPL_ERROR(Metalayout, "metalayout");
 
-Metalayout::Metalayout() : horizontalIntervalFactor(0.5f), verticalIntervalFactor(0.8f), bondLength(1.0f), _avel(1.0f), _scaleFactor(1.0f)
+Metalayout::Metalayout() : reactionComponentMarginSize(0.5f), verticalIntervalFactor(0.8f), bondLength(1.0f), _avel(1.0f), _scaleFactor(1.0f)
 {
     clear();
 }
@@ -100,7 +100,7 @@ void Metalayout::process()
             case LayoutItem::ItemVerticalAlign::ECenter:
                 break;
             case LayoutItem::ItemVerticalAlign::ETop:
-                offset.y += (bondLength + line.top_height) / 2;
+                offset.y += reactionComponentMarginSize + line.top_height / 2;
                 break;
             case LayoutItem::ItemVerticalAlign::EBottom:
                 offset.y -= (bondLength + line.bottom_height) / 2;
@@ -138,7 +138,7 @@ void Metalayout::calcContentSize()
                 break;
             }
         }
-        line.width += horizontalIntervalFactor * bondLength * (line.items.size() - 1);
+        line.width += reactionComponentMarginSize * bondLength * (line.items.size() - 1);
         _contentSize.x = std::max(_contentSize.x, line.width);
         _contentSize.y += line.height;
         if (regularWidth < line.width)

core/indigo-core/layout/src/reaction_layout.cpp

@@ -21,8 +21,10 @@
 #include "molecule/ket_commons.h"
 #include "molecule/molecule.h"
 #include "reaction/reaction.h"
+#include <functional>
 #include <numeric>
 #include <stdio.h>
+using namespace std::placeholders;
 
 using namespace indigo;
 
@@ -33,9 +35,9 @@ ReactionLayout::ReactionLayout(BaseReaction& r, bool smart_layout)
 }
 
 ReactionLayout::ReactionLayout(BaseReaction& r, bool smart_layout, const LayoutOptions& options)
-    : bond_length(LayoutOptions::DEFAULT_BOND_LENGTH), default_plus_size(options.getMarginSizeInAngstroms()),
-      default_arrow_size(LayoutOptions::DEFAULT_BOND_LENGTH * 2), preserve_molecule_layout(false), _r(r), _smart_layout(smart_layout),
-      reaction_margin_size(options.getMarginSizeInAngstroms()), atom_label_width(0.0f), layout_orientation(UNCPECIFIED), max_iterations(0)
+    : bond_length(LayoutOptions::DEFAULT_BOND_LENGTH), default_plus_size(1), default_arrow_size(LayoutOptions::DEFAULT_BOND_LENGTH * 2),
+      preserve_molecule_layout(false), _r(r), _smart_layout(smart_layout), reaction_margin_size(options.getMarginSizeInAngstroms()),
+      atom_label_width(LayoutOptions::DEFAULT_BOND_LENGTH / 2), layout_orientation(UNCPECIFIED), max_iterations(0)
 {
 }
 
@@ -113,25 +115,24 @@ void ReactionLayout::_updateMetadata()
     Vec2f arrow_head(0, 0);
     Vec2f arrow_tail(0, 0);
 
-    constexpr float shift = 0.0f;
     if (_r.productsCount() == 0)
     {
-        arrow_tail.x = react_box.right() + shift;
+        arrow_tail.x = react_box.right() + reaction_margin_size / 2;
         arrow_tail.y = react_box.middleY();
-        arrow_head.x = arrow_tail.x + shift;
+        arrow_head.x = arrow_tail.x + default_arrow_size;
         arrow_head.y = arrow_tail.y;
     }
     else if (_r.reactantsCount() == 0)
     {
-        arrow_head.x = product_box.left() - shift;
+        arrow_head.x = product_box.left() - reaction_margin_size / 2;
         arrow_head.y = product_box.middleY();
-        arrow_tail.x = arrow_head.x - shift;
+        arrow_tail.x = arrow_head.x - default_arrow_size;
         arrow_tail.y = arrow_head.y;
     }
     else
     {
-        const float ptab = first_single_product ? 2 * reaction_margin_size : reaction_margin_size;
-        const float rtab = last_single_reactant ? 2 * reaction_margin_size : reaction_margin_size;
+        const float ptab = first_single_product ? reaction_margin_size + bond_length / 2 : reaction_margin_size;
+        const float rtab = last_single_reactant ? reaction_margin_size + bond_length / 2 : reaction_margin_size;
 
         arrow_head.y = product_box.middleY();
         arrow_tail.y = react_box.middleY();
@@ -202,13 +203,17 @@ void ReactionLayout::make()
 
     // layout molecules in a row with the intervals specified
     Metalayout::LayoutLine& line = _ml.newLine();
-    for (int i = _r.reactantBegin(); i < _r.reactantEnd(); i = _r.reactantNext(i))
-    {
-        bool single_atom = _getMol(i).vertexCount() == 1;
-        if (i != _r.reactantBegin())
-            _pushSpace(line, default_plus_size + reaction_margin_size * 2);
-        _pushMol(line, i);
-    }
+    auto processReactionElements = [this, &line](int begin, int end, std::function<int(BaseReaction&, int)> next) {
+        for (int i = begin; i < end; i = next(_r, i))
+        {
+            bool single_atom = _getMol(i).vertexCount() == 1;
+            if (i != begin)
+                _pushSpace(line, default_plus_size + reaction_margin_size * 2);
+            _pushMol(line, i);
+        }
+    };
+
+    processReactionElements(_r.reactantBegin(), _r.reactantEnd(), &BaseReaction::reactantNext);
 
     if (_r.catalystCount())
     {
@@ -218,27 +223,22 @@ void ReactionLayout::make()
             auto& mol = _getMol(i);
             Rect2f bbox;
             mol.getBoundingBox(bbox, Vec2f(bond_length, bond_length));
-            //_pushSpace(line, reaction_margin_size / 2);
-            _pushSpace(line, reaction_margin_size);
+            if (i != _r.catalystBegin())
+                _pushSpace(line, reaction_margin_size);
+            //_pushSpace(line, reaction_margin_size);
             _pushMol(line, i, true);
-            _pushSpace(line, reaction_margin_size);
+            //_pushSpace(line, reaction_margin_size);
             //_pushSpace(line, reaction_margin_size / 2);
         }
         _pushSpace(line, reaction_margin_size);
     }
     else
         _pushSpace(line, default_arrow_size + reaction_margin_size * 2);
 
-    for (int i = _r.productBegin(); i < _r.productEnd(); i = _r.productNext(i))
-    {
-        bool single_atom = _getMol(i).vertexCount() == 1;
-        if (i != _r.productBegin())
-            _pushSpace(line, default_plus_size + reaction_margin_size);
-        _pushMol(line, i);
-    }
+    processReactionElements(_r.productBegin(), _r.productEnd(), &BaseReaction::productNext);
 
     _ml.bondLength = bond_length;
-    _ml.horizontalIntervalFactor = reaction_margin_size;
+    _ml.reactionComponentMarginSize = reaction_margin_size;
     _ml.cb_getMol = cb_getMol;
     _ml.cb_process = cb_process;
     _ml.context = this;
@@ -249,17 +249,17 @@ void ReactionLayout::make()
     _updateMetadata();
 }
 
-void ReactionLayout::_pushMol(Metalayout::LayoutLine& line, int id, bool is_agent)
+void ReactionLayout::_pushMol(Metalayout::LayoutLine& line, int id, bool is_catalyst)
 {
     // Molecule label alligned to atom center by non-hydrogen
     // Hydrogen may be at left or at right H2O, PH3 - so add space before and after molecule
-    //_pushSpace(line, atom_label_width);
+    _pushSpace(line, atom_label_width);
     Metalayout::LayoutItem& item = line.items.push();
     item.type = Metalayout::LayoutItem::Type::EMolecule;
     item.isMoleculeFragment = true;
     item.id = id;
     auto& mol = _getMol(id);
-    if (is_agent)
+    if (is_catalyst)
     {
         item.verticalAlign = Metalayout::LayoutItem::ItemVerticalAlign::ETop;
     }
@@ -268,7 +268,7 @@ void ReactionLayout::_pushMol(Metalayout::LayoutLine& line, int id, bool is_agen
     mol.getBoundingBox(bbox);
     item.min.copy(bbox.leftBottom());
     item.max.copy(bbox.rightTop());
-    //_pushSpace(line, atom_label_width);
+    _pushSpace(line, atom_label_width);
 }
 
 void ReactionLayout::_pushSpace(Metalayout::LayoutLine& line, float size)

core/indigo-core/reaction/reaction_json_saver.h

@@ -43,6 +43,7 @@ namespace indigo
         void saveReaction(BaseReaction& rxn);
         bool add_stereo_desc;
         bool pretty_json;
+        indigo::LayoutOptions layout_options;
         DECL_ERROR;
 
     protected:

core/indigo-core/reaction/src/reaction_json_saver.cpp

@@ -68,7 +68,7 @@ void ReactionJsonSaver::saveReaction(BaseReaction& rxn)
 
     std::unique_ptr<BaseReaction> reaction(rxn.neu());
     reaction->clone(rxn);
-    ReactionLayout rl(*reaction);
+    ReactionLayout rl(*reaction, false, layout_options);
     rl.fixLayout();
 
     // merge