Attachment 460256 Details for Bug 241546 – Patch for landing

[patch] Patch for landing

bug-241546-20220615214921.patch (text/plain), 12.42 KB, created by Antti Koivisto on 2022-06-15 11:49:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2022-06-15 11:49:23 PDT

Size: 12.42 KB

patch

obsolete

>From 1edf60cd802a5fb2a27ebcde1a318c450d72385c Mon Sep 17 00:00:00 2001
>From: Antti Koivisto <antti@apple.com>
>Date: Mon, 13 Jun 2022 15:49:57 +0300
>Subject: [PATCH] [CSS Container Queries] Invalidate animation keyframes using
> container units on when container size changes
> https://bugs.webkit.org/show_bug.cgi?id=241546
>
>Reviewed by Antoine Quint.
>
>Container size change also changes the interpretation of container units used in keyframes.
>
>* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-units-animation-expected.txt:
>* Source/WebCore/dom/Element.cpp:
>(WebCore::Element::invalidateForQueryContainerSizeChange):
>(WebCore::Element::needsUpdateQueryContainerDependentStyle const):
>(WebCore::Element::clearNeedsUpdateQueryContainerDependentStyle):
>(WebCore::Element::invalidateForQueryContainerChange): Deleted.
>
>Add a new bit that tells when a container has been resized.
>
>* Source/WebCore/dom/Element.h:
>* Source/WebCore/dom/Node.h:
>* Source/WebCore/rendering/style/KeyframeList.cpp:
>(WebCore::KeyframeList::usesContainerUnits const):
>
>Check for container unit use.
>
>* Source/WebCore/rendering/style/KeyframeList.h:
>* Source/WebCore/style/StyleScope.cpp:
>(WebCore::Style::Scope::updateQueryContainerState):
>* Source/WebCore/style/StyleTreeResolver.cpp:
>(WebCore::Style::TreeResolver::createAnimatedElementUpdate):
>
>Invalidate the keyframes if needed when computing the style.
>
>(WebCore::Style::TreeResolver::pushParent):
>
>Track if the subtree is withing a resized container.
>
>* Source/WebCore/style/StyleTreeResolver.h:
>* Source/WebCore/style/Styleable.cpp:
>(WebCore::Styleable::queryContainerDidChange const):
>* Source/WebCore/style/Styleable.h:
>---
> Source/WebCore/dom/Element.cpp                    | 13 ++++++++++++-
> Source/WebCore/dom/Element.h                      |  5 ++++-
> Source/WebCore/dom/Node.h                         |  5 +++--
> Source/WebCore/rendering/style/KeyframeList.cpp   | 10 ++++++++++
> Source/WebCore/rendering/style/KeyframeList.h     |  2 ++
> Source/WebCore/style/StyleScope.cpp               |  2 +-
> Source/WebCore/style/StyleTreeResolver.cpp        |  9 ++++++++-
> Source/WebCore/style/StyleTreeResolver.h          |  3 ++-
> Source/WebCore/style/Styleable.cpp                | 15 +++++++++++++++
> Source/WebCore/style/Styleable.h                  |  2 ++
> .../container-units-animation-expected.txt        | 12 ++++++------
> 11 files changed, 65 insertions(+), 13 deletions(-)
>
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index 509a89a656ba..3092494322bb 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -2265,10 +2265,21 @@ void Element::invalidateStyleForSubtreeInternal()
>     Node::invalidateStyle(Style::Validity::SubtreeInvalid);
> }
> 
>-void Element::invalidateForQueryContainerChange()
>+void Element::invalidateForQueryContainerSizeChange()
> {
>     // FIXME: Ideally we would just recompute things that are actually affected by containers queries within the subtree.
>     Node::invalidateStyle(Style::Validity::SubtreeInvalid);
>+    setNodeFlag(NodeFlag::NeedsUpdateQueryContainerDependentStyle);
>+}
>+
>+bool Element::needsUpdateQueryContainerDependentStyle() const
>+{
>+    return hasNodeFlag(NodeFlag::NeedsUpdateQueryContainerDependentStyle);
>+}
>+
>+void Element::clearNeedsUpdateQueryContainerDependentStyle()
>+{
>+    clearNodeFlag(NodeFlag::NeedsUpdateQueryContainerDependentStyle);
> }
> 
> void Element::invalidateEventListenerRegions()
>diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h
>index 1c11bbc586b6..d1952e77d78a 100644
>--- a/Source/WebCore/dom/Element.h
>+++ b/Source/WebCore/dom/Element.h
>@@ -639,7 +639,10 @@ public:
> 
>     void invalidateStyleInternal();
>     void invalidateStyleForSubtreeInternal();
>-    void invalidateForQueryContainerChange();
>+    void invalidateForQueryContainerSizeChange();
>+
>+    bool needsUpdateQueryContainerDependentStyle() const;
>+    void clearNeedsUpdateQueryContainerDependentStyle();
> 
>     void invalidateEventListenerRegions();
> 
>diff --git a/Source/WebCore/dom/Node.h b/Source/WebCore/dom/Node.h
>index 458c547b0843..568f80f6d302 100644
>--- a/Source/WebCore/dom/Node.h
>+++ b/Source/WebCore/dom/Node.h
>@@ -582,9 +582,10 @@ protected:
>         IsComputedStyleInvalidFlag = 1 << 25,
>         HasShadowRootContainingSlots = 1 << 26,
>         IsInTopLayer = 1 << 27,
>-        NeedsSVGRendererUpdate = 1 << 28
>+        NeedsSVGRendererUpdate = 1 << 28,
>+        NeedsUpdateQueryContainerDependentStyle = 1 << 29,
> 
>-        // Bits 29-31 are free.
>+        // Bits 30-31 are free.
>     };
> 
>     enum class TabIndexState : uint8_t {
>diff --git a/Source/WebCore/rendering/style/KeyframeList.cpp b/Source/WebCore/rendering/style/KeyframeList.cpp
>index 53568c58bbef..ed11fc0d1b09 100644
>--- a/Source/WebCore/rendering/style/KeyframeList.cpp
>+++ b/Source/WebCore/rendering/style/KeyframeList.cpp
>@@ -217,4 +217,14 @@ bool KeyframeList::containsAnimatableProperty() const
>     return false;
> }
> 
>+bool KeyframeList::usesContainerUnits() const
>+{
>+    for (auto& keyframe : m_keyframes) {
>+        if (keyframe.style()->usesContainerUnits())
>+            return true;
>+    }
>+    return false;
>+}
>+
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/rendering/style/KeyframeList.h b/Source/WebCore/rendering/style/KeyframeList.h
>index 1417cd7a9ad2..ad87796adc2e 100644
>--- a/Source/WebCore/rendering/style/KeyframeList.h
>+++ b/Source/WebCore/rendering/style/KeyframeList.h
>@@ -105,6 +105,8 @@ public:
>     auto begin() const { return m_keyframes.begin(); }
>     auto end() const { return m_keyframes.end(); }
> 
>+    bool usesContainerUnits() const;
>+
> private:
>     AtomString m_animationName;
>     Vector<KeyframeValue> m_keyframes; // Kept sorted by key.
>diff --git a/Source/WebCore/style/StyleScope.cpp b/Source/WebCore/style/StyleScope.cpp
>index ad49a12698db..bb1b41dee95a 100644
>--- a/Source/WebCore/style/StyleScope.cpp
>+++ b/Source/WebCore/style/StyleScope.cpp
>@@ -829,7 +829,7 @@ bool Scope::updateQueryContainerState(QueryContainerUpdateContext& context)
>     }
> 
>     for (auto* toInvalidate : containersToInvalidate)
>-        toInvalidate->invalidateForQueryContainerChange();
>+        toInvalidate->invalidateForQueryContainerSizeChange();
> 
>     return !containersToInvalidate.isEmpty();
> }
>diff --git a/Source/WebCore/style/StyleTreeResolver.cpp b/Source/WebCore/style/StyleTreeResolver.cpp
>index bb7bb6b39537..8c061c4875b9 100644
>--- a/Source/WebCore/style/StyleTreeResolver.cpp
>+++ b/Source/WebCore/style/StyleTreeResolver.cpp
>@@ -577,7 +577,9 @@ ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderSt
>     auto& document = element.document();
>     auto* oldStyle = element.renderOrDisplayContentsStyle(styleable.pseudoId);
> 
>-    OptionSet<AnimationImpact> animationImpact;
>+    // FIXME: Something like this is also needed for viewport units.
>+    if (oldStyle && parent().needsUpdateQueryContainerDependentStyle)
>+        styleable.queryContainerDidChange();
> 
>     // First, we need to make sure that any new CSS animation occuring on this element has a matching WebAnimation
>     // on the document timeline.
>@@ -592,6 +594,8 @@ ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderSt
>             styleable.updateCSSAnimations(oldStyle, *newStyle, resolutionContext);
>     }
> 
>+    OptionSet<AnimationImpact> animationImpact;
>+
>     // Now we can update all Web animations, which will include CSS Animations as well
>     // as animations created via the JS API.
>     if (styleable.hasKeyframeEffects()) {
>@@ -640,6 +644,9 @@ void TreeResolver::pushParent(Element& element, const RenderStyle& style, Change
>         parent.didPushScope = true;
>     }
> 
>+    parent.needsUpdateQueryContainerDependentStyle = m_parentStack.last().needsUpdateQueryContainerDependentStyle || element.needsUpdateQueryContainerDependentStyle();
>+    element.clearNeedsUpdateQueryContainerDependentStyle();
>+
>     m_parentStack.append(WTFMove(parent));
> }
> 
>diff --git a/Source/WebCore/style/StyleTreeResolver.h b/Source/WebCore/style/StyleTreeResolver.h
>index 5af9fbf9270d..acf00a48ecbd 100644
>--- a/Source/WebCore/style/StyleTreeResolver.h
>+++ b/Source/WebCore/style/StyleTreeResolver.h
>@@ -69,7 +69,7 @@ private:
>     enum class DescendantsToResolve : uint8_t { None, ChildrenWithExplicitInherit, Children, All };
>     std::pair<ElementUpdate, DescendantsToResolve> resolveElement(Element&, ResolutionType);
> 
>-    static ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, const Styleable&, Change, const ResolutionContext&);
>+    ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, const Styleable&, Change, const ResolutionContext&);
>     std::optional<ElementUpdate> resolvePseudoElement(Element&, PseudoId, const ElementUpdate&);
>     std::optional<ElementUpdate> resolveAncestorPseudoElement(Element&, PseudoId, const ElementUpdate&);
>     std::unique_ptr<RenderStyle> resolveAncestorFirstLinePseudoElement(Element&, const ElementUpdate&);
>@@ -95,6 +95,7 @@ private:
>         DescendantsToResolve descendantsToResolve { DescendantsToResolve::None };
>         bool didPushScope { false };
>         bool resolvedFirstLineAndLetterChild { false };
>+        bool needsUpdateQueryContainerDependentStyle { false };
> 
>         Parent(Document&);
>         Parent(Element&, const RenderStyle&, Change, DescendantsToResolve);
>diff --git a/Source/WebCore/style/Styleable.cpp b/Source/WebCore/style/Styleable.cpp
>index c44412ad94a5..d4916dcad11e 100644
>--- a/Source/WebCore/style/Styleable.cpp
>+++ b/Source/WebCore/style/Styleable.cpp
>@@ -625,4 +625,19 @@ void Styleable::updateCSSTransitions(const RenderStyle& currentStyle, const Rend
>         updateCSSTransitionsForStyleableAndProperty(*this, property, currentStyle, newStyle, generationTime);
> }
> 
>+void Styleable::queryContainerDidChange() const
>+{
>+    auto* animations = this->animations();
>+    if (!animations)
>+        return;
>+    for (auto animation : *animations) {
>+        auto* cssAnimation = dynamicDowncast<CSSAnimation>(animation.get());
>+        if (!cssAnimation)
>+            continue;
>+        auto* keyframeEffect = dynamicDowncast<KeyframeEffect>(cssAnimation->effect());
>+        if (keyframeEffect && keyframeEffect->blendingKeyframes().usesContainerUnits())
>+            cssAnimation->keyframesRuleDidChange();
>+    }
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/style/Styleable.h b/Source/WebCore/style/Styleable.h
>index 254058fd9b76..a95408a250a8 100644
>--- a/Source/WebCore/style/Styleable.h
>+++ b/Source/WebCore/style/Styleable.h
>@@ -161,6 +161,8 @@ struct Styleable {
>         element.keyframesRuleDidChange(pseudoId);
>     }
> 
>+    void queryContainerDidChange() const;
>+
>     bool animationListContainsNewlyValidAnimation(const AnimationList&) const;
> 
>     void elementWasRemoved() const;
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-units-animation-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-units-animation-expected.txt
>index 29d197aa122f..1323c364375c 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-units-animation-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-units-animation-expected.txt
>@@ -1,14 +1,14 @@
> 
> PASS Animation using cqw unit
>-FAIL Animation using cqw unit responds to changing container size assert_equals: expected "90px" but got "60px"
>+PASS Animation using cqw unit responds to changing container size
> PASS Animation using cqh unit
>-FAIL Animation using cqh unit responds to changing container size assert_equals: expected "90px" but got "60px"
>+PASS Animation using cqh unit responds to changing container size
> PASS Animation using cqi unit
>-FAIL Animation using cqi unit responds to changing container size assert_equals: expected "90px" but got "60px"
>+PASS Animation using cqi unit responds to changing container size
> PASS Animation using cqb unit
>-FAIL Animation using cqb unit responds to changing container size assert_equals: expected "90px" but got "60px"
>+PASS Animation using cqb unit responds to changing container size
> PASS Animation using cqmin unit
>-FAIL Animation using cqmin unit responds to changing container size assert_equals: expected "90px" but got "60px"
>+PASS Animation using cqmin unit responds to changing container size
> PASS Animation using cqmax unit
>-FAIL Animation using cqmax unit responds to changing container size assert_equals: expected "90px" but got "60px"
>+PASS Animation using cqmax unit responds to changing container size
>

Actions: View | Formatted Diff | Diff

Attachments on bug 241546: 460197 | 460253 | 460255 | 460256