Bug 310804 – [threaded-animations] animating `offset-path` with a percentage-based `offset-distance` value fails

• Component: Animations
• Status: RESOLVED FIXED

[Antoine Quint]

Created attachment 478803 [details]
Test

A keyframe such as this one:

```
@keyframes move {
    from { offset-path: rect(0px 100px 100px 0px) }
	to { offset-distance: 100% }
}
```

… fails a debug assertion for `PrimitiveNumeric`:

```
    IPCData ipcData() const
    {
        return WTF::switchOn(m_value,
            [](const Dimension& dimension) -> IPCData { return dimension; },
            [](const Percentage& percentage) -> IPCData { return percentage; },
            [](const Calc&) -> IPCData { ASSERT_NOT_REACHED(); return Dimension { 0 }; } // <<<< HERE
        );
    }
```

I find it surprising since we're not using `calc()` anywhere here.

[Antoine Quint]

Once we fix this, we'll crash in the blending code which needs to fix this way:

```
diff --git a/Source/WebCore/platform/animation/AcceleratedEffect.cpp b/Source/WebCore/platform/animation/AcceleratedEffect.cpp
index d72a1b1433b8..741a4cb602a4 100644
--- a/Source/WebCore/platform/animation/AcceleratedEffect.cpp
+++ b/Source/WebCore/platform/animation/AcceleratedEffect.cpp
@@ -366,8 +366,12 @@ static void blend(AcceleratedEffectProperty property, AcceleratedEffectValues& o
         output.offsetDistance = blend(from.offsetDistance, to.offsetDistance, blendingContext);
         break;
     case AcceleratedEffectProperty::OffsetPath:
-        if (auto& fromOffsetPath = from.offsetPath)
-            output.offsetPath = fromOffsetPath->blend(to.offsetPath.get(), blendingContext);
+        if (!from.offsetPath || !to.offsetPath) {
+            blendingContext.isDiscrete = true;
+            blendingContext.normalizeProgress();
+            output.offsetPath = blendingContext.progress ? to.offsetPath : from.offsetPath;
+        } else
+            output.offsetPath = from.offsetPath->blend(to.offsetPath.get(), blendingContext);
         break;
     case AcceleratedEffectProperty::OffsetPosition:
         if (!canBlend(from.offsetPosition, to.offsetPosition)) {
```

[Sam Weinig]

The reason there is a calc() here is that the `rect()` in `offset-path: rect(0px 100px 100px 0px)` gets converted to its "inset" form during style building, which introduces the calcs. 

rect(0px 100px 100px 0px) -> inset(0px calc(100% - 100px) calc(100% - 100px) 0px)

[Antoine Quint]

Will likely be fixed by bug 310943.

[Sam Weinig]

When https://bugs.webkit.org/show_bug.cgi?id=310943 lands, this no longer asserts, but the animation doesn't match the behavior when threaded animations are disabled.

To illustrate why it still doesn't match, I am going to make the values a bit more explicit in the example:

```
@keyframes move {
    from { 
        offset-path: rect(0px 100px 100px 0px);
        offset-distance: 0%;
    }
    to { 
        offset-path: none;
        offset-distance: 100%;
     }
}
```

When converted to AcceleratedEffectValues these become:

from -> AcceleratedEffectValues {
    offset-path: {some path}
    offset-distance: 0;
}

to -> AcceleratedEffectValues {
    offset-path: none
    offset-distance: 0;
}
```

Importantly, `offset-distance` stays `0`. This happens for two reasons.
1. Because `offset-distance` is only calculated at all if there is an `offset-path` (we can fix this one by just unconditionally evaluated the `offset-distance`)
2. Because percentage/calc `offset-distance` values are evaluated with respect to the length `offset-path`.

Due to #2, the lack of `offset-path` makes the `offset-distance: 100%` incalculable.

--

So what should we do?

We need to ensure that we don't use the accelerated animation path for keyframe pairs where:

- The length of `offset-path` is changing (this probably boils down to if "`offset-path` is changing" at all).
and
- The value of `offset-distance` is percentage or calc for either keyframe.

We should also change it so calculate values for all the `offset-*` properties regardless of whether `tryPath()` succeeds (except, of course, `offset-distance` when it is percentage or calc), so that they can properly interpolate even when one keyframe does not have a valid `offset-path`.

[Radar WebKit Bug Importer]

<rdar://problem/173926809>

[Antoine Quint]

Retitling given that the original issue was fixed by 310943.

[Antoine Quint]

Pull request: https://github.com/WebKit/WebKit/pull/62638

[EWS]

Committed 311224@main (89607e24306f): <https://commits.webkit.org/311224@main>

Reviewed commits have been landed. Closing PR #62638 and removing active labels.