Implement monitor auto-positioning

Author: adazem009

include/scratchcpp/monitor.h

@@ -85,10 +85,12 @@ class LIBSCRATCHCPP_EXPORT Monitor : public Entity
         bool discrete() const;
         void setDiscrete(bool discrete);
 
-        static Rect getInitialPosition(const std::vector<std::shared_ptr<Monitor>> &other, int monitorWidth, int monitorHeight);
         bool needsAutoPosition() const;
+        void autoPosition(const std::vector<std::shared_ptr<Monitor>> &allMonitors);
 
     private:
+        static bool monitorRectsIntersect(const Rect &a, const Rect &b);
+
         spimpl::unique_impl_ptr<MonitorPrivate> impl;
 };
 

src/engine/internal/engine.cpp

@@ -1851,11 +1851,6 @@ void Engine::addVarOrListMonitor(std::shared_ptr<Monitor> monitor, Target *targe
         monitor->setValueChangeFunction(changeFunc);
     }
 
-    // Auto-position the monitor
-    Rect rect = Monitor::getInitialPosition(m_monitors, monitor->width(), monitor->height());
-    monitor->setX(rect.left());
-    monitor->setY(rect.top());
-
     m_monitors.push_back(monitor);
     m_monitorAdded(monitor.get());
 

src/scratch/monitor.cpp

@@ -253,21 +253,130 @@ void Monitor::setDiscrete(bool discrete)
     impl->discrete = discrete;
 }
 
-/*! Returns true if the monitor needs auto positioning. */
+/*! Returns true if the monitor needs auto positioning. The renderer should call autoPosition() as soon as it knows the monitor size. */
 bool Monitor::needsAutoPosition() const
 {
     return impl->needsAutoPosition;
 }
 
-/*! Returns the initial position of a monitor. */
-Rect Monitor::getInitialPosition(const std::vector<std::shared_ptr<Monitor>> &other, int monitorWidth, int monitorHeight)
+/*!
+ * Auto-positions the monitor with the other monitors.
+ * \note Call this only when the monitor size is known.
+ */
+void Monitor::autoPosition(const std::vector<std::shared_ptr<Monitor>> &allMonitors)
 {
-    // TODO: Implement this like Scratch has: https://github.com/scratchfoundation/scratch-gui/blob/010e27937ecff531f23bfcf3c711cd6e565cc7f9/src/reducers/monitor-layout.js#L161-L243
-    // Place the monitor randomly
+    // https://github.com/scratchfoundation/scratch-gui/blob/010e27937ecff531f23bfcf3c711cd6e565cc7f9/src/reducers/monitor-layout.js#L161-L243
+    if (!impl->needsAutoPosition)
+        std::cout << "warning: auto-positioning already positioned monitor (" << impl->name << ")" << std::endl;
+
+    impl->needsAutoPosition = false;
+
+    // Try all starting positions for the new monitor to find one that doesn't intersect others
+    std::vector<int> endXs = { 0 };
+    std::vector<int> endYs = { 0 };
+    int lastX = 0;
+    int lastY = 0;
+    bool haveLastX = false;
+    bool haveLastY = false;
+
+    for (const auto monitor : allMonitors) {
+        if (monitor.get() != this) {
+            int x = monitor->x() + monitor->width();
+            x = std::ceil(x / 50.0) * 50; // Try to choose a sensible "tab width" so more monitors line up
+            endXs.push_back(x);
+            endYs.push_back(std::ceil(monitor->y() + monitor->height()));
+        }
+    }
+
+    std::sort(endXs.begin(), endXs.end());
+    std::sort(endYs.begin(), endYs.end());
+
+    // We'll use plan B if the monitor doesn't fit anywhere (too long or tall)
+    bool planB = false;
+    Rect planBRect;
+
+    for (const int x : endXs) {
+        if (haveLastX && x == lastX)
+            continue;
+
+        lastX = x;
+        haveLastX = true;
+
+        for (const int y : endYs) {
+            if (haveLastY && y == lastY)
+                continue;
+
+            lastY = y;
+            haveLastY = true;
+
+            const Rect monitorRect(x + PADDING, y + PADDING, x + PADDING + impl->width, y + PADDING + impl->height);
+
+            // Intersection testing rect that includes padding
+            const Rect rect(x, y, x + impl->width + 2 * PADDING, y + impl->height + 2 * PADDING);
+
+            bool intersected = false;
+
+            for (const auto monitor : allMonitors) {
+                if (monitor.get() != this) {
+                    const Rect currentRect(monitor->x(), monitor->y(), monitor->x() + monitor->width(), monitor->y() + monitor->height());
+
+                    if (monitorRectsIntersect(currentRect, rect)) {
+                        intersected = true;
+                        break;
+                    }
+                }
+            }
+
+            if (intersected) {
+                continue;
+            }
+
+            // If the rect overlaps the ends of the screen
+            if (rect.right() > SCREEN_WIDTH || rect.bottom() > SCREEN_HEIGHT) {
+                // If rect is not too close to completely off-screen, set it as plan B
+                if (!planB && !(rect.left() + SCREEN_EDGE_BUFFER > SCREEN_WIDTH || rect.top() + SCREEN_EDGE_BUFFER > SCREEN_HEIGHT)) {
+                    planBRect = monitorRect;
+                    planB = true;
+                }
+
+                continue;
+            }
+
+            setX(monitorRect.left());
+            setY(monitorRect.top());
+            return;
+        }
+    }
+
+    // If the monitor is too long to fit anywhere, put it in the leftmost spot available
+    // that intersects the right or bottom edge and isn't too close to the edge.
+    if (planB) {
+        setX(planBRect.left());
+        setY(planBRect.top());
+        return;
+    }
+
+    // If plan B fails and there's nowhere reasonable to put it, plan C is to place the monitor randomly
     if (!MonitorPrivate::rng)
         MonitorPrivate::rng = RandomGenerator::instance().get();
 
-    const double randX = std::ceil(MonitorPrivate::rng->randintDouble(0, SCREEN_WIDTH / 2.0));
-    const double randY = std::ceil(MonitorPrivate::rng->randintDouble(0, SCREEN_HEIGHT - SCREEN_EDGE_BUFFER));
-    return Rect(randX, randY, randX + monitorWidth, randY + monitorHeight);
+    const int randX = std::ceil(MonitorPrivate::rng->randintDouble(0, SCREEN_WIDTH / 2.0));
+    const int randY = std::ceil(MonitorPrivate::rng->randintDouble(0, SCREEN_HEIGHT - SCREEN_EDGE_BUFFER));
+    setX(randX);
+    setY(randY);
+    return;
+}
+
+bool Monitor::monitorRectsIntersect(const Rect &a, const Rect &b)
+{
+    // https://github.com/scratchfoundation/scratch-gui/blob/010e27937ecff531f23bfcf3c711cd6e565cc7f9/src/reducers/monitor-layout.js#L152-L158
+    // If one rectangle is on left side of other
+    if (a.left() >= b.right() || b.left() >= a.right())
+        return false;
+
+    // If one rectangle is above other
+    if (a.top() >= b.bottom() || b.top() >= a.bottom())
+        return false;
+
+    return true;
 }

test/engine/engine_test.cpp

@@ -1742,6 +1742,7 @@ TEST(EngineTest, CreateMissingMonitors)
         ASSERT_EQ(monitor->id(), var->id());
         ASSERT_EQ(monitor->opcode(), "data_variable");
         ASSERT_EQ(monitor->mode(), Monitor::Mode::Default);
+        ASSERT_TRUE(monitor->needsAutoPosition());
         ASSERT_FALSE(monitor->visible());
         ASSERT_EQ(block->fields().size(), 1);
 
@@ -1764,6 +1765,7 @@ TEST(EngineTest, CreateMissingMonitors)
         ASSERT_EQ(monitor->id(), list->id());
         ASSERT_EQ(monitor->opcode(), "data_listcontents");
         ASSERT_EQ(monitor->mode(), Monitor::Mode::Default);
+        ASSERT_TRUE(monitor->needsAutoPosition());
         ASSERT_FALSE(monitor->visible());
         ASSERT_EQ(block->fields().size(), 1);
 

test/scratch_classes/monitor_test.cpp

@@ -4,7 +4,6 @@
 #include <scratchcpp/rect.h>
 #include <scratchcpp/virtualmachine.h>
 #include <scratchcpp/script.h>
-#include <scratch/monitor_p.h>
 #include <monitorhandlermock.h>
 #include <randomgeneratormock.h>
 
@@ -20,7 +19,6 @@ using ::testing::_;
 static const int PADDING = 5;
 static const int SCREEN_WIDTH = 400;
 static const int SCREEN_HEIGHT = 300;
-static const int SCREEN_EDGE_BUFFER = 40;
 
 TEST(MonitorTest, Constructors)
 {
@@ -272,36 +270,103 @@ TEST(MonitorTest, Discrete)
     ASSERT_FALSE(monitor.discrete());
 }
 
-TEST(MonitorTest, GetInitialPosition)
+TEST(MonitorTest, AutoPosition_LineUpTopLeft)
 {
-    std::vector<std::shared_ptr<Monitor>> monitors;
+    // https://github.com/scratchfoundation/scratch-gui/blob/875bee35f178411b9149ab766d17b5fb88ddd749/test/unit/reducers/monitor-layout-reducer.test.js#L216-L238
     const int width = 100;
     const int height = 200;
 
-    auto monitor1 = std::make_shared<Monitor>("", "");
-    monitor1->setWidth(PADDING);
-    monitor1->setHeight(height);
-    monitor1->setX(100);
-    monitor1->setY(0);
-    monitors.push_back(monitor1);
-
-    auto monitor2 = std::make_shared<Monitor>("", "");
-    monitor2->setWidth(width);
-    monitor2->setHeight(height + PADDING - 100);
-    monitor2->setX(0);
-    monitor2->setY(100);
-    monitors.push_back(monitor2);
-
-    RandomGeneratorMock rng;
-    MonitorPrivate::rng = &rng;
-
-    EXPECT_CALL(rng, randintDouble(0, SCREEN_WIDTH / 2.0)).WillOnce(Return(SCREEN_WIDTH / 4.5));
-    EXPECT_CALL(rng, randintDouble(0, SCREEN_HEIGHT - SCREEN_EDGE_BUFFER)).WillOnce(Return(SCREEN_HEIGHT - SCREEN_EDGE_BUFFER * 2.3));
-    Rect rect = Monitor::getInitialPosition(monitors, width, height);
-    ASSERT_EQ(rect.left(), std::ceil(SCREEN_WIDTH / 4.5));
-    ASSERT_EQ(rect.top(), std::ceil(SCREEN_HEIGHT - SCREEN_EDGE_BUFFER * 2.3));
-    ASSERT_EQ(rect.width(), width);
-    ASSERT_EQ(rect.height(), height);
-
-    MonitorPrivate::rng = nullptr;
+    auto monitor = std::make_shared<Monitor>("", "");
+    monitor->setWidth(width);
+    monitor->setHeight(height);
+
+    // Add monitors to right and bottom, but there is a space in the top left
+    std::vector<std::shared_ptr<Monitor>> monitors = { std::make_shared<Monitor>("", ""), std::make_shared<Monitor>("", ""), monitor };
+
+    monitors[0]->setX(width + 2 * PADDING);
+    monitors[0]->setY(0);
+    monitors[0]->setWidth(100);
+    monitors[0]->setHeight(height);
+
+    monitors[1]->setX(0);
+    monitors[1]->setY(height + 2 * PADDING);
+    monitors[1]->setWidth(width);
+    monitors[1]->setHeight(100);
+
+    // Check that the added monitor appears in the space
+    monitor->autoPosition(monitors);
+    ASSERT_EQ(monitor->width(), width);
+    ASSERT_EQ(monitor->height(), height);
+    ASSERT_EQ(monitor->x(), PADDING);
+    ASSERT_EQ(monitor->y(), PADDING);
+    ASSERT_FALSE(monitor->needsAutoPosition());
+}
+
+TEST(MonitorTest, AutoPosition_LineUpLeft)
+{
+    // https://github.com/scratchfoundation/scratch-gui/blob/875bee35f178411b9149ab766d17b5fb88ddd749/test/unit/reducers/monitor-layout-reducer.test.js#L261-L270
+    auto monitor = std::make_shared<Monitor>("", "");
+    monitor->setWidth(20);
+    monitor->setHeight(20);
+
+    const int monitor1EndY = 60;
+
+    // Add a monitor that takes up the upper left corner
+    std::vector<std::shared_ptr<Monitor>> monitors = { std::make_shared<Monitor>("", ""), monitor };
+    monitors[0]->setX(0);
+    monitors[0]->setY(0);
+    monitors[0]->setWidth(100);
+    monitors[0]->setHeight(monitor1EndY);
+
+    // Check that added monitor is under it and lines up left
+    monitor->autoPosition(monitors);
+    ASSERT_EQ(monitor->width(), 20);
+    ASSERT_EQ(monitor->height(), 20);
+    ASSERT_GE(monitor->y(), monitor1EndY + PADDING);
+    ASSERT_FALSE(monitor->needsAutoPosition());
+}
+
+TEST(MonitorTest, LineUpTop)
+{
+    // https://github.com/scratchfoundation/scratch-gui/blob/875bee35f178411b9149ab766d17b5fb88ddd749/test/unit/reducers/monitor-layout-reducer.test.js#L272-L285
+    auto monitor = std::make_shared<Monitor>("", "");
+    monitor->setWidth(20);
+    monitor->setHeight(20);
+
+    const int monitor1EndX = 100;
+
+    // Add a monitor that takes up the whole left side
+    std::vector<std::shared_ptr<Monitor>> monitors = { std::make_shared<Monitor>("", ""), monitor };
+    monitors[0]->setX(0);
+    monitors[0]->setY(0);
+    monitors[0]->setWidth(monitor1EndX);
+    monitors[0]->setHeight(SCREEN_HEIGHT);
+
+    // Check that added monitor is to the right of it and lines up top
+    monitor->autoPosition(monitors);
+    ASSERT_EQ(monitor->width(), 20);
+    ASSERT_EQ(monitor->height(), 20);
+    ASSERT_EQ(monitor->y(), PADDING);
+    ASSERT_GE(monitor->x(), monitor1EndX + PADDING);
+}
+
+TEST(MonitorTest, AutoPosition_NoRoom)
+{
+    // https://github.com/scratchfoundation/scratch-gui/blob/875bee35f178411b9149ab766d17b5fb88ddd749/test/unit/reducers/monitor-layout-reducer.test.js#L287-L302
+    auto monitor = std::make_shared<Monitor>("", "");
+    monitor->setWidth(7);
+    monitor->setHeight(8);
+
+    // Add a monitor that takes up the whole screen
+    std::vector<std::shared_ptr<Monitor>> monitors = { std::make_shared<Monitor>("", ""), monitor };
+    monitors[0]->setX(0);
+    monitors[0]->setY(0);
+    monitors[0]->setWidth(SCREEN_WIDTH);
+    monitors[0]->setHeight(SCREEN_HEIGHT);
+
+    // Check that added monitor exists somewhere (we don't care where)
+    monitor->autoPosition(monitors);
+    ASSERT_EQ(monitor->width(), 7);
+    ASSERT_EQ(monitor->height(), 8);
+    ASSERT_FALSE(monitor->needsAutoPosition());
 }