Defer scroll offset clamping to after layout, fixes #452

Include/RmlUi/Core/Element.h

@@ -55,6 +55,7 @@ class ElementDefinition;
 class ElementDocument;
 class ElementScroll;
 class ElementStyle;
+class LayoutEngine;
 class ContainerBox;
 class InlineLevelBox;
 class ReplacedBox;
@@ -140,7 +141,8 @@ public:
 	/// Sets the dimensions of the element's scrollable overflow rectangle. This is the tightest fitting box surrounding
 	/// all of this element's logical children, and the element's padding box.
 	/// @param[in] scrollable_overflow_rectangle The dimensions of the box's scrollable content.
-	void SetScrollableOverflowRectangle(Vector2f scrollable_overflow_rectangle);
+	/// @param[in] clamp_scroll_offset Clamp scroll offset to the new rectangle, should only be set when the final overflow size has been determined.
+	void SetScrollableOverflowRectangle(Vector2f scrollable_overflow_rectangle, bool clamp_scroll_offset);
 	/// Sets the box describing the size of the element, and removes all others.
 	/// @param[in] box The new dimensions box for the element.
 	void SetBox(const Box& box);
@@ -692,6 +694,9 @@ private:
 	void OnDpRatioChangeRecursive();
 	void DirtyFontFaceRecursive();
 
+	void ClampScrollOffset();
+	void ClampScrollOffsetRecursive();
+
 	/// Start an animation, replacing any existing animations of the same property name. If start_value is null, the element's current value is used.
 	ElementAnimationList::iterator StartAnimation(PropertyId property_id, const Property* start_value, int num_iterations, bool alternate_direction,
 		float delay, bool initiated_by_animation_property);
@@ -797,6 +802,7 @@ private:
 	friend class Rml::ContainerBox;
 	friend class Rml::InlineLevelBox;
 	friend class Rml::ReplacedBox;
+	friend class Rml::LayoutEngine;
 	friend class Rml::ElementScroll;
 	friend RMLUICORE_API void Rml::ReleaseFontResources();
 };

Source/Core/Element.cpp

@@ -429,15 +429,13 @@ BoxArea Element::GetClipArea() const
 	return clip_area;
 }
 
-void Element::SetScrollableOverflowRectangle(Vector2f _scrollable_overflow_rectangle)
+void Element::SetScrollableOverflowRectangle(Vector2f _scrollable_overflow_rectangle, bool clamp_scroll_offset)
 {
 	if (scrollable_overflow_rectangle != _scrollable_overflow_rectangle)
 	{
 		scrollable_overflow_rectangle = _scrollable_overflow_rectangle;
-
-		scroll_offset.x = Math::Min(scroll_offset.x, GetScrollWidth() - GetClientWidth());
-		scroll_offset.y = Math::Min(scroll_offset.y, GetScrollHeight() - GetClientHeight());
-		DirtyAbsoluteOffset();
+		if (clamp_scroll_offset)
+			ClampScrollOffset();
 	}
 }
 
@@ -2907,4 +2905,26 @@ void Element::DirtyFontFaceRecursive()
 		GetChild(i)->DirtyFontFaceRecursive();
 }
 
+void Element::ClampScrollOffset()
+{
+	const Vector2f new_scroll_offset = {
+		Math::Min(scroll_offset.x, GetScrollWidth() - GetClientWidth()),
+		Math::Min(scroll_offset.y, GetScrollHeight() - GetClientHeight()),
+	};
+
+	if (new_scroll_offset != scroll_offset)
+	{
+		scroll_offset = new_scroll_offset;
+		DirtyAbsoluteOffset();
+	}
+}
+
+void Element::ClampScrollOffsetRecursive()
+{
+	ClampScrollOffset();
+	const int num_children = GetNumChildren(true);
+	for (int i = 0; i < num_children; ++i)
+		GetChild(i)->ClampScrollOffsetRecursive();
+}
+
 } // namespace Rml

Source/Core/Elements/WidgetTextInput.cpp

@@ -657,7 +657,7 @@ void WidgetTextInput::ProcessEvent(Event& event)
 		{
 			parent->SetPseudoClass("focus-visible", true);
 			if (UpdateSelection(false))
-				FormatElement();
+				FormatText();
 			ShowCursor(true, false);
 
 			if (TextInputHandler* handler = GetTextInputHandler())
@@ -678,7 +678,7 @@ void WidgetTextInput::ProcessEvent(Event& event)
 			if (TextInputHandler* handler = GetTextInputHandler())
 				handler->OnDeactivate(text_input_context.get());
 			if (ClearSelection())
-				FormatElement();
+				FormatText();
 			ShowCursor(false, false);
 		}
 	}
@@ -1218,7 +1218,7 @@ void WidgetTextInput::FormatElement()
 
 	// For text elements, make the content and padding on all sides reachable by scrolling.
 	const Vector2f padding_size = parent->GetBox().GetFrameSize(BoxArea::Padding);
-	parent->SetScrollableOverflowRectangle(content_area + padding_size);
+	parent->SetScrollableOverflowRectangle(content_area + padding_size, true);
 	scroll->FormatScrollbars();
 }
 

Source/Core/Layout/ContainerBox.cpp

@@ -216,17 +216,18 @@ bool ContainerBox::SubmitBox(const Vector2f content_overflow_size, const Box& bo
 			is_scroll_container ? element->GetElementScroll()->GetScrollbarSize(ElementScroll::HORIZONTAL) : 0.f,
 		};
 
+		element->SetBox(box);
+
 		// Scrollable overflow is the set of things extending our padding area, for which scrolling could be provided.
 		const Vector2f scrollable_overflow_size = Math::Max(padding_size - scrollbar_size, padding_top_left + content_overflow_size);
-
-		element->SetBox(box);
-		element->SetScrollableOverflowRectangle(scrollable_overflow_size);
+		// Set the overflow size but defer clamping of the scroll offset, see `LayoutEngine::FormatElement`.
+		element->SetScrollableOverflowRectangle(scrollable_overflow_size, false);
 
 		const Vector2f border_size = padding_size + box.GetFrameSize(BoxArea::Border);
 
 		// Set the visible overflow size so that ancestors can catch any overflow produced by us. That is, hiding it or
-		// providing a scrolling mechanism. If this box is a scroll container, we catch our own overflow here; then,
-		// just use the normal margin box as that will effectively remove the overflow from our ancestor's perspective.
+		// providing a scrolling mechanism. If this box is a scroll container we catch our own overflow here. Thus, in
+		// this case, only our border box is visible from our ancestor's perpective.
 		if (is_scroll_container)
 		{
 			visible_overflow_size = border_size;

Source/Core/Layout/LayoutEngine.cpp

@@ -29,6 +29,7 @@
 #include "LayoutEngine.h"
 #include "../../../Include/RmlUi/Core/Element.h"
 #include "../../../Include/RmlUi/Core/Log.h"
+#include "../../../Include/RmlUi/Core/Profiling.h"
 #include "ContainerBox.h"
 #include "FormattingContext.h"
 
@@ -45,6 +46,16 @@ void LayoutEngine::FormatElement(Element* element, Vector2f containing_block)
 	{
 		Log::Message(Log::LT_ERROR, "Error while formatting element: %s", element->GetAddress().c_str());
 	}
+
+	{
+		RMLUI_ZoneScopedN("ClampScrollOffsetRecursive");
+		// The size of the scrollable area might have changed, so clamp the scroll offset to avoid scrolling outside the
+		// scrollable area. During layouting, we might be changing the scrollable overflow area of the element several
+		// times, such as after enabling scrollbars. For this reason, we don't clamp the scroll offset during layouting,
+		// as that could inadvertently clamp it to a temporary size. Now that we know the final layout, including the
+		// size of each element's scrollable area, we can finally clamp the scroll offset.
+		element->ClampScrollOffsetRecursive();
+	}
 }
 
 } // namespace Rml

changelog.md

@@ -237,6 +237,7 @@ The following improvements apply to both the textarea and text input elements.
 
 ### General fixes
 
+- Fix some situations where the scroll offset of an element would reset or change after layout updates. #452
 - Fix wrong logic for assertion of released textures. #589 (thanks @pgruenbacher)
 - Fix some situations where units were not shown in properties, ensure all invoked types define a string converter.
 - In `demo` sample, fix form submit animation not playing smoothly on power saving mode.