Просмотр исходного кода

LibWeb: Improve vertical margin collapse between adjacent blocks

Collect all the preceding block-level siblings whose vertical margins
are collapsible. Both margin-top and margin-bottom now (previously,
we only considered the margin-bottom of siblings.)

Use the right margin in part-negative and all-negative situations.
Andreas Kling 3 лет назад
Родитель
Сommit
c02e6f991a

+ 36 - 22
Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp

@@ -329,9 +329,8 @@ void BlockFormattingContext::compute_height(Box const& box, FormattingState& sta
 
     auto& box_state = state.get_mutable(box);
 
-    // FIXME: While negative values are generally allowed for margins, for now just ignore those for height calculation
-    box_state.margin_top = max(computed_values.margin().top.resolved(box, width_of_containing_block_as_length).to_px(box), 0);
-    box_state.margin_bottom = max(computed_values.margin().bottom.resolved(box, width_of_containing_block_as_length).to_px(box), 0);
+    box_state.margin_top = computed_values.margin().top.resolved(box, width_of_containing_block_as_length).to_px(box);
+    box_state.margin_bottom = computed_values.margin().bottom.resolved(box, width_of_containing_block_as_length).to_px(box);
 
     box_state.border_top = computed_values.border_top().width;
     box_state.border_bottom = computed_values.border_bottom().width;
@@ -439,42 +438,57 @@ void BlockFormattingContext::place_block_level_element_in_normal_flow_vertically
 
     compute_vertical_box_model_metrics(child_box, containing_block);
 
-    float y = box_state.margin_box_top()
+    float y = box_state.border_box_top()
         + box_state.offset_top;
 
-    // NOTE: Empty (0-height) preceding siblings have their margins collapsed with *their* preceding sibling, etc.
-    float collapsed_bottom_margin_of_preceding_siblings = 0;
+    Vector<float> collapsible_margins;
 
     auto* relevant_sibling = child_box.previous_sibling_of_type<Layout::BlockContainer>();
     while (relevant_sibling != nullptr) {
         if (!relevant_sibling->is_absolutely_positioned() && !relevant_sibling->is_floating()) {
             auto const& relevant_sibling_state = m_state.get(*relevant_sibling);
-            collapsed_bottom_margin_of_preceding_siblings = max(collapsed_bottom_margin_of_preceding_siblings, relevant_sibling_state.margin_bottom);
+            collapsible_margins.append(relevant_sibling_state.margin_bottom);
+            // NOTE: Empty (0-height) preceding siblings have their margins collapsed with *their* preceding sibling, etc.
             if (relevant_sibling_state.border_box_height() > 0)
                 break;
+            collapsible_margins.append(relevant_sibling_state.margin_top);
         }
         relevant_sibling = relevant_sibling->previous_sibling_of_type<Layout::BlockContainer>();
     }
 
     if (relevant_sibling) {
+        // Collapse top margin with the collapsed margin(s) of preceding siblings.
+        collapsible_margins.append(box_state.margin_top);
+
+        float smallest_margin = 0;
+        float largest_margin = 0;
+        size_t negative_margin_count = 0;
+        for (auto margin : collapsible_margins) {
+            if (margin < 0)
+                ++negative_margin_count;
+            largest_margin = max(largest_margin, margin);
+            smallest_margin = min(smallest_margin, margin);
+        }
+
+        float collapsed_margin = 0;
+        if (negative_margin_count == collapsible_margins.size()) {
+            // When all margins are negative, the size of the collapsed margin is the smallest (most negative) margin.
+            collapsed_margin = smallest_margin;
+        } else if (negative_margin_count > 0) {
+            // When negative margins are involved, the size of the collapsed margin is the sum of the largest positive margin and the smallest (most negative) negative margin.
+            collapsed_margin = largest_margin + smallest_margin;
+        } else {
+            // Otherwise, collapse all the adjacent margins by using only the largest one.
+            collapsed_margin = largest_margin;
+        }
+
         auto const& relevant_sibling_state = m_state.get(*relevant_sibling);
         y += relevant_sibling_state.offset.y()
             + relevant_sibling_state.content_height
-            + relevant_sibling_state.border_box_bottom();
-
-        // Collapse top margin with bottom margin of preceding siblings if needed
-        float my_margin_top = box_state.margin_top;
-
-        if (my_margin_top < 0 || collapsed_bottom_margin_of_preceding_siblings < 0) {
-            // Negative margins present.
-            float largest_negative_margin = -min(my_margin_top, collapsed_bottom_margin_of_preceding_siblings);
-            float largest_positive_margin = (my_margin_top < 0 && collapsed_bottom_margin_of_preceding_siblings < 0) ? 0 : max(my_margin_top, collapsed_bottom_margin_of_preceding_siblings);
-            float final_margin = largest_positive_margin - largest_negative_margin;
-            y += final_margin - my_margin_top;
-        } else if (collapsed_bottom_margin_of_preceding_siblings > my_margin_top) {
-            // Sibling's margin is larger than mine, adjust so we use sibling's.
-            y += collapsed_bottom_margin_of_preceding_siblings - my_margin_top;
-        }
+            + relevant_sibling_state.border_box_bottom()
+            + collapsed_margin;
+    } else {
+        y += box_state.margin_top;
     }
 
     auto clear_floating_boxes = [&](FloatSideData& float_side) {

+ 5 - 3
Userland/Libraries/LibWeb/Layout/FormattingContext.cpp

@@ -286,8 +286,9 @@ float FormattingContext::compute_auto_height_for_block_formatting_context_root(F
 
             auto const& child_box_state = state.get(child_box);
 
-            float child_box_top = child_box_state.offset.y() - child_box_state.margin_box_top();
-            float child_box_bottom = child_box_state.offset.y() + child_box_state.content_height + child_box_state.margin_box_bottom();
+            // FIXME: We're ignoring negative margins here, figure out the correct thing to do.
+            float child_box_top = child_box_state.offset.y() - child_box_state.border_box_top() - max(0, child_box_state.margin_top);
+            float child_box_bottom = child_box_state.offset.y() + child_box_state.content_height + child_box_state.border_box_bottom() + max(0, child_box_state.margin_bottom);
 
             if (!top.has_value() || child_box_top < top.value())
                 top = child_box_top;
@@ -305,7 +306,8 @@ float FormattingContext::compute_auto_height_for_block_formatting_context_root(F
                 return IterationDecision::Continue;
 
             auto const& child_box_state = state.get(child_box);
-            float child_box_bottom = child_box_state.offset.y() + child_box_state.content_height + child_box_state.margin_box_bottom();
+            // FIXME: We're ignoring negative margins here, figure out the correct thing to do.
+            float child_box_bottom = child_box_state.offset.y() + child_box_state.content_height + child_box_state.border_box_bottom() + max(0, child_box_state.margin_bottom);
 
             if (!bottom.has_value() || child_box_bottom > bottom.value())
                 bottom = child_box_bottom;