Fix some issues with absolute offsets not being properly fixed

Allow setting the relative offset from another box area.

Include/RmlUi/Core/Element.h

@@ -121,7 +121,7 @@ public:
 	/// @param[in] offset The offset (in pixels) of our primary box's top-left border corner from our offset parent's top-left border corner.
 	/// @param[in] offset_parent The element this element is being positioned relative to.
 	/// @param[in] offset_fixed True if the element is fixed in place (and will not scroll), false if not.
-	void SetOffset(Vector2f offset, Element* offset_parent, bool offset_fixed = false);
+	void SetOffset(Vector2f offset, Element* offset_parent, bool offset_fixed = false, BoxArea relative_offset_area = BoxArea::Border);
 	/// Returns the position of the top-left corner of one of the areas of this element's primary box, relative to its
 	/// offset parent's top-left border corner.
 	/// @param[in] area The desired area position.
@@ -738,8 +738,10 @@ private:
 
 	bool visible; // True if the element is visible and active.
 
-	bool offset_fixed;
 	bool absolute_offset_dirty;
+	bool offset_fixed : 1;
+
+	BoxArea relative_offset_area : 2;
 	bool rounded_main_padding_size_dirty : 1;
 
 	bool dirty_definition : 1; // Implies dirty child definitions as well.

Include/RmlUi/Core/Types.h

@@ -43,7 +43,7 @@ using byte = unsigned char;
 using ScriptObject = void*;
 
 enum class Character : char32_t { Null, Replacement = 0xfffd }; // Unicode code point
-enum class BoxArea { Margin, Border, Padding, Content, Auto };
+enum class BoxArea : uint8_t { Margin, Border, Padding, Content, Auto };
 
 } // namespace Rml
 

Source/Core/Element.cpp

@@ -94,9 +94,9 @@ static float GetScrollOffsetDelta(ScrollAlignment alignment, float begin_offset,
 
 Element::Element(const String& tag) :
 	local_stacking_context(false), local_stacking_context_forced(false), stacking_context_dirty(false), computed_values_are_default_initialized(true),
-	visible(true), offset_fixed(false), absolute_offset_dirty(true), rounded_main_padding_size_dirty(true), dirty_definition(false),
-	dirty_child_definitions(false), dirty_animation(false), dirty_transition(false), dirty_transform(false), dirty_perspective(false), tag(tag),
-	relative_offset_base(0, 0), relative_offset_position(0, 0), absolute_offset(0, 0), scroll_offset(0, 0)
+	visible(true), absolute_offset_dirty(true), offset_fixed(false), relative_offset_area(BoxArea::Border), rounded_main_padding_size_dirty(true),
+	dirty_definition(false), dirty_child_definitions(false), dirty_animation(false), dirty_transition(false), dirty_transform(false),
+	dirty_perspective(false), tag(tag), relative_offset_base(0, 0), relative_offset_position(0, 0), absolute_offset(0, 0), scroll_offset(0, 0)
 {
 	RMLUI_ASSERT(tag == StringUtilities::ToLower(tag));
 	parent = nullptr;
@@ -347,17 +347,19 @@ String Element::GetAddress(bool include_pseudo_classes, bool include_parents) co
 		return address;
 }
 
-void Element::SetOffset(Vector2f offset, Element* _offset_parent, bool _offset_fixed)
+void Element::SetOffset(Vector2f offset, Element* _offset_parent, bool _offset_fixed, BoxArea _relative_offset_area)
 {
 	_offset_fixed |= GetPosition() == Style::Position::Fixed;
 
 	// If our offset has definitely changed, or any of our parenting has, then these are set and
 	// updated based on our left / right / top / bottom properties.
-	if (relative_offset_base != offset || offset_parent != _offset_parent || offset_fixed != _offset_fixed)
+	if (relative_offset_base != offset || offset_parent != _offset_parent || offset_fixed != _offset_fixed ||
+		relative_offset_area != _relative_offset_area)
 	{
 		relative_offset_base = offset;
 		offset_fixed = _offset_fixed;
 		offset_parent = _offset_parent;
+		relative_offset_area = _relative_offset_area;
 		UpdateRelativeOffsetFromInsetConstraints();
 		DirtyAbsoluteOffset();
 	}
@@ -378,13 +380,13 @@ void Element::SetOffset(Vector2f offset, Element* _offset_parent, bool _offset_f
 
 Vector2f Element::GetRelativeOffset(BoxArea area)
 {
-	return relative_offset_base + relative_offset_position + GetBox().GetPosition(area);
+	return relative_offset_base + relative_offset_position + GetBox().GetPosition(area) - GetBox().GetPosition(relative_offset_area);
 }
 
 Vector2f Element::GetAbsoluteOffset(BoxArea area)
 {
 	UpdateAbsoluteOffsetAndRenderBoxData();
-	return area == BoxArea::Border ? absolute_offset : absolute_offset + GetBox().GetPosition(area);
+	return absolute_offset + GetBox().GetPosition(area);
 }
 
 void Element::UpdateAbsoluteOffsetAndRenderBoxData()
@@ -409,7 +411,7 @@ void Element::UpdateAbsoluteOffsetAndRenderBoxData()
 				offset_from_ancestors += ancestor->relative_offset_position;
 		}
 
-		const Vector2f relative_offset = relative_offset_base + relative_offset_position;
+		const Vector2f relative_offset = relative_offset_base + relative_offset_position - GetBox().GetPosition(relative_offset_area);
 		absolute_offset = relative_offset + offset_from_ancestors;
 
 		// Next, we find the rounded size of the box so that elements can be placed border-to-border next to each other
@@ -964,12 +966,12 @@ Element* Element::GetOffsetParent()
 
 float Element::GetOffsetLeft()
 {
-	return relative_offset_base.x + relative_offset_position.x;
+	return relative_offset_base.x + relative_offset_position.x - GetBox().GetPosition(relative_offset_area).x;
 }
 
 float Element::GetOffsetTop()
 {
-	return relative_offset_base.y + relative_offset_position.y;
+	return relative_offset_base.y + relative_offset_position.y - GetBox().GetPosition(relative_offset_area).y;
 }
 
 float Element::GetOffsetWidth()

Source/Core/ElementDocument.cpp

@@ -570,6 +570,9 @@ void ElementDocument::UpdateLayout()
 			LayoutEngine::FormatElement(element, committed_layout->containing_block_size,
 				committed_layout->absolutely_positioning_containing_block_size);
 
+			// TODO: A bit ugly
+			element->UpdateRelativeOffsetFromInsetConstraints();
+
 			return ElementUtilities::CallbackControlFlow::SkipChildren;
 		});
 
@@ -580,6 +583,8 @@ void ElementDocument::UpdateLayout()
 				containing_block = parent->GetBox().GetSize();
 
 			LayoutEngine::FormatElement(this, containing_block, containing_block);
+			// TODO: A bit ugly
+			this->UpdateRelativeOffsetFromInsetConstraints();
 		}
 
 		if (!any_layout_updates)

Source/Core/Layout/BlockFormattingContext.cpp

@@ -237,6 +237,21 @@ bool BlockFormattingContext::FormatBlockContainerChild(BlockContainer* parent_co
 	const Style::Position position_property = computed.position();
 	if (position_property == Style::Position::Absolute || position_property == Style::Position::Fixed)
 	{
+		// TODO: When laying out an absolutely positioned element independently, we need to do the equivalent of this,
+		// and the following `ContainerBox::ClosePositionedElements`. Do we need to store the static position and parent
+		// container element? Hmm, I guess we can assume that the static position stays the same, since otherwise we
+		// would have had an update occur in our parent, thereby closing it in the formatting context of the static
+		// element. ... Looking more closely at it, looks like all we need is to call
+		// `UpdateRelativeOffsetFromInsetConstraints`?
+		// TODO Part 2: UpdateRelativeOffsetFromInsetConstraints alone solves some problems, but not all. Another issue
+		// is when the box on the absolute element is changed, particularly margin edges. Then `ClosePositionedElements`
+		// would change the submitted offset. Should we store this in the layout cache? Should we store the static
+		// position and element, directly on the element? Maybe either/or that we are using the element's margin. Hmm,
+		// don't really like that. Can we know that we are in a block formatting context? No, shouldn't depend directly
+		// on that. Hacky solution: If absolutely positioned element layed out separately, check changed margin edges,
+		// and apply the diff to SetOffset with otherwise the same as existing.
+		// Can we set the static position directly on the element (here basically), and just call a function to update
+		// everything at the end?
 		const Vector2f static_position = parent_container->GetOpenStaticPosition(display) - parent_container->GetPosition();
 		parent_container->AddAbsoluteElement(element, static_position, parent_container->GetElement());
 		return true;

Source/Core/Layout/ContainerBox.cpp

@@ -157,14 +157,10 @@ void ContainerBox::ClosePositionedElements()
 			// Lay out the element.
 			FormattingContext::FormatIndependent(this, absolute_element, nullptr, FormattingContextType::Block);
 
-			// Now that the element's box has been built, we can offset the position we determined was appropriate for
-			// it by the element's margin. This is necessary because the coordinate system for the box begins at the
-			// border, not the margin.
-			offset.x += absolute_element->GetBox().GetEdge(BoxArea::Margin, BoxEdge::Left);
-			offset.y += absolute_element->GetBox().GetEdge(BoxArea::Margin, BoxEdge::Top);
-
 			// Set the offset of the element; the element itself will take care of any RCSS-defined positional offsets.
-			absolute_element->SetOffset(offset, element);
+			// Use the margin box area as the origin, so that the element can modify its own box and have that reflected
+			// in its position, without having to do a new layout run in its this element.
+			absolute_element->SetOffset(offset, element, false, BoxArea::Margin);
 		}
 	}
 }