Closed Bug 868871 Opened 7 years ago Closed 7 years ago

Blank lines flashes during loading of images with (CSS) opacity

Categories

(Core :: ImageLib, defect)

Product:
Core
Component:
ImageLib
Version:
23 Branch
Platform:
x86_64
All
Type:
defect
Priority:
Not set

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla23
Tracking Flags:
Tracking Status
firefox21 --- unaffected
firefox22 --- unaffected
firefox23 + verified

People

(Reporter: me, Assigned: joe)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

testcase.html
429 bytes, text/html
Details
Make a not-notifying RequestDecodeType
4.04 KB, patch
seth
: review+
Details | Diff | Splinter Review
Attached file testcase.html — 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130505 Firefox/23.0
Build ID: 20130505030916

Steps to reproduce:

STR:

- Load testcase
- CTRL - F5 a few times
- Notice random blank lines flashing during the image loading process.

This only happens on non-opague images (the testcase uses .9 opacity), and appeared for me in a recent version of Nightly, within the last few days. A quick test on Firefox 20 showed it is not affected.

The lines are very noticeable when loading big images. It appears as parts of the image are loaded with opacity 0.

OS: Windows x64
Comment on attachment 745713 [details]
testcase.html

><html>
>    <head>
>        <title>testcase</title>
>        <style>
>            body {
>                position: absolute;
>                height: 100%;
>                width: 100%;
>                margin: 0;
>            }
>
>            img {
>                max-height: 100%;
>                opacity: .9;
>            }
>        </style>
>    </head>
>    <body>
>        <img src="http://i.imgur.com/d5j9zty.jpg" />
>    </body>
></html>
Attachment #745713 - Attachment mime type: text/plain → text/html
OS: Windows 7 → All
Also reproducible on Linux (with additional tearing artifacts).

I noticed that the image gets re-painted more often when it is not opague. You can compare the loading process when opening the same image via View Image. It's loading is a lot more choppy when opague, but it won't produce artifacts.
Regression window:
Good:
http://hg.mozilla.org/mozilla-central/rev/40dafc376794
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130427 Firefox/23.0 ID:20130427083404
Bad:
http://hg.mozilla.org/mozilla-central/rev/9d8977cbbfc6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130427 Firefox/23.0 ID:20130427182505
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=40dafc376794&tochange=9d8977cbbfc6


Regression window:
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/0e45f1b9521f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130426 Firefox/23.0 ID:20130426134704
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/45d40d37b1df
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130426 Firefox/23.0 ID:20130426134805
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0e45f1b9521f&tochange=45d40d37b1df

Triggered by
45d40d37b1df	Joe Drew — Bug 853564 - Deal with outstanding decoding-done notifications from within RequestDecode so we don't have to wait for the event loop. r=seth
Blocks: 853564
Status: UNCONFIRMED → NEW
status-firefox21: --- → unaffected
tracking-firefox22: --- → ?
tracking-firefox23: --- → ?
Component: Layout: Images → ImageLib
Ever confirmed: true
Keywords: regression
Version: 23 Branch → 22 Branch
Matt, do you have any ideas as to where I should look here? This happens before we are finished downloading the image, and so we're not layerizing the image yet (see RasterImage::GetImageContainer()).

This is reproducible on OS X, but it's way easier to reproduce on Linux, maybe because of basic layers.

I suspect something related to the ThebesLayer missing an update, but haven't been able to prove that yet.
Flags: needinfo?(matt.woodrow)
Summary: Blank lines flashes during loading of semi-transparent images - Nightly regression → Blank lines flashes during loading of images with opacity:
Summary: Blank lines flashes during loading of images with opacity: → Blank lines flashes during loading of images with (CSS) opacity
Flags: needinfo?(matt.woodrow)
What happens here is that the image synchronously marks its frame as dirty from RasterImage::GetImageContainer(), which is called from underneath nsDisplayList::PaintForFrame.

The frame then gets marked as clean (since it was just drawn!) and that part of the frame, since it was just marked as valid, doesn't get drawn.

Tim, do you have any idea if we have a general way of solving this problem? Or should RasterImage::GetImageContainer() simply never invalidate?
Flags: needinfo?(tnikkel)
Assuming that we absolutely can't invalidate while painting, this patch fixes the problem.

The naming is sort of awful. But it's pretty simple.
Attachment #747124 - Flags: review?(seth)
Whoops - remove some debugging code.
Attachment #747124 - Attachment is obsolete: true
Attachment #747124 - Flags: review?(seth)
Attachment #747127 - Flags: review?(seth)
Comment on attachment 747127 [details] [diff] [review]
Make a not-notifying RequestDecodeType

Review of attachment 747127 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.
Attachment #747127 - Flags: review?(seth) → review+
Wait, why is GetImageContainer doing anything at all in this case? GetCurrentImage makes us do nothing if mDecoded is false.
Flags: needinfo?(tnikkel)
Also, I thought we could handle invalidations during painting, but I'm not sure how DLBI changed that, and I'd have to check to see if I'm remembering correctly.
No, that would definitely make DLBI sad.
(In reply to Timothy Nikkel (:tn) from comment #10)
> Wait, why is GetImageContainer doing anything at all in this case?
> GetCurrentImage makes us do nothing if mDecoded is false.

Current code:

already_AddRefed<layers::Image>
RasterImage::GetCurrentImage()
{
  if (!mDecoded) {
    // We can't call StartDecoding because that can synchronously notify
    // which can cause DOM modification
    RequestDecode();
    return nullptr;
  }

This patch changes RequestDecode to RequestDecodeCore(ASYNCHRONOUS).
Luckily, this doesn't happen on Firefox 22, because I fixed bug 853564 in a totally different way on Aurora.
Version: 22 Branch → 23 Branch
https://hg.mozilla.org/mozilla-central/rev/98e5c35041c0
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox23: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0

Verified as fixed on Firefox 23 beta 7 (buildID: 20130718163513) and latest Nightly (buildID: 20130719030204).
Status: RESOLVED → VERIFIED
status-firefox23: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.