Closed Bug 917160 Opened 6 years ago Closed 6 years ago

preserveDrawingBuffer:false causes black canvas

Categories

(Core :: Canvas: WebGL, defect)

All
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla27
Tracking Status
firefox24 --- wontfix
firefox25 + verified
firefox26 + verified
firefox27 + verified
firefox-esr17 - wontfix
firefox-esr24 25+ verified

People

(Reporter: jdarpinian, Assigned: jgilbert)

References

Details

(Keywords: verifyme)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.37 Safari/537.36

Steps to reproduce:

Open the attached file on Windows 7. It uses WebGL to draw a green canvas. It runs a continuous requestAnimationFrame loop, but only uses WebGL once every 60 frames.


Actual results:

The canvas flashes black. If you don't see it immediately, try switching tabs.


Expected results:

The canvas should be green continuously.
Attachment #805788 - Attachment mime type: text/plain → text/html
Component: General → Canvas: WebGL
I'm not able to repro the issue with FF26 on Win 7.

Working range:
bad=2013-09-12
good=2013-09-13
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a4e9c9c9dbf9&tochange=b9029b1de410
Does not repro on Linux with basic or OGL layers.
I would guess it's one of the compositor changes that went in on that day. I'll switch over to windows to poke around.
Works on Win7 on 2013-09-09 and 2013-09-18.
Seems to work on Aurora26.

Does this still reproduce anywhere for you?
Flags: needinfo?(jdarpinian)
Looks like it's fixed in Nightly 27. My version of Aurora is still 25 (where it still repros), how do I get Aurora 26?
Flags: needinfo?(jdarpinian)
Aurora 25 should update to 26 by itself. You can also grab it off our FTP servers: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/
If it was reproducing on 25, that means it's in Beta, which is real bad.
Yep, that's what I see.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: x86_64 → All
I can reproduce it in 23, 24, and 25. It's been around for a long time apparently.

Actually, scratch my earlier report of it being fixed, because I just found a way to repro in 26 and 27 too. Start dragging the tab, and it will turn black as soon as the drag preview appears. This is a more reliable repro than before, though it can still happen randomly too.
(In reply to James Darpinian from comment #9)
> I can reproduce it in 23, 24, and 25. It's been around for a long time
> apparently.
> 
> Actually, scratch my earlier report of it being fixed, because I just found
> a way to repro in 26 and 27 too. 

Changed the tracking flags to match this comment.
See Also: → 890472
This reproduces consistently on 25 and below when you switch away from the testcase tab and switch back.
About 0.5s after switching back, the canvas almost always goes black for up to a second. During this time, we don't get to the SharedSurf part of UpdateSurface, likely because we don't think it's dirty.
Bas helped me figure this out. This is caused by (what appears to be) snap-shotting code, where we construct a BasicLayer tree. This means we call GLScreenbuffer::Readback() on the current consumer buffer, so that we can readback the pixels from what we're already displaying via d3d layers.

What this does on ANGLE is eglMakeCurrent() using the surface we desire. However, once we've swapped out the usual backbuffer for the current frontbuffer, we call ReadPixels, which sees that the 'draw' buffer is dirty, and so blits the cleared draw buffer onto our current frontbuffer!

The fix is to AssureBlitted when we change surfaces with MakeCurrent.
Attached patch blit-makecurrent (obsolete) — Splinter Review
This should do it, but I'm waiting on a clobber build. :\
Attachment #813911 - Flags: review?(bjacob)
Attachment #813911 - Attachment is patch: true
Attachment #813911 - Attachment mime type: message/rfc822 → text/plain
qref'd
Attachment #813911 - Attachment is obsolete: true
Attachment #813911 - Flags: review?(bjacob)
Attachment #813915 - Flags: review?(bjacob)
Awesome! Any idea whether the fix can be backported to 25 and 26?
Attachment #813915 - Flags: review?(bjacob) → review+
(In reply to James Darpinian from comment #15)
> Awesome! Any idea whether the fix can be backported to 25 and 26?

26 yes, and probably 25.
See Also: → 924241
Bug 924241 is somewhat related, where we forcibly clear-to-black when resizing the canvas.
https://hg.mozilla.org/mozilla-central/rev/1134b18371d3
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment on attachment 813915 [details] [diff] [review]
patch: AssureBlitted before changing EGLSurfaces with MakeCurrent

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Snapshotting + StreamingBuffers
User impact if declined: Intermittent rendering artifacts (black screens) on WebGL apps, particularly after changing tabs. Currently blocks activation of MapsGL on windows.
Testing completed (on m-c, etc.): local, on m-c
Risk to taking this patch (and alternatives if risky): Very low.
String or IDL/UUID changes made by this patch: None
Attachment #813915 - Flags: approval-mozilla-beta?
Attachment #813915 - Flags: approval-mozilla-aurora?
We'd like to unblock MapsGL, so we should take this on ESR 24 and pre-release branches. Is the current patch OK to approve for ESR24?
(In reply to Alex Keybl [:akeybl] from comment #21)
> We'd like to unblock MapsGL, so we should take this on ESR 24 and
> pre-release branches. Is the current patch OK to approve for ESR24?

Yes.
If this lands in 24, will the user agent string allow us to tell an updated version of 24 from a older version? I doubt we would want to enable WebGL for 24 unless we could guarantee that older versions wouldn't be a problem.
(In reply to James Darpinian from comment #23)
> If this lands in 24, will the user agent string allow us to tell an updated
> version of 24 from a older version? I doubt we would want to enable WebGL
> for 24 unless we could guarantee that older versions wouldn't be a problem.

No, it intentionally will not. That being said, we can land there anyway and this feature can optionally be enabled for ESR users once most Release channel users update away.
Attachment #813915 - Flags: approval-mozilla-esr24+
Attachment #813915 - Flags: approval-mozilla-beta?
Attachment #813915 - Flags: approval-mozilla-beta+
Attachment #813915 - Flags: approval-mozilla-aurora?
Attachment #813915 - Flags: approval-mozilla-aurora+
Blocks: 901574
Duplicate of this bug: 901574
Keywords: verifyme
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Windows NT 6.1; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Windows NT 6.1; rv:26.0) Gecko/20100101 Firefox/26.0



The issue still reproduces, the canvas flashes black if the "alt" key is pressed to bring up the menu bar on Firefox 25 beta 7 (build ID: 20131010180222) and on latest Aurora (build ID: 20131010004002).
QA Contact: cornel.ionce
Summoning the menu bar with 'alt' resizes the page, which is tracked separately in bug 924241. I've confirmed that bug 924241 doesn't affect Google Maps because we redraw the canvas every time the page is resized. This patch fixes the issue for us in 25, 26, and 27. Thanks for being so responsive on this!
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; rv:27.0) Gecko/20100101 Firefox/27.0

Also verified on latest Nightly and indeed it not reproduces if the menu bar is not summoned. 

Marking as verified for FF 25, 26 and 27.
Cornel, can you verify ESR 24? Thank you.
(In reply to Matt Wobensmith from comment #30)
> Cornel, can you verify ESR 24? Thank you.

Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0

Verified on ESR 24 (build ID: 20131021230807) and it not reproduces if the menu bar is not summoned (tracked in bug 924241). 
Marking this issue as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.