LibWeb: Invalidate descendant paint caches on text decoration changes

Previously, changing a text decoration only invalidated the cached paint
commands of the decorating box itself. Descendant boxes record
propagated decorations in their own cached commands, so they kept
painting the old decoration.

Text decoration changes now also invalidate every descendant paint
cache up to the propagation boundaries. Underline offset and position
changes take this path as well: they are repaints rather than
relayouts, and a descendant that overrides either inherited property
keeps an unchanged style when the decorating box changes them, so
nothing else would repaint it.

Author: tcl3

Libraries/LibWeb/Animations/AnimationEffect.cpp

@@ -932,10 +932,21 @@ AnimationUpdateContext::~AnimationUpdateContext()
 
         if (invalidation.needs_repaint()) {
             if (element.pseudo_element().has_value()) {
-                if (auto pseudo_element_node = target->pseudo_element_unsafe_layout_node(*element.pseudo_element()); pseudo_element_node && pseudo_element_node->paintable())
-                    pseudo_element_node->paintable()->set_needs_repaint();
+                if (auto pseudo_element_node = target->pseudo_element_unsafe_layout_node(*element.pseudo_element())) {
+                    if (auto paintable = pseudo_element_node->paintable()) {
+                        paintable->set_needs_repaint();
+                        if (invalidation.repaint_propagated_text_decorations)
+                            paintable->invalidate_propagated_text_decoration_caches();
+                    }
+                }
             } else {
                 target->set_needs_repaint();
+                if (invalidation.repaint_propagated_text_decorations) {
+                    if (auto* layout_node = target->unsafe_layout_node()) {
+                        if (auto paintable = layout_node->paintable())
+                            paintable->invalidate_propagated_text_decoration_caches();
+                    }
+                }
             }
         }
         if (invalidation.needs_stacking_context_tree_rebuild())

Libraries/LibWeb/CSS/Properties.json

@@ -4445,6 +4445,7 @@
     ]
   },
   "text-underline-offset": {
+    "affects-layout": false,
     "animation-type": "by-computed-value",
     "inherited": true,
     "initial": "auto",
@@ -4459,6 +4460,7 @@
     "percentages-resolve-to": "length"
   },
   "text-underline-position": {
+    "affects-layout": false,
     "animation-type": "discrete",
     "inherited": true,
     "initial": "auto",

Libraries/LibWeb/CSS/StyleInvalidation.cpp

@@ -345,6 +345,13 @@ RequiredInvalidationAfterStyleChange compute_property_invalidation(CSS::Property
     if (AK::first_is_one_of(property_id, CSS::PropertyID::ContainerName, CSS::PropertyID::ContainerType))
         invalidation.recompute_descendant_styles = true;
 
+    // Text decorations propagate to descendant boxes, which paint them from this element's computed
+    // values, so their cached paint commands must be discarded even though their style is unchanged.
+    if (AK::first_is_one_of(property_id, CSS::PropertyID::TextDecorationLine, CSS::PropertyID::TextDecorationColor,
+            CSS::PropertyID::TextDecorationStyle, CSS::PropertyID::TextDecorationThickness,
+            CSS::PropertyID::TextUnderlineOffset, CSS::PropertyID::TextUnderlinePosition))
+        invalidation.repaint_propagated_text_decorations = true;
+
     // OPTIMIZATION: Special handling for CSS `visibility`:
     if (property_id == CSS::PropertyID::Visibility) {
         // We don't need to relayout if the visibility changes from visible to hidden or vice versa. Only collapse requires relayout.

Libraries/LibWeb/CSS/StyleInvalidation.h

@@ -57,6 +57,9 @@ struct RequiredInvalidationAfterStyleChange {
     // descendants. Containing block pointers are only recomputed by a full layout pass, so
     // partial relayout boundary qualification cannot be trusted until one runs.
     bool changes_containing_block_establishment : 1 { false };
+    // A property controlling decorations originated by this box changed. Descendant boxes paint
+    // the propagated decorations from these values, so their cached paint commands are stale.
+    bool repaint_propagated_text_decorations : 1 { false };
 
     void operator|=(RequiredInvalidationAfterStyleChange const& other)
     {
@@ -67,6 +70,7 @@ struct RequiredInvalidationAfterStyleChange {
         recompute_descendant_styles |= other.recompute_descendant_styles;
         inherited_style_changed |= other.inherited_style_changed;
         changes_containing_block_establishment |= other.changes_containing_block_establishment;
+        repaint_propagated_text_decorations |= other.repaint_propagated_text_decorations;
     }
 
     [[nodiscard]] bool is_none() const
@@ -76,7 +80,8 @@ struct RequiredInvalidationAfterStyleChange {
             && !m_needs_scrollable_overflow_recalculation
             && !recompute_descendant_styles
             && !inherited_style_changed
-            && !changes_containing_block_establishment;
+            && !changes_containing_block_establishment
+            && !repaint_propagated_text_decorations;
     }
 
     static RequiredInvalidationAfterStyleChange full()

Libraries/LibWeb/DOM/Element.cpp

@@ -1154,8 +1154,13 @@ void Element::apply_computed_style_to_layout_node_if_needed(CSS::RequiredInvalid
     auto element_computed_values = m_computed_values;
     VERIFY(element_computed_values);
     unsafe_layout_node()->apply_style(element_computed_values.release_nonnull());
-    if (invalidation.needs_repaint())
+    if (invalidation.needs_repaint()) {
         set_needs_repaint();
+        if (invalidation.repaint_propagated_text_decorations) {
+            if (auto paintable = unsafe_layout_node()->paintable())
+                paintable->invalidate_propagated_text_decoration_caches();
+        }
+    }
 
     // Do the same for pseudo-elements.
     for_each_synthetic_pseudo_element([&](CSS::PseudoElement pseudo_element_type, SyntheticPseudoElement const& pseudo_element) {
@@ -1165,8 +1170,11 @@ void Element::apply_computed_style_to_layout_node_if_needed(CSS::RequiredInvalid
 
         if (auto node_with_style = pseudo_element.unsafe_layout_node()) {
             node_with_style->apply_style(pseudo_element_style.release_nonnull());
-            if (invalidation.needs_repaint() && node_with_style->paintable())
-                node_with_style->paintable()->set_needs_repaint();
+            if (auto paintable = node_with_style->paintable(); paintable && invalidation.needs_repaint()) {
+                paintable->set_needs_repaint();
+                if (invalidation.repaint_propagated_text_decorations)
+                    paintable->invalidate_propagated_text_decoration_caches();
+            }
         }
     });
 }

Libraries/LibWeb/Painting/Paintable.cpp

@@ -1041,6 +1041,18 @@ void Paintable::invalidate_paint_cache() const
     m_cached_paint_data = nullptr;
 }
 
+// Descendant boxes paint this box's propagated text decorations from its computed values, so their
+// cached commands go stale when those values change even though the descendants' style is unchanged.
+void Paintable::invalidate_propagated_text_decoration_caches() const
+{
+    for_each_in_subtree([](Paintable const& descendant) {
+        if (descendant.layout_node().is_text_decoration_propagation_boundary())
+            return TraversalDecision::SkipChildrenAndContinue;
+        descendant.invalidate_paint_cache();
+        return TraversalDecision::Continue;
+    });
+}
+
 void Paintable::set_cached_commands(PaintPhase phase, ByteBuffer const& commands) const
 {
     if (!m_cached_paint_data)

Libraries/LibWeb/Painting/Paintable.h

@@ -409,6 +409,7 @@ class WEB_API Paintable
     static constexpr size_t paint_phase_count = to_underlying(PaintPhase::Overlay) + 1;
 
     void invalidate_paint_cache() const;
+    void invalidate_propagated_text_decoration_caches() const;
 
     bool has_cached_commands(PaintPhase) const;
     ReadonlyBytes cached_commands(PaintPhase) const;

Tests/LibWeb/Ref/expected/text-decoration-propagation-repaint-ref.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<style>
+    #color { text-decoration: underline red; }
+
+    #offset {
+        text-decoration: underline;
+        text-decoration-skip-ink: none;
+        text-underline-offset: 12px;
+    }
+
+    #offset-descendant { text-underline-offset: 0; }
+
+    #position {
+        text-decoration: underline;
+        text-decoration-skip-ink: none;
+        text-underline-position: under;
+    }
+
+    #position-descendant { text-underline-position: auto; }
+</style>
+<span id="color"><div>This text must have a red underline.</div></span>
+<span><div>This text must not have an underline.</div></span>
+<span id="offset">before <div id="offset-descendant">nested</div> after</span>
+<span id="position">before <div id="position-descendant">nested</div> after</span>

Tests/LibWeb/Ref/input/text-decoration-propagation-repaint.html

@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<link rel="match" href="../expected/text-decoration-propagation-repaint-ref.html">
+<style>
+    #color { text-decoration: underline transparent; }
+    #color.on { text-decoration: underline red; }
+
+    #removal.on { text-decoration: underline red; }
+
+    #offset {
+        text-decoration: underline;
+        text-decoration-skip-ink: none;
+        text-underline-offset: 0;
+    }
+
+    #offset.on { text-underline-offset: 12px; }
+    #offset-descendant { text-underline-offset: 0; }
+
+    #position {
+        text-decoration: underline;
+        text-decoration-skip-ink: none;
+        text-underline-position: auto;
+    }
+
+    #position.on { text-underline-position: under; }
+    #position-descendant { text-underline-position: auto; }
+</style>
+<span id="color"><div>This text must have a red underline.</div></span>
+<span id="removal" class="on"><div>This text must not have an underline.</div></span>
+<span id="offset">before <div id="offset-descendant">nested</div> after</span>
+<span id="position">before <div id="position-descendant">nested</div> after</span>
+<script>
+    requestAnimationFrame(() => {
+        requestAnimationFrame(() => {
+            for (let id of ["color", "offset", "position"])
+                document.getElementById(id).classList.add("on");
+            document.getElementById("removal").classList.remove("on");
+            requestAnimationFrame(() => {
+                if (window.internals)
+                    internals.signalTestIsDone("PASS");
+                else
+                    document.documentElement.classList.remove("reftest-wait");
+            });
+        });
+    });
+</script>
+</html>