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?

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.

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.

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.

(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.)

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?

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.

Thank you very much for checking, Steve. I'll try to figure out some way to reproduce this bug on my end.

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

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.

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.

Try looks green.

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.

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

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!

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