Bug 93458 – [Texmap] Render gif animation well.

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Dongseong Hwang]

GraphicsLayerTextureMapper::setContentsToImage() checks the pointer to the
image, not nativeImagePtr, so Texmap currently draws only the first frame of gif
animations. This patch makes Texmap draw gif animations.

[Dongseong Hwang]

Created attachment 157169 [details]
Patch

[Dongseong Hwang]

Created attachment 157170 [details]
Not for review : Layout Test : Animated GIF on a compositing layer 

I created this test in order to test the animated gif on Texmap.
It is easy for human to check animated gif rendering, but it is hard to make a complete layout test. I think there is no functionality to capture the specific frame of gif animations in the webkit test runner.
So, I submit this test not for review.

[Dongseong Hwang]

(In reply to comment #2)

The attached test is same to this site : http://www.dorothybrowser.com/test/webkitTest/css3/anigifAC.html

[Noam Rosenthal]

Comment on attachment 157169 [details]
Patch

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

Good catch, see comments.

> Source/WebCore/ChangeLog:12
> +        No new tests, hard to write a test case.

Even harder to catch regressions later on... Please find the right test case for this, e.g. changing the frame and then testing for pixels.

> Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:329
> +    NativeImagePtr newNativeImagePtr = image->nativeImageForCurrentFrame();

You need a null check.

[Dongseong Hwang]

 (In reply to comment #4)
> Even harder to catch regressions later on... Please find the right test case for this, e.g. changing the frame and then testing for pixels.

Thanks for review.
How to test it came to my mind after reading Bug 93171.
I think if php sends a partial gif animation file to webkit, webkit will render the end frame of the received file.
I'll update it.

[Dongseong Hwang]

Created attachment 158939 [details]
Patch

[Dongseong Hwang]

(In reply to comment #4)
> Even harder to catch regressions later on... Please find the right test case for this, e.g. changing the frame and then testing for pixels.

Unfortunately, I could not find prominent solution. I think DRT does not draw animated gif. Even if I tried to capture a image using setInterval, DRT always captured the first frame. We need to change DRT earlier than making a test for this patch.

> 
> You need a null check.

Done. I wrote GraphicsLayerTextureMapper::setContentsToImage similar to GraphicsLayerCA::setContentsToImage.

[Noam Rosenthal]

Comment on attachment 158939 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        No new tests, hard to write a test case.

Please change this to "could not write a test due to DRT limitation, see (enter bug ID).

[Dongseong Hwang]

Created attachment 158948 [details]
Patch

[Dongseong Hwang]

(In reply to comment #8)
> Please change this to "could not write a test due to DRT limitation, see (enter bug ID).

I changed the changelog as you suggested. :)

[WebKit Review Bot]

Comment on attachment 158948 [details]
Patch

Clearing flags on attachment: 158948

Committed r125834: <http://trac.webkit.org/changeset/125834>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.