Closed
Bug 1504237
Opened 6 years ago
Closed 6 years ago
Some animation WebPs with lossy-compression aren't rendered correctly
Categories
(Core :: Graphics: ImageLib, defect, P3)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: yamadat501, Assigned: aosmond)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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•6 years ago
|
Flags: needinfo?(aosmond)
Priority: -- → P3
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(aosmond)
Assignee | ||
Comment 1•6 years 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•6 years 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•6 years 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•6 years 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•6 years ago
|
||
For decoders which produce unpaletted partial frames (APNG, WebP), the
surface format should always be BGRA.
Assignee | ||
Comment 6•6 years 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•6 years 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•6 years 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•6 years 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•6 years ago
|
Status: NEW → ASSIGNED
Comment 10•6 years 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.
Comment 11•6 years ago
|
||
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•6 years 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•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Comment 14•6 years 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 years 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.
Description
•