Closed Bug 1145560 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: ImageLib, defect, major)

39 Branch
x86_64
Windows 7
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla40
Tracking Status
firefox37 --- unaffected
firefox38 --- unaffected
firefox39 + unaffected
firefox38.0.5 --- unaffected
firefox40 + verified

People

(Reporter: Virtual, Assigned: seth)

References

()

Details

(Keywords: nightly-community, regression)

Attachments

(2 files)

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?
Flags: needinfo?(seth)
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.
Assignee: nobody → seth
Summary: Horizontal black lines when decoding large image → Horizontal black lines when decoding large image if downscale-during-decode pref is turned on
No longer blocks: 1130802
Flags: needinfo?(seth)
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.
Flags: needinfo?(seth)
(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.)
Flags: needinfo?(seth)
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?
Flags: needinfo?(steve.england)
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.
Flags: needinfo?(steve.england)
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.
Attachment #8601263 - Flags: review?(tnikkel)
Try looks green.
Attachment #8601263 - Flags: review?(tnikkel) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/8fb4d69e9f44
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
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!
Depends on: 1162350
You need to log in before you can comment on or make changes to this bug.