Changeset 60375 in webkit


Ignore:
Timestamp:
May 28, 2010 2:15:19 PM (14 years ago)
Author:
pkasting@chromium.org
Message:

​https://bugs.webkit.org/show_bug.cgi?id=39857
Make GIFs loop the correct number of times. Previously, everyone looped
one time too few for non-infinitely-looping GIFs.

Reviewed by Darin Adler.

Modified a Qt manual test to be correct and moved it to the general
manual test directory.

  • manual-tests/animated-gif-looping.html: Copied from WebCore/manual-tests/qt/qt-gif-test.html.
  • manual-tests/qt/qt-10loop-anim.gif: Removed.
  • manual-tests/qt/qt-anim.gif: Removed.
  • manual-tests/qt/qt-gif-test.html: Removed.
  • manual-tests/qt/qt-noanim.gif: Removed.
  • manual-tests/resources/animated-10x.gif: Copied from WebCore/manual-tests/qt/qt-10loop-anim.gif and modified.
  • manual-tests/resources/animated-infinite.gif: Copied from WebCore/manual-tests/qt/qt-anim.gif.
  • manual-tests/resources/non-animated.gif: Copied from WebCore/manual-tests/qt/qt-noanim.gif.
  • platform/graphics/BitmapImage.cpp:

(WebCore::BitmapImage::internalAdvanceAnimation): For a loop count of n, show a total of n + 1 animation cycles.

  • platform/graphics/ImageSource.h:
  • platform/graphics/cg/ImageSourceCG.cpp:

(WebCore::ImageSource::repetitionCount):

  • platform/graphics/qt/ImageDecoderQt.cpp:

(WebCore::ImageDecoderQt::repetitionCount): Remove translation code now that WebCore matches Qt's internal handling of the loop count. Qt itself may still have a bug here.

  • platform/image-decoders/gif/GIFImageDecoder.cpp:

(WebCore::GIFImageDecoder::repetitionCount):

  • platform/image-decoders/gif/GIFImageReader.cpp:

(GIFImageReader::read): Translate loop count 0 to "loop infinitely" (by restoring one piece of the Mozilla code we'd removed).

Location:
trunk/WebCore
Files:
7 edited
4 moved

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r60374 r60375  
     12010-05-28  Peter Kasting  <pkasting@google.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=39857
     6        Make GIFs loop the correct number of times.  Previously, everyone looped
     7        one time too few for non-infinitely-looping GIFs.
     8
     9        Modified a Qt manual test to be correct and moved it to the general
     10        manual test directory.
     11
     12        * manual-tests/animated-gif-looping.html: Copied from WebCore/manual-tests/qt/qt-gif-test.html.
     13        * manual-tests/qt/qt-10loop-anim.gif: Removed.
     14        * manual-tests/qt/qt-anim.gif: Removed.
     15        * manual-tests/qt/qt-gif-test.html: Removed.
     16        * manual-tests/qt/qt-noanim.gif: Removed.
     17        * manual-tests/resources/animated-10x.gif: Copied from WebCore/manual-tests/qt/qt-10loop-anim.gif and modified.
     18        * manual-tests/resources/animated-infinite.gif: Copied from WebCore/manual-tests/qt/qt-anim.gif.
     19        * manual-tests/resources/non-animated.gif: Copied from WebCore/manual-tests/qt/qt-noanim.gif.
     20        * platform/graphics/BitmapImage.cpp:
     21        (WebCore::BitmapImage::internalAdvanceAnimation): For a loop count of n, show a total of n + 1 animation cycles.
     22        * platform/graphics/ImageSource.h:
     23        * platform/graphics/cg/ImageSourceCG.cpp:
     24        (WebCore::ImageSource::repetitionCount):
     25        * platform/graphics/qt/ImageDecoderQt.cpp:
     26        (WebCore::ImageDecoderQt::repetitionCount): Remove translation code now that WebCore matches Qt's internal handling of the loop count.  Qt itself may still have a bug here.
     27        * platform/image-decoders/gif/GIFImageDecoder.cpp:
     28        (WebCore::GIFImageDecoder::repetitionCount):
     29        * platform/image-decoders/gif/GIFImageReader.cpp:
     30        (GIFImageReader::read): Translate loop count 0 to "loop infinitely" (by restoring one piece of the Mozilla code we'd removed).
     31
    1322010-05-28  Ben Murdoch  <benm@google.com>
    233

  • trunk/WebCore/manual-tests/animated-gif-looping.html

    r60323 r60375  
    22<body>
    33<div>
    4     <img src="qt-anim.gif">
     4    <img src="./resources/animated-infinite.gif">
    55    <p>This should animate.</p>
    66</div>
    77<div>
    8     <img src="qt-noanim.gif">
     8    <img src="./resources/non-animated.gif">
    99    <p>This should not animate and you should see a cross.</p>
    1010</div
    1111<div>
    12     <img src="qt-10loop-anim.gif">
     12    <img src="./resources/animated-10x.gif">
    1313    <p>This animation should loop 10 times.</p>
    1414</div

  • trunk/WebCore/platform/graphics/BitmapImage.cpp

    r55199 r60375  
    405405        // repetition count before, we should have decoded the whole image by
    406406        // now, so it should now be available.
    407         if (repetitionCount(true) && m_repetitionsComplete >= m_repetitionCount) {
     407        // Note that we don't need to special-case cAnimationLoopOnce here
     408        // because it is 0 (see comments on its declaration in ImageSource.h).
     409        if (repetitionCount(true) != cAnimationLoopInfinite && m_repetitionsComplete > m_repetitionCount) {
    408410            m_animationFinished = true;
    409411            m_desiredFrameStartTime = 0;

  • trunk/WebCore/platform/graphics/ImageSource.h

    r59619 r60375  
    9292#endif
    9393
    94 const int cAnimationLoopOnce = -1;
     94// Right now GIFs are the only recognized image format that supports animation.
     95// The animation system and the constants below are designed with this in mind.
     96// GIFs have an optional 16-bit unsigned loop count that describes how an
     97// animated GIF should be cycled.  If the loop count is absent, the animation
     98// cycles once; if it is 0, the animation cycles infinitely; otherwise the
     99// animation plays n + 1 cycles (where n is the specified loop count).  If the
     100// GIF decoder defaults to cAnimationLoopOnce in the absence of any loop count
     101// and translates an explicit "0" loop count to cAnimationLoopInfinite, then we
     102// get a couple of nice side effects:
     103//   * By making cAnimationLoopOnce be 0, we allow the animation cycling code in
     104//     BitmapImage.cpp to avoid special-casing it, and simply treat all
     105//     non-negative loop counts identically.
     106//   * By making the other two constants negative, we avoid conflicts with any
     107//     real loop count values.
     108const int cAnimationLoopOnce = 0;
     109const int cAnimationLoopInfinite = -1;
    95110const int cAnimationNone = -2;
    96111

  • trunk/WebCore/platform/graphics/cg/ImageSourceCG.cpp

    r58831 r60375  
    203203        return result;
    204204
    205     // A property with value 0 means loop forever.
    206205    RetainPtr<CFDictionaryRef> properties(AdoptCF, CGImageSourceCopyProperties(m_decoder, imageSourceOptions()));
    207206    if (properties) {
    … …  
    209208        if (gifProperties) {
    210209            CFNumberRef num = (CFNumberRef)CFDictionaryGetValue(gifProperties, kCGImagePropertyGIFLoopCount);
    211             if (num)
     210            if (num) {
     211                // A property with value 0 means loop forever.
    212212                CFNumberGetValue(num, kCFNumberIntType, &result);
     213                if (!result)
     214                    result = cAnimationLoopInfinite;
     215            }
    213216        } else
    214217            result = cAnimationNone; // Turns out we're not a GIF after all, so we don't animate.

  • trunk/WebCore/platform/graphics/qt/ImageDecoderQt.cpp

    r58744 r60375  
    116116int ImageDecoderQt::repetitionCount() const
    117117{
    118     if (m_reader && m_reader->supportsAnimation()) {
     118    if (m_reader && m_reader->supportsAnimation())
    119119        m_repetitionCount = m_reader->loopCount();
    120 
    121         // Qt and WebCore have a incompatible understanding of
    122         // the loop count and we can not completely map everything.
    123         //  Qt   |   WebCore          | description
    124         //  -1   |     0              | infinite animation
    125         //   0   | cAnimationLoopOnce | show every frame once
    126         //   n   |     n+1            | Qt returns the # of iterations - 1
    127         //  n/a  | cAnimationNone     | show only the first frame
    128         if (m_repetitionCount == -1)
    129             m_repetitionCount = 0;
    130         else if (m_repetitionCount == 0)
    131             m_repetitionCount = cAnimationLoopOnce;
    132         else
    133             ++m_repetitionCount;
    134     }
    135 
    136120    return m_repetitionCount;
    137121}

  • trunk/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp

    r58589 r60375  
    9999        // the result from the reader _less_ authoritative on future calls.  To
    100100        // detect this, the reader returns cLoopCountNotSeen (-2) instead of
    101         // cAnimationLoopOnce (-1) when its current incarnation hasn't actually
     101        // cAnimationLoopOnce (0) when its current incarnation hasn't actually
    102102        // seen a loop count yet; in this case we return our previously-cached
    103103        // value.

  • trunk/WebCore/platform/image-decoders/gif/GIFImageReader.cpp

    r60255 r60375  
    7878#include <string.h>
    7979#include "GIFImageDecoder.h"
     80#include "ImageSource.h"
    8081
    8182using WebCore::GIFImageDecoder;
    … …  
    689690        loop_count = GETINT16(q + 1);
    690691
     692        /* Zero loop count is infinite animation loop request */
     693        if (loop_count == 0)
     694          loop_count = WebCore::cAnimationLoopInfinite;
     695
    691696        GETN(1, gif_netscape_extension_block);
    692697      }
Note: See TracChangeset for help on using the changeset viewer.