[Webkit-unassigned] [Bug 113124] Add animation support for WebP images

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=113124

Carlos Alberto Lopez Perez <clopez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #307748|review?                     |review-
              Flags|                            |

--- Comment #11 from Carlos Alberto Lopez Perez <clopez at igalia.com> ---
Comment on attachment 307748
  --> https://bugs.webkit.org/attachment.cgi?id=307748
Patch

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

I was unable to test this because it currently fails to build (on WebKitGTK+ trunk of today=r216073).
Can you try to fix the comments below and upload a new version? Thanks

> Source/WebCore/ChangeLog:23
> +        - layout and pixel tests

I think at least one layout test is needed for the patch to land. So please, add some layout test on the next version of the patch.
I think there are some examples of how this can be done on the old patch attached on this bug.

Since the Mac port doesn't use this image decoders, I think any test added should be marked as skipped on the general TestExpectation file <LayoutTests/TestExpectations> and then marked to pass (overriding the general expectation) on the GTK test expectation file <LayoutTests/platform/gtk/TestExpectations>

> Source/WebCore/ChangeLog:24
> +        - remove support of old WebP versions in CMake files (all distros with WebKitGTK ship a recent enough WebP with demuxer enabled)

Our current minimum baseline supported distribution (Debian 8) ships with this. So I think is fine to assume demuxer should be available.

> Source/WebCore/platform/image-decoders/webp/WEBPImageDecoder.cpp:57
> +    , m_formatFlags(0)
> +    , m_frameBackgroundHasAlpha(false)
> +    , m_demux(0)
> +    , m_demuxState(WEBP_DEMUX_PARSING_HEADER)
> +    , m_haveAlreadyParsedThisData(false)
> +    , m_repetitionCount(WebCore::RepetitionCountOnce)
> +    , m_decodedHeight(0)

Where are this class variables defined?

This fails to build for me: http://sprunge.us/PMGC
The EWS was happy, so perhaps something changed since the patch was uploaded?

> Source/cmake/FindWebP.cmake:34
>  pkg_check_modules(PC_WEBP QUIET libwebp)
>  

Maybe this could be:
pkg_check_modules(PC_WEBP QUIET libwebp libwebpdemux)

So below you don't need to append to those arrays, neither change the variable names on find_path() and find_library()

This means we would require webp with demuxer support built-in or we will skip to build any webp support at all.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170503/5f05bf90/attachment.html>