New issue
Advanced search Search tips

Issue 700892 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 437662
issue 701143



Sign in to add a comment

Add a LayoutTest for a partially loaded APNG

Project Member Reported by scroggo@chromium.org, Mar 13 2017

Issue description

APNG support ( issue 437662 ) is in progress at https://codereview.chromium.org/2618633004/

From that review (https://codereview.chromium.org/2618633004/diff/240001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp#newcode486):

"could we add a test for that in the animated image case please?  A bug about it would be enough.  You would add a HTTP layout test using the load-and-stall script, see for example:

LayoutTests/http/tests/images/png-partial-load.html
LayoutTests/http/tests/images/jpeg-partial-load.html"
 

Comment 1 by noel@chromium.org, Mar 13 2017

Thanks for filing this, some notes.  Perhaps use the APNG we added on 

https://crrev.com/427dfb952160b6402103f5db70a0e3dc6c5c500d/third_party/WebKit/LayoutTests/images/resources/count-down-color-test.gif

Copy http/tests/images/jpeg-partial-load.html to http/tests/images/png-animated-partial-load.html, and use the image above in it, set the mimeType for animated PNG (image/apng, I believe), then manually work out the &stallAt= value.

That value is the offset into the encode file stream bytes to stall at.  Set it so that the test draws ~1/2 the initial decoded image frame (frame 0).  This will require some fiddling about to determine the value in bytes.

Indirectly, this test will also check the decoder's alpha handling of the first frame.  The background CSS color should be seen (green IIRC) since the frame is partially decoded (it's state is not ImageFrame::FrameComplete), it should have alpha everywhere outside the decoded regions.

HTH.

Comment 3 by noel@chromium.org, Mar 16 2017

Summary: Add a LayoutTest for a partially loaded APNG (was: Add a LayoutTest for progressively loading APNG)

Comment 4 by noel@chromium.org, Mar 16 2017

Cc: scroggo@chromium.org
Owner: noel@chromium.org
Status: Assigned (was: Untriaged)

Comment 5 by noel@chromium.org, Mar 16 2017

Status: Started (was: Assigned)
https://codereview.chromium.org/2743253009/
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ef26ee13e1d61e0d52daf64bd4ad2ef7a7907d5

commit 2ef26ee13e1d61e0d52daf64bd4ad2ef7a7907d5
Author: noel <noel@chromium.org>
Date: Thu Mar 16 21:44:07 2017

Add a partial load test for an animated PNG image

Use HTTP test load-and-stall script to partially load the initial
frame of a animated PNG image.

Render the image over CSS background color green and confirm that
the CSS color appears in the undecoded regions of the image.

BUG= 700892 

Review-Url: https://codereview.chromium.org/2743253009
Cr-Commit-Position: refs/heads/master@{#457569}

[modify] https://crrev.com/2ef26ee13e1d61e0d52daf64bd4ad2ef7a7907d5/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/2ef26ee13e1d61e0d52daf64bd4ad2ef7a7907d5/third_party/WebKit/LayoutTests/http/tests/images/png-animated-partial-load.html

Comment 7 by noel@chromium.org, Mar 16 2017

Results look good, watching it run around the bots for some number of cycles to check results are stable (not flakey).

Comment 8 by noel@chromium.org, Mar 16 2017

Blocking: 701143
> Ahem, that's a GIF, meant the APNG image of course ...

And we should add a test for an animated GIF too, since APNG and GIF test coverage came up while reviewing https://codereview.chromium.org/2749703002  bug 701143 .

Animated GIF case done on  bug 702148 .

Comment 9 by noel@chromium.org, Mar 17 2017

#7 Stability looks good.


png-animated-partial-load.png
331 KB View Download

Comment 10 by noel@chromium.org, Mar 17 2017

Result looks good, CSS green background color visible in undecoded parts of the image.
png-animated-partial-load-result.png
23.1 KB View Download

Comment 11 by noel@chromium.org, Mar 17 2017

Status: Fixed (was: Started)

Comment 12 by noel@chromium.org, Mar 17 2017

This to cover any changes in https://codereview.chromium.org/2749703002 and the patch split out from there.

Comment 13 by noel@chromium.org, Mar 31 2017

Blockedon: -437662
Blocking: 437662

Sign in to add a comment