Summary: | GIF clamping time differs from Firefox | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peter Kasting <pkasting> | ||||||||
Component: | Images | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, commit-queue, darin, eric, evan, gavin.sharp, hyatt, mkterra, mrowe | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Peter Kasting
2009-06-16 14:02:28 PDT
Normally we follow Firefox instead of IE for otherwise arbitrary Web compatibility issues, unless we think following IE will give better compatibility. Part of the reason we do that is due to the prevalence of dual code path sites. In this case though, it's unlikely a site will have different versions of an animated GIF for Firefox and IE. Some thoughts: 1) It seems that, for reasons stated above, following IE is probably the right default choice for this issue, in the absence of other reasons. 2) Making the clamp limit lower does seem more right in the abstract - the main purpose of having a limit is to give reasonable behavior for a frame time of 0 or other unreasonably low values. But an animated GIF should be allowed to legitimately do 60fps. 3) The exact details of the clamping limit don't seem hugely important. For high quality animations it's likely better to use a real video codec, and current animated GIF uses seem to be in situations where quality of presentation is not a prime concern. Given this, I'm personally fairly indifferent between the options. Thanks for the comments Maciej. Hyatt has said he's also fairly indifferent. (In reply to comment #1) > 1) It seems that, for reasons stated above, following IE is probably the right > default choice for this issue, in the absence of other reasons. Could you clarify which reasons you meant by this? > 3) The exact details of the clamping limit don't seem hugely important. For > high quality animations it's likely better to use a real video codec, and > current animated GIF uses seem to be in situations where quality of > presentation is not a prime concern. I agree with this. One user on a forum noted that videos (often the source of animated GIFs) are generally either 24 FPS or 30 FPS, both of which are over 20 FPS and thus would currently be dropped to 10 FPS by WebKit. Of course, with the prevalence of sites like YouTube, the need to use animated GIFs to display video is presumably decreasing. Here's an entire site that relies on Firefox' clamp times: http://www.sorting-algorithms.com/ (In reply to comment #1) > 3) The exact details of the clamping limit don't seem hugely important. For > high quality animations it's likely better to use a real video codec, and > current animated GIF uses seem to be in situations where quality of > presentation is not a prime concern. The sorting-algorithms.com demos involve pixel-perfect graphics. I think even a high-quality video codec will introduce artifacts. So at this point for that application, lossless GIF animations are the best option for the author. (I have no strong opinion on the actual bug, except to note that someone came into #chromium and wondered why were were "slower" than Firefox due to this issue. But I can see from the prior discussion that both options are reasonable.) Created attachment 47786 [details]
50 frames with 0.02s duration
Created attachment 47787 [details]
50 frames with 0.01s duration
For the record, Opera seems to match Firefox's behavior. I've attached two spinner gifs for testing; in Firefox and Opera the 0.02s version doesn't trigger clamping and thus plays faster than the 0.01s version, whereas in IE and WebKit both versions play the same. *** Bug 36082 has been marked as a duplicate of this bug. *** Bug 36082 has some research into Web compatibility effects of changing to Firefox behavior. This data makes simply matching Firefox unfeasible, even though we do have compatibility problems either way, and finding a solution would be most welcome. (In reply to comment #10) > Bug 36082 has some research into Web compatibility effects of changing to > Firefox behavior. This data makes simply matching Firefox unfeasible This is your opinion, not objective fact. In my opinion it is obvious from the research I have done (some of which is on these bugs) here that changing our behavior will result in a net win. The fact that that win is not perfect does not make it unreasonable. There is a good analogy here with the BMP decoder I wrote. Transparency in BMPs is not specced anywhere and different programs produce different output. There is no perfect way to display all BMPs as their creators intend; you _must_ use heuristics to decide what to do, and no matter what you do some BMPs will display incorrectly. In that situation it would be silly to say that one cannot prefer behavior X over behavior Y because behavior X causes some BMPs to display incorrectly which behavior Y does not. There are problems either way, and thus the right behavior is a judgment call. Here too, it is impossible to craft a behavior that makes all images display perfectly. Therefore, what we should do is a judgment call. I agree with bdash that it would be good judgment to change. I think you misunderstood. Introducing a significant number of regressions is unacceptable, and that's more than a subjective opinion. By saying "a significant number" instead of "any", I'm already suggesting a lower standard than we normally require, acknowledging that there is no perfect answer. Now, this is a subjective opinion, and someone may disagree that a lower standard is acceptable here. (In reply to comment #12) > I think you misunderstood. Introducing a significant number of regressions is > unacceptable, and that's more than a subjective opinion. I didn't misunderstand a thing. Regressions are bugs -- just like existing bugs are bugs. Introducing a regression is bad, but not fixing an existing bug is also bad. Saying "no new regressions" without context or acknowledgement of what gets fixed isn't good policy, it's blind dogmatism. With this change, I claim we fix more bugs than we introduce, our new behavior makes much more sense, there is more agreement amongst browsers (everyone except IE will agree), and it's possible for people to fix GIFs to comply with the new behavior, which isn't possible with the old behavior. HTML5's new parsing rules introduce some regressions too. When browsers disagree, there are usually regressions of some sort when you try to converge on something. If this were so cut-and-dried as you make it, I'd have expected any commentary to that effect on comment 1 from Maciej or the linked bug from Mark, both of whom are pretty meticulous about doing things The Right Way. Created attachment 50734 [details]
Patch
Comment on attachment 50734 [details]
Patch
We are considering ways to solve the compatibility problem without introducing an equal number of regressions.
One suggestion was that rather than matching Firefox we clamp anything trying to play faster than 30fps. This would allow the reasonably common practice of video clip style animated GIFs to play back smoothly. (In reply to comment #16) > One suggestion was that rather than matching Firefox we clamp anything trying > to play faster than 30fps. This would allow the reasonably common practice of > video clip style animated GIFs to play back smoothly. ...And mean that the three major browser engines have three different GIF clamping algorithms. Moving further away from a common behavior seems like backwards progress to me. (In reply to comment #17) > ...And mean that the three major browser engines have three different GIF > clamping algorithms. > > Moving further away from a common behavior seems like backwards progress to me. I’m all out of ideas then. Both the Firefox and IE behavior have obvious downsides. The Firefox behavior seems preferable to me because at least it’s possible for content authors to achieve the effect that they’re after, while the IE behavior makes certain types of content impossible. I spoke with Maciej and he said that matching the Firefox behavior seems preferable unless the Internet Explorer behavior is obviously better in terms of web compatibility. (In reply to comment #19) > I spoke with Maciej and he said that matching the Firefox behavior seems > preferable unless the Internet Explorer behavior is obviously better in terms > of web compatibility. That's also my sense, and both my anecdotal experience as well as the stuff I looked up online originally suggested that, if anything, the Firefox behavior is more broadly compatible. This makes some logical sense, given that in order to work with IE but not Firefox, you have to set frame timings that are longer than 10 ms and expect them to be raised to 100 ms--that is, you have to tell your animation to run at e.g. 30 FPS and then expect it to run at 10. In the abstract this seems less likely than simply saying "run as fast as possible", which winds up either clamped or unclamped in both IE and Firefox, depending on whether the animation program knows about IE and sets the timings to 50 ms, or not and sets them to 0 or 1 or similar. It's this bounded-on-both-sides "window" that I didn't find large numbers of images falling into when I looked. So we are leaning towards following the exact frame duration unless the frame duration is <= 10 ms, in which case we will use a frame duration of 100 ms. Comment on attachment 50734 [details]
Patch
I'd like this patch to be re-considered for review based on the following:
* I continue to receive a large volume of complaints about this bug on the Chromium side. It is easily the #1 most-reported issue with images in Chromium.
* Firefox continues to ship with "<= 10 ms --> 100 ms". I think it highly unlikely there can be major web compat problems caused by this behavior with such a widely-used browser shipping it for so long. (FWIW, Opera also ships with this behavior [I tested Opera 11].)
* From comments 19 - 21, it seems like matching Firefox is what bdash, maciej, darin and I all want to do. I also talked with several Chromium developers and they all agreed that matching Firefox/Opera made the most sense here.
My previous arguments in favor of this change, of course, also still stand.
Comment on attachment 50734 [details]
Patch
As far as I can tell from the discusion. We're all in agreement... this is a good change. So I guess I'll set r+.
I don't think anything has changed, so I didn't repeat that we're not in agreement. (In reply to comment #24) > I don't think anything has changed I disagree. Darin, Mark, and Maciej have weighed in in favor, and the other points I make in comment 22 also represent the passage of time. I don't want to ignore or disregard your comments and just land this patch anyway. But I also don't think it's fair to hold the code hostage to the behavior you think is better when a group of hopefully-also-respected developers disagrees for what seem like valid reasons. I think we should publicly note that you don't like this change, proceed with what the group as a whole seems to have converged on, and take a look at what sorts of feedback we receive about the behavior change. I think we should proceed with this change. Overall it's a net improvement. Comment on attachment 50734 [details]
Patch
OK.
I don't know why the process of making a decision came down to counting opinions. Pretty much every argument for this change that was made in this discussion has been found unsound. So mark it as r- then. :) I'm simply going through the review queue attempting to move patches along. We topped over 250 this morning. :) Figured it was time for some housecleaning. I'm not attempting to stir controversy with this (or other) r+'s. If this decision is not obvious, than someone more involved in the decision should bring this bug to a close. Comment on attachment 50734 [details] Patch Clearing flags on attachment: 50734 Committed r73295: <http://trac.webkit.org/changeset/73295> All reviewed patches have been landed. Closing bug. In fact, this hasn't been ignored - someone else at Apple was actively working on weighing the arguments, and had some new ones. This hasn't been reflected in the bug, unfortunately. |