Bug 22995 - REGRESSION: radar loops don't animate
Summary: REGRESSION: radar loops don't animate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.5
: P1 Normal
Assignee: Peter Kasting
URL: http://radar.weather.gov/ridge/radar_...
Keywords: Regression
: 21753 22366 22646 23000 23224 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-12-25 17:12 PST by pilotgi
Modified: 2009-01-11 10:02 PST (History)
5 users (show)

See Also:


Attachments
patch v1 (7.28 KB, patch)
2009-01-08 17:29 PST, Peter Kasting
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description pilotgi 2008-12-25 17:12:51 PST
Bug #22108 was marked resolved related to the problem of radar loops not animating but it has never worked for me. The claim was it was fixed with version r39309 but I don't get any animations from this version or any subsequent versions.

Is there some additional work around that is needed?
Comment 1 Alexey Proskuryakov 2008-12-26 01:29:59 PST
Confirmed with r39437. On first load, there is no animation - and on a reload, animation seems to work, but a lot of errors are printed to console:

Fri Dec 26 12:27:17 ap-mbp.local Safari[31961] <Error>: CGImageSourceUpdateData image source was already finalized

No such problem on shipping Safari/WebKit, although the behavior is not very good either, with a lot of flashing.

Comment 2 Robert Blaut 2008-12-26 10:25:56 PST
*** Bug 21753 has been marked as a duplicate of this bug. ***
Comment 3 Robert Blaut 2008-12-26 10:26:45 PST
*** Bug 23000 has been marked as a duplicate of this bug. ***
Comment 4 Robert Blaut 2008-12-26 10:27:51 PST
*** Bug 22646 has been marked as a duplicate of this bug. ***
Comment 5 Robert Blaut 2008-12-26 10:28:08 PST
*** Bug 22366 has been marked as a duplicate of this bug. ***
Comment 6 Peter Kasting 2008-12-26 19:52:10 PST
The errors to console would be fixed by the patch on bug 22929.

I don't know why this isn't animating.  I am almost at the point of wanting Safari to just use the open source decoders instead of the CG ones since they're both more performant and less buggy :( (of course, they lack color profile handling, etc...)
Comment 7 pilotgi 2009-01-01 12:49:54 PST
Just downloaded and installed r39524. Went to weather.gov to try animating radar loops but no luck. On a whim, I control-clicked the image and choose the selection 'Inspect Element'.

The window that pops up shows html code and behind this I noticed that the radar loop had started animating. I closed the Inspect Element window and was able to turn the animation on and off with no problems. I was also able to reload the page and start the animation again. CPU usage was 1 to 5 %.

If I click on a different part of the map to try a different radar loop, some will work and some don't but they always start animating after I ctrl-click and select 'Inspect Element'.

Weird, huh?
Comment 8 Jim Oase 2009-01-02 17:44:31 PST
(In reply to comment #7)
> Just downloaded and installed r39524. Went to weather.gov to try animating
> radar loops but no luck. On a whim, I control-clicked the image and choose the
> selection 'Inspect Element'.
> 
> The window that pops up shows html code and behind this I noticed that the
> radar loop had started animating. I closed the Inspect Element window and was
> able to turn the animation on and off with no problems. I was also able to
> reload the page and start the animation again. CPU usage was 1 to 5 %.
> 
> If I click on a different part of the map to try a different radar loop, some
> will work and some don't but they always start animating after I ctrl-click and
> select 'Inspect Element'.
> 
> Weird, huh?
> 
I duplicated this operation the first couple times I tried it (using the same build as above), then the could not get animation again.  I have just loaded  WebKit-SVN-r39553 and still can not get animation to function even using the above technique.  

Maybe this site doesn't like me.
Comment 9 Jim Oase 2009-01-03 19:28:49 PST
(In reply to comment #7)
> Just downloaded and installed r39524. Went to weather.gov to try animating
> radar loops but no luck. On a whim, I control-clicked the image and choose the
> selection 'Inspect Element'.
> 
> The window that pops up shows html code and behind this I noticed that the
> radar loop had started animating. I closed the Inspect Element window and was
> able to turn the animation on and off with no problems. I was also able to
> reload the page and start the animation again. CPU usage was 1 to 5 %.
> 
> If I click on a different part of the map to try a different radar loop, some
> will work and some don't but they always start animating after I ctrl-click and
> select 'Inspect Element'.
> 
> Weird, huh?
> 

I reported earlier that I tried the above and the first couple of times it animation started just as reported above.  Then i could not get it to work again, even after load the latest build.

This afternoon I tried it again.  This time I clicked on the xxxx.gif and waited.  Animation finally began.

I am going with "Weird, huh?"
Comment 10 Peter Kasting 2009-01-05 11:34:58 PST
Has the fix for bug 23082 fixed this as well?
Comment 11 Robert Blaut 2009-01-05 12:26:30 PST
(In reply to comment #10)
> Has the fix for bug 23082 fixed this as well?
> 

No, it doesn't work for me in WebKit r39572 on Mac OS X.
Comment 12 Jim Oase 2009-01-05 16:52:22 PST
The bug still exists as of  WebKit-SVN-r39572 

Animation can be made operate most of the time using the instructions in comment #7, right click on the map, click inspect element, wait, click on the highlighted line, wait some more and most times the animation will begin to work.

I still haven't found the magic sequence that makes animation work immediately.

Jim

Comment 13 Jim Oase 2009-01-07 12:24:22 PST
Radar loop animation still does not work with build WebKit-SVN-r39671

Animation can be made to work using the Inspect Element scheme pilotgi@gmail.com discovered... listed in the comment section.

When animation is working, using the inspect elements scheme, scrolling becomes erratic when using the scroll ball on my super mouse.

Scrolling has been a problem and been fixed with this site before.

Jim
Comment 14 Simon Fraser (smfr) 2009-01-07 20:11:07 PST
I think I see the issue. You get into a state where you've got frame 0, and are waiting until frame 1 is decoded. frameIsCompleteAtIndex(1) has been called before the frame is ready, so frame 1 has m_frame == 0, but m_haveMetadata == true.

Now you keep calling frameIsCompleteAtIndex(1) over and over via startAnimation(). frameIsCompleteAtIndex() never calls cacheFrame again, because m_haveMetadata is true, so it does nothing (and m_isComplete stays at false). So you never make progress.

Someone needs to call cacheFrame() if m_frame is still 0. Something like this:

diff --git a/WebCore/WebCore.xcodeproj/project.pbxproj b/WebCore/WebCore.xcodeproj/project.pbxproj
diff --git a/WebCore/platform/graphics/BitmapImage.cpp b/WebCore/platform/graphics/BitmapImage.cpp
index 779787e..2eed9c6 100644
--- a/WebCore/platform/graphics/BitmapImage.cpp
+++ b/WebCore/platform/graphics/BitmapImage.cpp
@@ -287,6 +287,8 @@ void BitmapImage::startAnimation(bool catchUpIfNecessary)
 
     // Don't advance the animation to an incomplete frame.
     size_t nextFrame = (m_currentFrame + 1) % frameCount();
+    // Update the state of the frame by asking for it
+    (void)frameAtIndex(nextFrame);
     if (!frameIsCompleteAtIndex(nextFrame))
         return;
 
Comment 15 Simon Fraser (smfr) 2009-01-07 20:17:41 PST
Also, how can you trust the frame duration, alpha and size in BitmapImage::cacheFrame() if you don't have a frame yet? Seems like this method is over-zealous in thinking that it has valid metadata.
Comment 16 Simon Fraser (smfr) 2009-01-07 21:04:03 PST
Here's a good test URL for animated GIFs:
http://www.neogaf.com/forum/showthread.php?t=294041
Comment 17 Peter Kasting 2009-01-08 17:29:30 PST
Created attachment 26552 [details]
patch v1

This isn't tested, but if smfr is correct about the cause of the flakiness (and it certainly looks like a problem to me too), this should fix it, and in a more robust way than the proposed workaround of forcing the frame to re-cache in startAnimation().

This clears the frame's metadata if we get more data for it.  This should match fairly well with when the metadata _can_ actually change (it's just slightly aggressive in clearing the metadata, but not in a way that will have any real effect).
Comment 18 Dave Hyatt 2009-01-09 10:41:20 PST
Comment on attachment 26552 [details]
patch v1

r=me
Comment 19 Peter Kasting 2009-01-09 10:47:30 PST
Fixed, I hope, in r39751.
Comment 20 Robert Blaut 2009-01-10 13:50:17 PST
*** Bug 23224 has been marked as a duplicate of this bug. ***
Comment 21 pilotgi 2009-01-11 05:20:39 PST
I just installed r39790. Radar loops work for me!
Comment 22 Jim Oase 2009-01-11 08:29:15 PST
The radar animation loop works.....But....... now initial scrolling with a super mouse ball is delayed and jerky.     ARGH!!!

Takes a bite for animation to start.
After animation starts slowly scroll continuously east.
The picture will not move at first... then jerk to the correct location and in that axis work smoothly from then on.
Change axis to north/south and repeat the scroll process.
Again the picture will not move at first then jerk to the correct location
Comment 23 Simon Fraser (smfr) 2009-01-11 10:02:58 PST
Jim: please file a new bug on the performance problem, and, if you can, try to identify the nightly build where it first occurred.