Bug 167901 - Animated GIFs fail to play in multi-column layout
Summary: Animated GIFs fail to play in multi-column layout
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari 10
Hardware: All All
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-02-06 13:04 PST by Anton Eprev
Modified: 2017-03-07 10:55 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.52 KB, patch)
2017-03-07 08:45 PST, Dave Hyatt
zalan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Eprev 2017-02-06 13:04:29 PST
When multiple animated GIFs are placed in multi-column container, Safari (10 and TP) will manage to play only a few of them.
More GIFs you put in, higher chance they fail to play. This is happening only when the value of `columns` property is greater than 1.

How to reproduce:
1) Open https://jsfiddle.net/sdxaxuv5/
2) Usually last two images (in 3rd column) fail to start playing.
3) Change `columns` property's value from 3 to 1, and the issue will disappear.

Checked on both iMac and iOS. Other browsers play all the images.
Comment 1 Radar WebKit Bug Importer 2017-02-06 13:13:47 PST
<rdar://problem/30382262>
Comment 2 Said Abou-Hallawa 2017-02-06 16:23:05 PST
This is not an image drawing or animation issue. It is a rendering issue. I think it is related to column layout.

In the test case https://jsfiddle.net/sdxaxuv5/, RenderElement::newImageAnimationFrameAvailable(), which is called via BitmapImage::advanceAnimation(, succeeds to call RenderObject::imageChanged() for the first four <img> elements in the column <div> element. But it fails to call it for the last two <img> elements. This happens because shouldRepaintForImageAnimation() returns true for the first four images but it returns false for the last two images.

Because the image rect is not marked for painting, the last two images do not animate. Fixing the repaint issue should force the image to draw the next image frame.
Comment 3 Dave Hyatt 2017-03-07 08:45:45 PST
Created attachment 303659 [details]
Patch
Comment 4 zalan 2017-03-07 08:52:25 PST
Comment on attachment 303659 [details]
Patch

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

Has this ever worked?

> Source/WebCore/rendering/RenderBox.cpp:2301
> +    if (is<RenderMultiColumnFlowThread>(this)) {

is<RenderMultiColumnFlowThread>(*this)

> Source/WebCore/rendering/RenderBox.cpp:2309
> +        if (RenderRegion* region = downcast<RenderMultiColumnFlowThread>(this)->physicalTranslationFromFlowToRegion((physicalPoint))) {

if (auto* region = downcast<RenderMultiColumnFlowThread>(*this).physicalTranslationFromFlowToRegion((physicalPoint)))
Comment 5 Dave Hyatt 2017-03-07 09:59:51 PST
Fixed in r213523.
Comment 6 Simon Fraser (smfr) 2017-03-07 10:55:31 PST
Comment on attachment 303659 [details]
Patch

Seems like this could be tested via tracking repaint rects.