Bug 93458

Summary: [Texmap] Render gif animation well.
Product: WebKit Reporter: Dongseong Hwang <dongseong.hwang>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: noam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 102043    
Attachments:
Description Flags
Patch
none
Not for review : Layout Test : Animated GIF on a compositing layer
none
Patch
none
Patch none

Dongseong Hwang
Reported 2012-08-08 03:59:41 PDT
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.
Attachments
Patch (3.68 KB, patch)
2012-08-08 04:02 PDT, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Not for review : Layout Test : Animated GIF on a compositing layer (1014 bytes, patch)
2012-08-08 04:04 PDT, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (3.78 KB, patch)
2012-08-16 16:34 PDT, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (3.81 KB, patch)
2012-08-16 17:00 PDT, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Dongseong Hwang
Comment 1 2012-08-08 04:02:31 PDT
Created attachment 157169 [details] Patch
Dongseong Hwang
Comment 2 2012-08-08 04:04:40 PDT
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
Comment 3 2012-08-08 04:08:31 PDT
(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 4 2012-08-08 06:35:08 PDT
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
Comment 5 2012-08-10 00:23:22 PDT
(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
Comment 6 2012-08-16 16:34:57 PDT
Created attachment 158939 [details] Patch
Dongseong Hwang
Comment 7 2012-08-16 16:44:49 PDT
(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 8 2012-08-16 16:46:29 PDT
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
Comment 9 2012-08-16 17:00:06 PDT
Created attachment 158948 [details] Patch
Dongseong Hwang
Comment 10 2012-08-16 17:00:39 PDT
(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 11 2012-08-16 17:49:21 PDT
Comment on attachment 158948 [details] Patch Clearing flags on attachment: 158948 Committed r125834: <http://trac.webkit.org/changeset/125834>
WebKit Review Bot
Comment 12 2012-08-16 17:49:25 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.