Bug 17022 - Add APNG support
: Add APNG support
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Images
: 528+ (Nightly build)
: All All
: P3 Enhancement
Assigned To: Nobody
http://wiki.mozilla.org/APNG_Specific...
:
Depends on: 142726
Blocks: 142731
  Show dependency treegraph
 
Reported: 2008-01-26 09:26 PST by José Jeria
Modified: 2015-03-20 00:57 PDT (History)
38 users (show)

See Also:


Attachments
Add APNG support, re-bundle libpng with APNG patch (945.32 KB, patch)
2009-05-28 20:11 PDT, Mark Steele
no flags Details | Formatted Diff | Diff
v01_apng_webkit.patch (22.76 KB, patch)
2013-03-13 10:13 PDT, Max Stepin
abarth: review-
Details | Formatted Diff | Diff
v02_apng_webkit.patch (23.60 KB, patch)
2013-04-04 15:32 PDT, Max Stepin
benjamin: review-
Details | Formatted Diff | Diff
v03_apng_webkit.patch (75.58 KB, patch)
2014-08-06 14:36 PDT, Max Stepin
no flags Details | Formatted Diff | Diff
v04_apng_webkit.patch (75.45 KB, patch)
2014-08-09 08:12 PDT, Max Stepin
no flags Details | Formatted Diff | Diff
v05_apng_webkit.patch (99.67 KB, patch)
2014-08-11 09:30 PDT, Max Stepin
no flags Details | Formatted Diff | Diff
v06_apng_webkit.patch (85.04 KB, patch)
2014-08-11 09:57 PDT, Max Stepin
buildbot: commit‑queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (555.80 KB, application/zip)
2014-08-11 13:44 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (745.79 KB, application/zip)
2014-08-11 14:30 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (567.74 KB, application/zip)
2014-08-11 15:22 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (601.23 KB, application/zip)
2014-08-11 16:30 PDT, Build Bot
no flags Details
v07_apng_webkit.patch (85.66 KB, patch)
2014-08-12 07:27 PDT, Max Stepin
no flags Details | Formatted Diff | Diff
v08_apng_webkit.patch (85.99 KB, patch)
2015-03-11 14:44 PDT, Max Stepin
no flags Details | Formatted Diff | Diff
v09_apng_webkit.patch (86.12 KB, patch)
2015-03-12 14:58 PDT, Max Stepin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description José Jeria 2008-01-26 09:26:20 PST
Please add APNG support to WebKit. Firefox and Opera already supports this.
Comment 1 Mark Rowe (bdash) 2008-01-26 11:09:57 PST
WebKit uses the platform's underlying image libraries for format support, so APNG would need to be implemented there for WebKit to decode it.  This would require ImageIO on Mac/Windows to support it (<rdar://problem/5618737>), and QImage for Qt.  Only the Gtk and wx ports make use of the bare basic image decoders in the WebKit source tree.
Comment 2 Mark Rowe (bdash) 2008-01-26 11:15:04 PST
Closer inspection shows that our in-tree PNG decoder is just a wrapper around libpng.  For the Gtk and wx ports to support APNG we would need libpng to support it.
Comment 3 Christian Dywan 2008-01-30 08:10:06 PST
Incidentally there are patches available for libpng to support apng, but they are still waiting for approval. Time will tell whether WebKit needs to ship a fork of libpng or upstream supports this in the future.

http://littlesvr.ca/apng/
Comment 4 Mark Steele 2009-05-28 20:11:43 PDT
Created attachment 30764 [details]
Add APNG support, re-bundle libpng with APNG patch

Rough patch to PNGImageDecoder to support APNG.
Bundle libpng with APNG patch.
A few things are missing/broken, but basic support works on Webkit/GTK+
Comment 5 Alexey Proskuryakov 2010-08-11 02:28:35 PDT
Bug 43828 tracks APNG support in Safari (as mentioned above, that's blocked on ImageIO).
Comment 6 Jared Grippe 2011-04-12 18:01:01 PDT
want!
Comment 7 Max Stepin 2013-03-13 10:13:50 PDT
Created attachment 192943 [details]
v01_apng_webkit.patch

If external libpng supports APNG extension, ask libpng to decode animation frames. 

Some Linux distros (Arch, Gentoo, Mageia, etc) ship with apng-patched libpng as a system library. With this patch WebKit would take advantage of that and request libpng to read APNG frames.
Comment 8 Adam Barth 2013-03-20 22:55:22 PDT
Comment on attachment 192943 [details]
v01_apng_webkit.patch

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

We shouldn't land this patch unless one or more WebKit work wishes to enable this code.  If we're going to accept this patch, we'll also need a number of tests.

> Source/WebCore/ChangeLog:3
> +        [GTK] [wx] Add APNG support

This issue is not specific to the GTK and wx ports, so it shouldn't have these prefixes.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:108
> +#ifdef PNG_APNG_SUPPORTED

This should read:

#if ENABLE(APNG)

Also, all the code introduced to support APNG should be behind this compile flag.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:241
> +    , m_frameIsHidden(false)
> +    , m_curFrame(0)
> +    , m_loopsCount(0)
> +    , m_delayNum(1)
> +    , m_delayDen(1)
> +    , m_dispose(0)
> +    , m_blend(0)

All this state as well.
Comment 9 ManDay 2013-04-01 09:30:35 PDT
Please do NOT add "apng" support to Webkit. "gif" was a first mistake in design of the web - these days luckily being replaced by proper video one after another. Adding apng works against the combined efforts to build a clean (flash free, GIF free) web where images are images (displayed by webkit itself) and animations are animations (displayed by gstreamer or a similar library which specializes on rendering animations).

Seperation of responsibilities and specialization is the foundation of open source and provides clean, stream-lined concepts. The PNG-developers do not disapprove of apng without a reason.
Comment 10 Christian Dywan 2013-04-01 10:07:58 PDT
(In reply to comment #8)
> We shouldn't land this patch unless one or more WebKit work wishes to enable this code.  If we're going to accept this patch, we'll also need a number of tests.

From Midori's perspective, I'd encourage enabling APNG - if WebKitGTK/ Epiphany team agree. In Archlinux it could work out of the box.
Comment 11 Max Stepin 2013-04-04 15:32:14 PDT
Created attachment 196541 [details]
v02_apng_webkit.patch

New code is now in 

#if ENABLE(APNG) && defined(PNG_APNG_SUPPORTED)
#endif

Also added this ENABLE_APNG flag into Source/WTF/wtf/FeatureDefines.h
Comment 12 Benjamin Poulain 2013-04-04 15:44:03 PDT
Comment on attachment 196541 [details]
v02_apng_webkit.patch

No tests?

Did any port actually agree to maintain this? If not, there is no point.
Comment 13 Allan Sandfeld Jensen 2013-04-07 13:40:55 PDT
(In reply to comment #11)
> Created an attachment (id=196541) [details]
> v02_apng_webkit.patch
> 
> New code is now in 
> 
> #if ENABLE(APNG) && defined(PNG_APNG_SUPPORTED)
> #endif
> 
> Also added this ENABLE_APNG flag into Source/WTF/wtf/FeatureDefines.h

I would suggest setting ENABLE_APNG based on defined(PNG_APNG_SUPPORTED), that way you don't have to check for both, and ENABLE_APNG will only be set when APNG is supported.
Comment 14 Allan Sandfeld Jensen 2013-04-11 02:32:05 PDT
(In reply to comment #12)
> (From update of attachment 196541 [details])
> No tests?
> 
> Did any port actually agree to maintain this? If not, there is no point.

Hopefully it will not require much maintaince. Though it will need tests, and someone should run a bot with it enabled. I guess for GTK and Qt it will depend on the libpng available on the system.
Comment 15 Max Stepin 2014-08-06 14:36:17 PDT
Created attachment 236137 [details]
 v03_apng_webkit.patch

I rewrote the APNG support code, please take a look.
     
This version decodes APNG using only standard libpng (turns out it's possible).
     
I also added test files: a dozen small animations that would play once, and finish asap. After a short delay a pixel-test can be compared against the expected result. It covers most of the apng features at different png colortypes.
     
I'm going to ask GTK port people, and will try to contact other ports too.

     
P.S. Safari's own implementation of APNG might benefit from the test suite too.
Comment 16 Benjamin Poulain 2014-08-06 17:36:06 PDT
I think you could convert the test to a ref-test instead of a pixel test. Tests should have a description and explanations of what is tested and how.
Comment 17 Max Stepin 2014-08-06 21:26:33 PDT
OK. I was looking at LayoutTests/fast/images/png-suite/test.html 
as an example. But I can convert it into a reftest.
Comment 18 Max Stepin 2014-08-09 08:12:12 PDT
Created attachment 236335 [details]
v04_apng_webkit.patch

Now with reftest. All animations included in the reftest should play once and stop at the last frame. After that, compare the result against reference static PNGs.
Comment 19 Max Stepin 2014-08-11 09:30:32 PDT
Created attachment 236373 [details]
v05_apng_webkit.patch

Previous patch was formatted incorrectly. Fixed now.
Comment 20 Max Stepin 2014-08-11 09:57:11 PDT
Created attachment 236375 [details]
v06_apng_webkit.patch

Really fixed now. Sorry, I'm new to this.
Comment 21 Build Bot 2014-08-11 13:44:43 PDT
Comment on attachment 236375 [details]
v06_apng_webkit.patch

Attachment 236375 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6305820962717696

New failing tests:
fast/images/animated-png.html
Comment 22 Build Bot 2014-08-11 13:44:50 PDT
Created attachment 236396 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 23 Build Bot 2014-08-11 14:30:28 PDT
Comment on attachment 236375 [details]
v06_apng_webkit.patch

Attachment 236375 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4795727104442368

New failing tests:
fast/images/animated-png.html
Comment 24 Build Bot 2014-08-11 14:30:40 PDT
Created attachment 236402 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 25 Build Bot 2014-08-11 15:22:46 PDT
Comment on attachment 236375 [details]
v06_apng_webkit.patch

Attachment 236375 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5712234470703104

New failing tests:
fast/images/animated-png.html
Comment 26 Build Bot 2014-08-11 15:22:58 PDT
Created attachment 236406 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 27 Build Bot 2014-08-11 16:30:35 PDT
Comment on attachment 236375 [details]
v06_apng_webkit.patch

Attachment 236375 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5681974278619136

New failing tests:
fast/images/animated-png.html
Comment 28 Build Bot 2014-08-11 16:30:47 PDT
Created attachment 236412 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 29 Max Stepin 2014-08-12 07:27:45 PDT
Created attachment 236444 [details]
v07_apng_webkit.patch

It appears that mac target doesn't use WebKit's internal image decoders, so I'll going to skip that test, just for mac.
Comment 30 Max Stepin 2014-08-19 05:00:24 PDT
Comment on attachment 236444 [details]
v07_apng_webkit.patch

I'm adding "review?" flag, since EFL is ok with enabling it as experimental feature. See here:
https://lists.webkit.org/pipermail/webkit-efl/2014-August/thread.html

I'm willing to help with the maintenance, the code is well tested and clean, so I don't expect any issues.
Comment 31 OC2PS 2015-02-27 20:42:23 PST
+1

Add APNG support. 

Max's patch works. And he's willing to maintain. 

Takes away all excuses not to.
Comment 32 Benjamin Poulain 2015-02-28 01:02:55 PST
(In reply to comment #31)
> +1
> 
> Add APNG support. 
> 
> Max's patch works. And he's willing to maintain. 
> 
> Takes away all excuses not to.

This is for GTK right?

Has anyone contacted the GTK folks to get a review on this patch? WebKit-GTK has a mailing list: https://lists.webkit.org/mailman/listinfo/webkit-gtk
Comment 33 Max Stepin 2015-03-03 05:01:55 PST
I asked them twice back at August, but couldn't get any straight answer:
https://lists.webkit.org/pipermail/webkit-gtk/2014-August/thread.html

ELF folks seems to be a little bit interested, and willing to try it as experimental feature, but I'll understand if you think one small port is not enough.

I really wish GTK folks would make their position clear.
Comment 34 Benjamin Poulain 2015-03-03 13:52:52 PST
(In reply to comment #33)
> I asked them twice back at August, but couldn't get any straight answer:
> https://lists.webkit.org/pipermail/webkit-gtk/2014-August/thread.html
> 
> ELF folks seems to be a little bit interested, and willing to try it as
> experimental feature, but I'll understand if you think one small port is not
> enough.
> 
> I really wish GTK folks would make their position clear.

The GTK port is more active than EFL, they also have more users. I am concerned about landing this if they do not approve.
Comment 35 Carlos Garcia Campos 2015-03-05 01:52:51 PST
Comment on attachment 236444 [details]
v07_apng_webkit.patch

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

I'm fine with adding APNG support to WebKitGTK+. I don't know the APNG details, so I can't review this from that point of view (fortunately it includes layout tests), but I can comment about coding style and other general nits. Thanks for the patch!

> Source/WTF/ChangeLog:3
> +        Added ENABLE(APNG)

Use the bug title here

> Source/WebCore/platform/image-decoders/ImageDecoder.h:169
> +        inline unsigned divide255(unsigned a)

This could probably be static

> Source/WebCore/platform/image-decoders/ImageDecoder.h:177
> +            if (!a)
> +                return;

Shouldn't we initialize dest in this case? *dest = 0;

> Source/WebCore/platform/image-decoders/ImageDecoder.h:201
> +                } else
> +                if (m_premultiplyAlpha) {

This should be a single line.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:244
> +    , m_png(0)

Use nullptr instead of 0 to initialize all pointer members.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:259
> +    , m_width(0)
> +    , m_height(0)

Could this be an IntSize?

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:261
> +    , m_xOffset(0)
> +    , m_yOffset(0)

And this IntPoint?

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:295
> +        return 0;

nullptr.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:433
> +            } else
> +            if (colorType == PNG_COLOR_TYPE_GRAY) {

This should be a single line.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:478
> +        m_gamma = (int)(gamma*100000);

Use C++ style cast here and leave space around the *

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:568
> +#if ENABLE(APNG)
> +        if (!m_curFrame)
> +#endif

So, it seems many things depend on m_curFrame, I wonder if we could move m_curFrame out of the #ifdef, but it will always be 0 for non APNG. This way we can avoid a lof of #ifdefs in the code I think.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:725
> +            return 1;

So, what's this function supposed to return? It seems we are always retuning 1. If that's the case, I would make this function void and return 1 unconditionally in the static readChunks()

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:742
> +        for (size_t i = 0; i < m_frameCount; ++i)
> +            m_frameBufferCache[i].setPremultiplyAlpha(m_premultiplyAlpha);

You could use a modern loop here 

for (auto& imageFrame : m_frameBufferCache)
    imageFrame.setPremultiplyAlpha(m_premultiplyAlpha);

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:744
> +    } else
> +    if (!memcmp(chunk->name, "fcTL", 4) && chunk->size == 26) {

This should be a single line.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:785
> +    } else
> +    if (!memcmp(chunk->name, "fdAT", 4) && chunk->size >= 4) {

This should be a single line

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:808
> +    double gamma;

Move this where it's first needed.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:809
> +    int bitDepth = png_get_bit_depth(m_png, m_info);

Ditto, this could be moved after the first if

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:849
> +        return; // Nothing to do.

I don't think the comment is needed.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:880
> +    if (frameIndex >= frameCount())
> +        return;

Could we check this before? I guess the frameRect computation doesn't affect this.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:882
> +    ImageFrame* const buffer = &m_frameBufferCache[frameIndex];

Could this be const ImageFrame& instead of using a pointer?

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:935
> +    else
> +    if (!m_delayDen)

This should be a single line

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:943
> +    else
> +    if (m_dispose == 1)

Ditto.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:1022
> +    png_byte dataPNG[8] = {137, 80, 78, 71, 13, 10, 26, 10};
> +    png_byte datagAMA[16] = {0, 0, 0, 4, 103, 65, 77, 65};

Could this be static?

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:1054
> +    png_byte dataIEND[12] = {0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130};

Ditto.

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:1078
> +    return 1;

Why does this method returns int if it always returns 1?

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.h:31
> +#include "png.h"

I guess this should be <png.h>

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.h:49
> +        virtual size_t frameCount() { return m_frameCount; }
> +        virtual int repetitionCount() const { return m_playCount-1; }

These should be marked as override

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.h:112
> +        unsigned m_curFrame;

m_currentFrame

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.h:116
> +        unsigned m_seqNum;

m_sequenceNumber

> Source/WebCore/platform/image-decoders/png/PNGImageDecoder.h:122
> +        unsigned m_delayNum;
> +        unsigned m_delayDen;

What do Num and Den stand for here? Do not use abbreviations.

> LayoutTests/ChangeLog:4
> +        Added APNG reftest.
> +        https://bugs.webkit.org/show_bug.cgi?id=17022

Keep the bug title as the title of all changelogs of the same commit.
Comment 36 Max Stepin 2015-03-11 14:44:46 PDT
Created attachment 248452 [details]
v08_apng_webkit.patch

OK, most of the suggestions implemented.

Few other cases:

> Shouldn't we initialize dest in this case? *dest = 0;

No, this is by design. Applying top layer with alpha = 0 over the background layer means nothing will change, and we can bailout early.

> Could this be an IntSize?
> And this IntPoint?

I would really like to keep them unsigned, makes it easier to work with libpng.

> I wonder if we could move m_curFrame out of the #ifdef, but it will always be 0 for non APNG. 
> This way we can avoid a lof of #ifdefs in the code I think.

Yes, it's always 0 for non APNG, but I would like to keep all new code inside #if ENABLE(APNG) as requested in comment 8.
Comment 37 Carlos Garcia Campos 2015-03-12 00:15:08 PDT
(In reply to comment #36)
> Created attachment 248452 [details]
> v08_apng_webkit.patch
> 
> OK, most of the suggestions implemented.

Thanks!

> Few other cases:
> 
> > Shouldn't we initialize dest in this case? *dest = 0;
> 
> No, this is by design. Applying top layer with alpha = 0 over the background
> layer means nothing will change, and we can bailout early.

Ok.

> > Could this be an IntSize?
> > And this IntPoint?
> 
> I would really like to keep them unsigned, makes it easier to work with
> libpng.

Ah, I see.
 
> > I wonder if we could move m_curFrame out of the #ifdef, but it will always be 0 for non APNG. 
> > This way we can avoid a lof of #ifdefs in the code I think.
> 
> Yes, it's always 0 for non APNG, but I would like to keep all new code
> inside #if ENABLE(APNG) as requested in comment 8.

Well, I disagree, as long as it builds with and without the APNG enabled, and the code not protected by the ifdefs doesn't affect the performance, I think it's better to reduce the amount of ifdefs to improve the code readability and maintainability.
Comment 38 Max Stepin 2015-03-12 00:22:18 PDT
(In reply to comment #37)
> Well, I disagree, as long as it builds with and without the APNG enabled,
> and the code not protected by the ifdefs doesn't affect the performance, I
> think it's better to reduce the amount of ifdefs to improve the code
> readability and maintainability.

All right, I'll fix that.
Comment 39 Max Stepin 2015-03-12 14:58:29 PDT
Created attachment 248547 [details]
v09_apng_webkit.patch

OK, moved m_currentFrame outside of the #ifdef, and simplified the code in a few places.
.
Comment 40 Carlos Garcia Campos 2015-03-16 06:38:59 PDT
Comment on attachment 248547 [details]
v09_apng_webkit.patch

Looks good to me, the test pass and demos work great. Thanks!
Comment 41 WebKit Commit Bot 2015-03-16 07:25:21 PDT
Comment on attachment 248547 [details]
v09_apng_webkit.patch

Clearing flags on attachment: 248547

Committed r181553: <http://trac.webkit.org/changeset/181553>
Comment 42 WebKit Commit Bot 2015-03-16 07:25:33 PDT
All reviewed patches have been landed.  Closing bug.
Comment 43 Brent Fulgham 2015-03-16 10:10:34 PDT
This change introduced a test failure on Windows. I'm working on getting tests running on Windows EWS again to avoid this problem in the future.

Any ideas why this might be failing?
Comment 44 Brent Fulgham 2015-03-16 10:13:40 PDT
(In reply to comment #43)
> This change introduced a test failure on Windows. I'm working on getting
> tests running on Windows EWS again to avoid this problem in the future.
> 
> Any ideas why this might be failing?

I'm guessing ImageIO on Windows doesn't support APNG. Since WinCairo uses the same image decoders as gtk+, it probably works there.
Comment 45 Carlos Garcia Campos 2015-03-16 10:34:02 PDT
(In reply to comment #44)
> (In reply to comment #43)
> > This change introduced a test failure on Windows. I'm working on getting
> > tests running on Windows EWS again to avoid this problem in the future.
> > 
> > Any ideas why this might be failing?
> 
> I'm guessing ImageIO on Windows doesn't support APNG. Since WinCairo uses
> the same image decoders as gtk+, it probably works there.

oh, right, I'm sorry, I guess the test should be skipped in win mac, but should probably work just fine in win cairo.
Comment 46 Brent Fulgham 2015-03-16 11:17:23 PDT
(In reply to comment #45)
> (In reply to comment #44)
> > (In reply to comment #43)
> > > This change introduced a test failure on Windows. I'm working on getting
> > > tests running on Windows EWS again to avoid this problem in the future.
> > > 
> > > Any ideas why this might be failing?
> > 
> > I'm guessing ImageIO on Windows doesn't support APNG. Since WinCairo uses
> > the same image decoders as gtk+, it probably works there.
> 
> oh, right, I'm sorry, I guess the test should be skipped in win mac, but
> should probably work just fine in win cairo.

I skipped the test on Apple Windows <https://trac.webkit.org/changeset/181564>.
Comment 47 Max Stepin 2015-03-16 12:22:52 PDT
Thanks everybody!

The same APNG Test Suite, online:
http://www.littlesvr.ca/apng/test.html

Now we have 3 implementations:

1. Firefox - open source (looks good)
2. WebKit - open source (looks good)
3. Safari - closed source (a little bit buggy)
Comment 48 Jon Lee 2015-03-16 12:34:36 PDT
(In reply to comment #47)
> Thanks everybody!
> 
> The same APNG Test Suite, online:
> http://www.littlesvr.ca/apng/test.html
> 
> Now we have 3 implementations:
> 
> 1. Firefox - open source (looks good)
> 2. WebKit - open source (looks good)
> 3. Safari - closed source (a little bit buggy)

Filed bug 142741 to cover this.
Comment 49 ChangSeok Oh 2015-03-19 20:28:07 PDT
(In reply to comment #48)
> (In reply to comment #47)
> > Thanks everybody!
> > 
> > The same APNG Test Suite, online:
> > http://www.littlesvr.ca/apng/test.html
> > 
> > Now we have 3 implementations:
> > 
> > 1. Firefox - open source (looks good)
> > 2. WebKit - open source (looks good)
> > 3. Safari - closed source (a little bit buggy)
> 
> Filed bug 142741 to cover this.

Out of interest, why apng, not webp nor jxr? If the compatibility among vendors is important to you, then is webp a better candidate for an alternative image format?
Comment 50 OC2PS 2015-03-19 21:49:04 PDT
> why apng, not webp nor jxr? 
JPEG XR is a *still-image* compression standard and file format.
APNG is an *animation* format.
 
>  If the compatibility among vendors is important to you, then is webp a better candidate for an alternative image format?
No. 
Only Google supports webp.

In any case this ticket is about getting APNG which is already supported by a few browsers to be supported by Webkit, not about which is the best image file format.

If you feel strongly about webp, please contribute at https://bugs.webkit.org/show_bug.cgi?id=105915
Comment 51 ChangSeok Oh 2015-03-20 00:57:24 PDT
(In reply to comment #50)
> > why apng, not webp nor jxr? 
> JPEG XR is a *still-image* compression standard and file format.
> APNG is an *animation* format.
>  
> >  If the compatibility among vendors is important to you, then is webp a better candidate for an alternative image format?
> No. 
> Only Google supports web.
And Opera does. =)

> In any case this ticket is about getting APNG which is already supported by
> a few browsers to be supported by Webkit, not about which is the best image
> file format.
Agree.

> If you feel strongly about webp, please contribute at
> https://bugs.webkit.org/show_bug.cgi?id=105915
Oh. I didn't know that webp was already there! Thanks for pointing the ticket.