Detect recursive layouts and prevent a crash in such cases (fixes #198)

include/TGUI/Layout.hpp

@@ -273,6 +273,8 @@ TGUI_MODULE_EXPORT namespace tgui
         Widget* m_boundWidget = nullptr; // The widget on which this layout depends in case the operation is a binding
         String m_boundString; // String referring to a widget on which this layout depends in case the layout was created from a string and contains a binding operation
         std::function<void()> m_connectedWidgetCallback = nullptr; // Function to call when the value of the layout changes in case the layout and sublayouts are not all constants
+        bool m_callingCallbackDuringConnect = false; // Used to detect that connectWidget is called in an infinity loop if certain layouts depend on each other
+        bool m_callingCallbackDuringRecalculate = false; // Used to detect that recalculateValue is called in an infinity loop if layouts depend on each other
 
         /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
     };

src/Layout.cpp

@@ -356,7 +356,9 @@ namespace tgui
         m_rightOperand   {other.m_rightOperand ? std::make_unique<Layout>(*other.m_rightOperand) : nullptr},
         m_boundWidget    {other.m_boundWidget},
         m_boundString    {other.m_boundString},
-        m_connectedWidgetCallback{nullptr}
+        m_connectedWidgetCallback{nullptr},
+        m_callingCallbackDuringConnect{false},
+        m_callingCallbackDuringRecalculate{false}
     {
         // Disconnect the bound widget if a string was used, the same name may apply to a different widget now
         if (!m_boundString.empty())
@@ -375,7 +377,9 @@ namespace tgui
         m_rightOperand   {std::move(other.m_rightOperand)},
         m_boundWidget    {other.m_boundWidget},
         m_boundString    {std::move(other.m_boundString)},
-        m_connectedWidgetCallback{std::move(other.m_connectedWidgetCallback)}
+        m_connectedWidgetCallback{std::move(other.m_connectedWidgetCallback)},
+        m_callingCallbackDuringConnect{false},
+        m_callingCallbackDuringRecalculate{false}
     {
         resetPointers();
     }
@@ -396,6 +400,8 @@ namespace tgui
             m_boundWidget     = other.m_boundWidget;
             m_boundString     = other.m_boundString;
             m_connectedWidgetCallback = nullptr;
+            m_callingCallbackDuringConnect = false;
+            m_callingCallbackDuringRecalculate = false;
 
             // Disconnect the bound widget if a string was used, the same name may apply to a different widget now
             if (!m_boundString.empty())
@@ -423,6 +429,8 @@ namespace tgui
             m_boundWidget     = other.m_boundWidget;
             m_boundString     = std::move(other.m_boundString);
             m_connectedWidgetCallback = std::move(other.m_connectedWidgetCallback);
+            m_callingCallbackDuringConnect = false;
+            m_callingCallbackDuringRecalculate = false;
 
             resetPointers();
         }
@@ -622,7 +630,17 @@ namespace tgui
         if (m_value != oldValue)
         {
             if (m_connectedWidgetCallback)
+            {
+                if (m_callingCallbackDuringConnect)
+                {
+                    TGUI_PRINT_WARNING("Dependency cycle detected in layout!")
+                    return;
+                }
+
+                m_callingCallbackDuringConnect = true;
                 m_connectedWidgetCallback();
+                m_callingCallbackDuringConnect = false;
+            }
         }
     }
 
@@ -720,7 +738,17 @@ namespace tgui
             {
                 // The topmost layout must tell the connected widget about the new value
                 if (m_connectedWidgetCallback)
+                {
+                    if (m_callingCallbackDuringRecalculate)
+                    {
+                        TGUI_PRINT_WARNING("Dependency cycle detected in layout!")
+                        m_value = 0;
+                    }
+
+                    m_callingCallbackDuringRecalculate = true;
                     m_connectedWidgetCallback();
+                    m_callingCallbackDuringRecalculate = false;
+                }
             }
         }
     }

tests/Layouts.cpp

@@ -196,6 +196,9 @@ TEST_CASE("[Layouts]")
 
                 button4->setSize(80, 120);
                 REQUIRE(button5->getSize() == tgui::Vector2f(120, 80));
+
+                button5->setPosition(bindSize(button5));
+                REQUIRE(button5->getPosition() == tgui::Vector2f(120, 80));
             }
 
             SECTION("Container inner size")
@@ -437,6 +440,21 @@ TEST_CASE("[Layouts]")
             REQUIRE(button3->getPosition() == tgui::Vector2f(995, 5560));
             REQUIRE(button3->getAbsolutePosition() == tgui::Vector2f(1005, 5585));
             REQUIRE(button3->getPositionLayout().toString() == "(((2 * (b1.left + b1.width)) + (b2.x / 4)) + b1.w, (50 - (b2.top + b2.height)) + (75 * b2.y))");
+
+            auto button4 = std::make_shared<tgui::Button>();
+            button4->setSize(200, 50);
+            panel->add(button4, "b4");
+
+            auto button5 = std::make_shared<tgui::Button>();
+            panel->add(button5);
+            button5->setSize({"max(b4.w, b4.h)", "min(b4.w, b4.h)"});
+            REQUIRE(button5->getSize() == tgui::Vector2f(200, 50));
+
+            button4->setSize(80, 120);
+            REQUIRE(button5->getSize() == tgui::Vector2f(120, 80));
+
+            button5->setPosition("size");
+            REQUIRE(button5->getPosition() == tgui::Vector2f(120, 80));
         }
 
         SECTION("No ambiguity with 0")
@@ -460,7 +478,7 @@ TEST_CASE("[Layouts]")
 
             // Button width becomes -10
             panel->setSize(10, 10);
-            button->setSize({"&.w - 20", "&.h"});
+            button->setSize({"&.iw - 20", "&.ih"});
             REQUIRE(button->getSize() == tgui::Vector2f(-10, 10));
 
             // Button width will become positive again
@@ -492,5 +510,27 @@ TEST_CASE("[Layouts]")
             // layouts that are connected to the panel.
             panel->setSize({800, 600});
         }
+
+        SECTION("Recursive layouts could crash (https://github.com/texus/TGUI/issues/198)")
+        {
+            std::streambuf *oldbuf = std::cerr.rdbuf(nullptr);
+
+            auto widget = tgui::ClickableWidget::create();
+            widget->setSize({100, 100});
+            widget->setSize({100, "5 + height"});
+            REQUIRE(widget->getSize().y == 5);
+
+            auto widget2 = tgui::ClickableWidget::create();
+            widget2->setSize(bindSize(widget) * 2);
+
+            widget->setSize({100, 100});
+            REQUIRE(widget2->getSize() == tgui::Vector2f{200, 200});
+
+            widget->setSize(bindSize(widget2) * 200);
+            REQUIRE(widget->getSize() == tgui::Vector2f{0, 0});
+            REQUIRE(widget2->getSize() == tgui::Vector2f{0, 0});
+
+            std::cerr.rdbuf(oldbuf);
+        }
     }
 }

tests/Tests.cpp

@@ -86,6 +86,8 @@ void testWidgetSignals(const tgui::Widget::Ptr& widget)
         parent->mouseMoved({220, 155});
         REQUIRE(mouseEnteredCount == 1);
         REQUIRE(mouseLeftCount == 1);
+
+        parent->remove(widget);
     }
 }
 

tests/Widgets/MenuBar.cpp

@@ -337,6 +337,11 @@ TEST_CASE("[MenuBar]")
         menuBar->addMenu("Help");
         menuBar->addMenuItem("About");
 
+        SECTION("Widget")
+        {
+            testWidgetSignals(menuBar);
+        }
+
         // The menu bar needs to be attached to a Gui object as it will create a new widget when a menu opens.
         // All events also need to be send to the gui to determine to which widget the event goes.
         globalGui->add(menuBar);
@@ -360,11 +365,6 @@ TEST_CASE("[MenuBar]")
             globalGui->handleEvent(event);
         };
 
-        SECTION("Widget")
-        {
-            testWidgetSignals(menuBar);
-        }
-
         SECTION("Opening and closing menu")
         {
             simulateLeftMouseClick(50, 10);