Bug 226216 – REGRESSION (r272201): iPad render flashing on load

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Roland Soos]

Created attachment 429641 [details]
Screen recording on iPad

Steps to reproduce:
1. Open https://smartslider3.com/bugs/webkit/flashing2/

What's went wrong:
The see through black area and it's content is flashing for a few frame.

I was able to reproduce it only on iPad. IOS version: 14.5.1

[Simon Fraser (smfr)]

Roland, do you know if this occurred in earlier iOS versions?

[Roland Soos]

I don't know, sorry.

[Roland Soos]

Also on this page: https://smartslider3.com/full-page-hotel/
A very similar flashing occurs on load only on iPad. Do you want me to create a static page from it?

[Simon Fraser (smfr)]

The https://smartslider3.com/full-page-hotel/ is a flash caused by async image decoding. In cases like this where you animated a large image on-screen, you need to be using the image.decode() API (https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/decode) to know when the image is ready to display.

[Roland Soos]

These texts are flashing for a moment, not the image in the background: https://i.imgur.com/d9z1ZsC.png

[Radar WebKit Bug Importer]

<rdar://problem/78466306>

[Roland Soos]

Created attachment 429750 [details]
Flashing text on load

[Simon Fraser (smfr)]

I think we're changing from a tiled layer to a non-tiled layer, and fail to move the animations to the new layer.

[Simon Fraser (smfr)]

Antoine, I think GraphicsLayerCA::moveOrCopyLayerAnimation() is broken with animation groups.

PlatformCALayers have animation groups on them:

    (layer 29
      (animation group-c0316d87-a66f-4fd0-9c2a-b1d002514449_5_0_0 type=group keyPath=
        (beginTime 1.00)

but GraphicsLayerCA::moveOrCopyLayerAnimation() only deals with animationForKey(animationIdentifier)

[Antoine Quint]

Cool, thanks for the analysis, hopefully that's an easy fix.

[Antoine Quint]

On iPhone this reproduces as well.

[Antoine Quint]

While the issue appears differently between iPhone and iPad, the regression point is the same.

[Antoine Quint]

I think I have done the required changes to GraphcisLayerCA::moveOrCopyAnimations() but the bug reproduces still. I need to check whether Core Animation allows setting a group animation on different layers while holding the children animations.

[Antoine Quint]

Found the issue: Core Animation seems to compute the begin time when calling animationForKey(), so we need to re-set it to the original value when setting the animation anew on the new layer.

The tricky bit now is to write a test.

[Antoine Quint]

Created attachment 430610 [details]
Patch for EWS

[Antoine Quint]

Created attachment 430759 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 430759 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430759&action=review

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:714
> +        if ((animatedPropertyIsTransformOrRelated(animationGroup.m_property)
> +            || animationGroup.m_property == AnimatedPropertyOpacity
> +            || animationGroup.m_property == AnimatedPropertyBackgroundColor
> +            || animationGroup.m_property == AnimatedPropertyFilter))

I wonder why we check the types here. Can't we just move all animations?

[Antoine Quint]

(In reply to Simon Fraser (smfr) from comment #17)
> Comment on attachment 430759 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430759&action=review
> 
> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:714
> > +        if ((animatedPropertyIsTransformOrRelated(animationGroup.m_property)
> > +            || animationGroup.m_property == AnimatedPropertyOpacity
> > +            || animationGroup.m_property == AnimatedPropertyBackgroundColor
> > +            || animationGroup.m_property == AnimatedPropertyFilter))
> 
> I wonder why we check the types here. Can't we just move all animations?

I think it was to isolate the AnimatedPropertyBackgroundColor and AnimatedPropertyWebkitBackdropFilter animations which apply to something other than the primary layer.

[Antoine Quint]

Committed r278566 (238564@main): <https://commits.webkit.org/238564@main>