From ad5a755b10bddd0c9849a512be6b3f7d420eddf0 Mon Sep 17 00:00:00 2001
From: reuk <reuk@users.noreply.github.com>
Date: Tue, 1 Oct 2024 18:08:04 +0100
Subject: [PATCH] Viewport: Avoid stack overflows when displaying transformed
 content components

For some transforms, the program could get stuck in the following loop:
- The content component emits a resized/moved notification, leading to
  the initial call to Viewport::updateVisibleArea.
- New positions are computed for the viewport scrollbars, and scrollbar
  listeners are notified synchronously that the scrollbars have been
  updated.
- The viewport itself listens to the scrollbars, so it receives a
  notification and updates the position of the content component.
- The scrollbar position (quantised to an integer) resolves to a
  component position (also quantised to an integer) that differs from
  the existing position, so the new position is applied.
- The viewport now attempts to set the scrollbars to the correct
  position in response, and notifies listeners that the scrollbars
  have moved...

Normally, the recursion would exit at the point where the component position
is set to its current position. If we're unlucky, though, converting
from view pos to scrollbar pos, then scrollbar pos back to view pos may
result in a view pos that differs from the original value.

This fix adds a new exit condition from the recursion. On receiving a
scrollbar move notification, we check whether the scrollbar position
computed from the current view position matches the incoming scrollbar
position. If it does, there's no need to compute and apply a new view
position from the incoming scrollbar position.
---
 .../juce_gui_basics/layout/juce_Viewport.cpp  | 41 ++++++++++++-------
 .../juce_gui_basics/layout/juce_Viewport.h    |  1 +
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/modules/juce_gui_basics/layout/juce_Viewport.cpp b/modules/juce_gui_basics/layout/juce_Viewport.cpp
index b01851ba27..21463288c6 100644
--- a/modules/juce_gui_basics/layout/juce_Viewport.cpp
+++ b/modules/juce_gui_basics/layout/juce_Viewport.cpp
@@ -247,14 +247,22 @@ Point<int> Viewport::viewportPosToCompPos (Point<int> pos) const
 {
     jassert (contentComp != nullptr);
 
-    auto contentBounds = contentHolder.getLocalArea (contentComp.get(), contentComp->getLocalBounds());
+    const auto contentBounds = getContentBounds();
 
-    Point<int> p (jmax (jmin (0, contentHolder.getWidth()  - contentBounds.getWidth()),  jmin (0, -(pos.x))),
-                  jmax (jmin (0, contentHolder.getHeight() - contentBounds.getHeight()), jmin (0, -(pos.y))));
+    const Point p (jmax (jmin (0, contentHolder.getWidth()  - contentBounds.getWidth()),  jmin (0, -(pos.x))),
+                   jmax (jmin (0, contentHolder.getHeight() - contentBounds.getHeight()), jmin (0, -(pos.y))));
 
     return p.transformedBy (contentComp->getTransform().inverted());
 }
 
+Rectangle<int> Viewport::getContentBounds() const
+{
+    if (auto* cc = contentComp.get())
+        return contentHolder.getLocalArea (cc, cc->getLocalBounds());
+
+    return {};
+}
+
 void Viewport::setViewPosition (const int xPixelsOffset, const int yPixelsOffset)
 {
     setViewPosition ({ xPixelsOffset, yPixelsOffset });
@@ -406,11 +414,7 @@ void Viewport::updateVisibleArea()
             break;
     }
 
-    Rectangle<int> contentBounds;
-
-    if (auto cc = contentComp.get())
-        contentBounds = contentHolder.getLocalArea (cc, cc->getLocalBounds());
-
+    const auto contentBounds = getContentBounds();
     auto visibleOrigin = -contentBounds.getPosition();
 
     auto& hbar = getHorizontalScrollBar();
@@ -521,15 +525,22 @@ int Viewport::getScrollBarThickness() const
 
 void Viewport::scrollBarMoved (ScrollBar* scrollBarThatHasMoved, double newRangeStart)
 {
-    auto newRangeStartInt = roundToInt (newRangeStart);
+    const auto contentOrigin = -getContentBounds().getPosition();
+    const auto newRangeStartInt = roundToInt (newRangeStart);
 
-    if (scrollBarThatHasMoved == horizontalScrollBar.get())
+    for (const auto& [member, bar] : { std::tuple (&Point<int>::x, horizontalScrollBar.get()),
+                                       std::tuple (&Point<int>::y, verticalScrollBar.get()) })
     {
-        setViewPosition (newRangeStartInt, getViewPositionY());
-    }
-    else if (scrollBarThatHasMoved == verticalScrollBar.get())
-    {
-        setViewPosition (getViewPositionX(), newRangeStartInt);
+        if (scrollBarThatHasMoved != bar)
+            continue;
+
+        if (contentOrigin.*member == newRangeStartInt)
+            return;
+
+        auto pt = getViewPosition();
+        pt.*member = newRangeStartInt;
+        setViewPosition (pt);
+        return;
     }
 }
 
diff --git a/modules/juce_gui_basics/layout/juce_Viewport.h b/modules/juce_gui_basics/layout/juce_Viewport.h
index f87a19de60..2ab00f802c 100644
--- a/modules/juce_gui_basics/layout/juce_Viewport.h
+++ b/modules/juce_gui_basics/layout/juce_Viewport.h
@@ -373,6 +373,7 @@ private:
     std::unique_ptr<DragToScrollListener> dragToScrollListener;
 
     Point<int> viewportPosToCompPos (Point<int>) const;
+    Rectangle<int> getContentBounds() const;
 
     void updateVisibleArea();
     void deleteOrRemoveContentComp();