1145560 - Horizontal black lines when decoding large image if downscale-during-decode pref is turned on

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Pushlog:
(In reply to Alice0775 White from bug #1144899 comment #3)
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=161eb635963d&tochange=d45aaa6d2217
> Triggered by: Bug 1124084
> image.downscale-during-decode.enabled = false helps

STR:
(In reply to Steve England [:stevee] from bug #1144899 comment #0)
> 1. New profile, start firefox
> 2. Navigate to https://imgur.com/gallery/RaGvZ
> 3. Observe as the images are decoding, there are horizontal black lines
> across the image, which will disappear periodically and after decoding has
> completed
> CTRL+F5 will show the issue again if you need to see it.


[Tracking Requested - why for this release]: Regression


@ Seth Fowler [:seth] - Could you look on this issue?

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bug.png

Comment 2

[Seth Fowler [:seth] [:s2h]]

We ended up backing out bug 1124084 over this, but I'm going to leave this open, because all bug 1124084 did was turn on downscale-during-decode by default. This is still reproducable by turning on the pref manually.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

It sounds like it may be unlikely to happen for very many users at this point, since the pref is disabled by default now.  Tracking anyway for now, since it's a recent regression.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Seth is this something you will have a chance to work on while 39 is still on aurora? I'm going through tracked bugs for 39 and wondering what you think.

Comment 6

[Seth Fowler [:seth] [:s2h]]

(In reply to Liz Henry (:lizzard) from comment #5)
> Seth is this something you will have a chance to work on while 39 is still
> on aurora? I'm going through tracked bugs for 39 and wondering what you
> think.

I'll see what I can do, but other things are taking priority at the moment. The pref is off by default and we definitely aren't going to turn it on by default in 39 or 40, so it probably doesn't matter much whether we fix it for those releases.

(FWIW, it may already be fixed by bug 1146335, but I haven't had a chance to test it again since.)

Comment 7

[Seth Fowler [:seth] [:s2h]]

Steve, I wanted to come back to this and try to figure out whether bug 1146335 fixed it, and I remembered that I couldn't reproduce this on my machine anyway.

Could you please try setting "image.downscale-during-decode.enabled" to true on current Nightly and let me know whether you can still reproduce the horizontal black lines you reported before?

Comment 8

[Steve England [:stevee]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0 ID:20150429030202 CSet: 1ad65cbeb2f4

I tested with the above build, a new profile, and image.downscale-during-decode.enabled set to true and I was still able to reproduce the bug.

bug 1146335 has not changed Firefox's behaviour; black lines are still visible whilst the images are decoding but disappear when the image has finished being decoded.

Comment 9

[Seth Fowler [:seth] [:s2h]]

Thank you very much for checking, Steve.

I'll try to figure out some way to reproduce this bug on my end.

Comment 10

[Steve England [:stevee]]

I just noticed that the build I tested with was a couple of days out of date, but the bug is still present in:

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0 ID:20150501030205 CSet: 7723b15ea695

Comment 11

[Seth Fowler [:seth] [:s2h]]

OK, I have figured out how to reproduce this. I needed to use a proxy to slow down my network speeds. If I throttle all the way down to modem speeds, I can reproduce the issue on a single image, for example this one:

https://i.imgur.com/CneDJ6C.jpg

Now that I can reproduce the issue, hopefully I can get to the bottom of this.

Comment 12

[Seth Fowler [:seth] [:s2h]]

Attachment: During downscale-during-decode, compute the original size invalid rect based on the target size invalid rect

OK, got it. The problem is in how we're computing the "original size" invalidation rect.

So during downscale-during-decode, we need *two* invalidation rects. One of them
represents the area of the image that we've invalidated in terms of its original
size; this is the invalidation rect that everything outside of ImageLib sees.
The other represents the same thing, but in terms of the target size that we're
downscaling to; this is the invalidation rect that imgFrame uses to track
whether it's complete and which areas we're allowed to draw.

In nsJPEGDecoder.cpp, we were using Downscaler to compute the target size
invalidation rect, but we were assuming that the original size invalidation rect
was the same as usual. Unfortunately, this isn't true, because the Downscaler
may consume lines of the original image without writing them to the output
buffer right away. If we invalidate for those lines, then we end up drawing
black lines where there's nothing written in the buffer. Even worse, we'll never
invalidate for those lines again during decoding, so we never repaint those
areas and the black lines stick around until the image is completely decoded and
we do a final, post-decode invalidation.

The solution is to compute the original size invalid rect from the target size
invalid rect when we're using the Downscaler, which ensures that we're taking
into account what the Downscaler has *actually* written. I've placed this
computation in the Downscaler code, as every client is going to need this.

This completely gets rid of the black lines on my system.

Comment 13

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7250e020c71

Comment 14

[Seth Fowler [:seth] [:s2h]]

Try looks green.

Comment 15

[Seth Fowler [:seth] [:s2h]]

Thanks for the review! I just did another pass of testing in my Windows 8 VM, and everything looks good, so I think this is ready to land.

Comment 16

[Seth Fowler [:seth] [:s2h]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8fb4d69e9f44

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8fb4d69e9f44

Comment 18

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Thank you very much.
Verified as fixed with latest Nightly (2015-05-06).

Comment 19

[Steve England [:stevee]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0 ID:20150506030206 CSet: 60349cbc3d4e

Flipped image.downscale-during-decode.enabled to true and tested, also fixed for me. Thanks Seth!

Comment 20

[Seth Fowler [:seth] [:s2h]]

Happy to help! Thanks so much for verifying the fix!