Some animation WebPs with lossy-compression aren't rendered correctly

RESOLVED FIXED in Firefox 65

Status

()

defect
P3
normal
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: yamadat501, Assigned: aosmond)

Tracking

unspecified
mozilla65
Points:
---

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 months ago
I noticed it while checking Bug 1503935 was fixed.

STR:
1. Set image.webp.enabled to true.
2. Open http://littlesvr.ca/apng/gif_apng_webp3.html

Expected result:
All animation plays normally.

Actual result:
Animation with lossy webp sometimes shows black blinks.

This happens in Test5 also and doesn't happen in Test 1,2,4.
(Assignee)

Updated

7 months ago
Flags: needinfo?(aosmond)
Priority: -- → P3
(Assignee)

Updated

7 months ago
Blocks: WebP
Flags: needinfo?(aosmond)
(Assignee)

Updated

7 months ago
Flags: needinfo?(aosmond)
(Assignee)

Comment 1

7 months ago
I thought that bug fixed this as I wasn't able to reproduce with it, but I saw it just now. Investigating.
Assignee: nobody → aosmond
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(aosmond)
(Assignee)

Comment 2

7 months ago
It it consistently reproducible in a debug build for me. I set the discarding threshold to be very high so it never discards the frames after producing them. Setting image.animated.generate-full-frames to true seems to resolve the issue (for me), so I wonder if it is a bug in FrameAnimator.
(Reporter)

Comment 3

7 months ago
(In reply to Andrew Osmond [:aosmond] from comment #2)
> Setting image.animated.generate-full-frames to true seems to
> resolve the issue (for me), so I wonder if it is a bug in FrameAnimator.

Confirmed that this works as a workaround on current nightly build(2018-11-02-10), too.
(Assignee)

Comment 4

7 months ago
(In reply to Toshihiro Yamada from comment #3)
> (In reply to Andrew Osmond [:aosmond] from comment #2)
> > Setting image.animated.generate-full-frames to true seems to
> > resolve the issue (for me), so I wonder if it is a bug in FrameAnimator.
> 
> Confirmed that this works as a workaround on current nightly
> build(2018-11-02-10), too.

Awesome, thanks.

I think what is going wrong (and there should be a similar bug in APNG) is that the partial frames are incorrectly produced. APNG and WebP currently allocate a full sized frame buffer, but move the partial frame contents to the right location; soon to be historical, the pref I mentioned fixes that. The rest of the frame data should all be zeroes. However if the frame is marked as having no alpha, then it will actually use BGRX and memset everything to 0xFF, which is solid white. Hence the distortions. What bothers me is why it seems inconsistent about it from decode to decode. It should be very consistent.
(Assignee)

Comment 5

7 months ago
For decoders which produce unpaletted partial frames (APNG, WebP), the
surface format should always be BGRA.
(Assignee)

Comment 6

7 months ago
So this is actually a bug in RemoveFrameRectFilter. If it needed to buffer inside the filter, it does the right thing:

https://searchfox.org/mozilla-central/rev/39cb1e96cf97713c444c5a0404d4f84627aee85d/image/SurfaceFilters.h#923

But if it could reuse the underlying buffer, it will only write the pixels that are provided -- it leaves the prefix and postfix of the row untouched. If we set the surface format to BGRA, we don't need to worry about writing them.
(Assignee)

Comment 7

7 months ago
After some thought, it is both, although mainly the surface format's fault.

If it comes in a BGRX, the pixman blending operation actually overwrites the entire compositing buffer with the new frame's data. Which is sort of garbage. Since pixman ignores the alpha channel, some of it will be black and some of it will be white (due to the RemoveFrameRectFilter bug). We don't see this immediately because animated images provide a dirty rect of what changed -- typically that means Gecko will only redraw what changed.

However Gecko is free to repaint as much as it wishes. When there are multiple things animated, it may choose to repaint broader areas as a result. This is probably why it is only observed sometimes, and with seemingly random severity.

Ideally to go with the format change, I can write a mochitest that forces a full repaint, and confirms the compositing buffer wasn't overwritten with garbage.
(Assignee)

Comment 8

7 months ago
It is pretty easy to observe on an animated image that ends with a bad frame, and setting image.animation_mode to once. If you zoom in and out, it forces a full repaint, and you see the bad contents.
(Assignee)

Comment 9

7 months ago
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a8912441d21f8336dba704a248dedc99d205b9e

Wrote the tests as gtests instead, BlendWebPWithAnimator failed before, and passes after the decoder fix.
(Assignee)

Updated

7 months ago
Status: NEW → ASSIGNED

Comment 10

7 months ago
Can anybody else see a significant delay to load/render WebP images?
Pageload seems to hang when they are fist loaded. Loading them from cache seems to be no problem, so a page refresh fixes the problem for me.
Same as sjw: on first load it doesn't show the animated webp, I have  to reload from cache for them to be displayed.

Comment 12

7 months ago
Pushed by aosmond@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fb87d2312ea
Ensure partial animated frames always use BGRA instead of BGRX. r=tnikkel

Comment 13

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5fb87d2312ea
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65

Comment 14

6 months ago
I can confirm this behavior (sJw and Robert-André). I also mentioned it in:
https://bugzilla.mozilla.org/show_bug.cgi?id=1504120

Just an example webp animation
https://pullzone1-corydowdywebdesi.netdna-ssl.com/assets/blog/apngwebp/squirrel.default.webp
It only loads when it's in the cache. For the first load or if you ignore caches (strg + F5) it just keeps on loading (endlessly) without displaying anything

This part of the bug is not fixed in this nightly: 65.0a1 (2018-11-06) (64-Bit)

It seems to be the the case for every webp animation and not just some.
(Assignee)

Comment 15

6 months ago
(In reply to Djfe from comment #14)
> I can confirm this behavior (sJw and Robert-André). I also mentioned it in:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1504120
> 
> Just an example webp animation
> https://pullzone1-corydowdywebdesi.netdna-ssl.com/assets/blog/apngwebp/
> squirrel.default.webp
> It only loads when it's in the cache. For the first load or if you ignore
> caches (strg + F5) it just keeps on loading (endlessly) without displaying
> anything
> 
> This part of the bug is not fixed in this nightly: 65.0a1 (2018-11-06)
> (64-Bit)
> 
> It seems to be the the case for every webp animation and not just some.

Sounds like bug 1504448.
You need to log in before you can comment on or make changes to this bug.