1467406 - Black screen with Esenthel Engine WebGL 2.0 demo

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect, P3)

Description

[esenthel]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce:

http://www.esenthel.com/?id=live_demo


Actual results:

Screen is black


Expected results:

This WebGL 2.0 demo works fine on Google Chrome, but on FireFox I don't see anything.

Comment 1

[Gingerbread Man]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
20180606220131

Looks like this was already noted 2 years ago in bug 1109708, comment 22.

Comment 2

[esenthel]

This demo used to work fine on an older version of FireFox, so something got broken.

http://esenthel.com/site/Esenthel%20Fantasy%20Demo%20WebGL1/Esenthel%20Fantasy%20Demo%20WebGL1.html
This is same demo but with WebGL 1.0 (instead of 2.0) and it works OK on latest FireFox.

http://esenthel.com/site/Esenthel%20Fantasy%20Demo%20Test/Esenthel%20Fantasy%20Demo%20Test.html
This is WebGL 2.0 but with all calls to 'glInvalidateFramebuffer' disabled, and it works fine.

So latest FireFox must have some problem with handling 'glInvalidateFramebuffer'

Comment 3

[Gingerbread Man]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=86318931910b1842ac8ca5ded448e0d6d0984343&tochange=2ba0c8df9a6786d5a70065b03ac66ffbe05695cb

Comment 4

[esenthel]

Any news?

Comment 5

[esenthel]

Any news?

Comment 6

[Kelsey Gilbert [:jgilbert]]

I think this is over-invalidation on Firefox's part. We try to optimize invalidation of the default framebuffer, but we assume right now that that any invalidation call is for all attachments, which is probably not the case here. (Are you invalidating only depth/stencil, by chance?)

Comment 7

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1467406 - Remove broken default-fb invalidation path.

Comment 8

[esenthel]

Yes,

In my engine I call 'glInvalidateFramebuffer' with all kinds of different parameters, sometimes color render targets, sometimes depth buffer.

So yes, in FireFox you should definitely make sure to invalidate only what was requested, and not all.

Thanks

Comment 9

[Kelsey Gilbert [:jgilbert]]

I wouldn't normally try to uplift this to ESR, but it's super low-risk, and fixes this issue. We'll try for it, at least.

Comment 10

[Kelsey Gilbert [:jgilbert]]

[Tracking Requested - why for this release]:
This is extremely low-risk and fixes this correctness issue.

Comment 11

[Kelsey Gilbert [:jgilbert]]

(In reply to esenthel from comment #8)
> Yes,
> 
> In my engine I call 'glInvalidateFramebuffer' with all kinds of different
> parameters, sometimes color render targets, sometimes depth buffer.
> 
> So yes, in FireFox you should definitely make sure to invalidate only what
> was requested, and not all.
> 
> Thanks

Awesome, thanks for using the new WebGL 2 functionality! I'll make sure that we have a test for this for the future.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb787b74bd04
Remove broken default-fb invalidation path. r=lsalzman

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/eb787b74bd04

Comment 14

[Julien Cristau [:jcristau]]

Please request uplift ASAP, we're building the last 64 beta today.

Comment 15

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9027822 [details]
Bug 1467406 - Remove broken default-fb invalidation path.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: Black screens in some WebGL applications

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Very low, because we're just removing some overzealous zeroing.

String changes made/needed: none

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9027822 [details]
Bug 1467406 - Remove broken default-fb invalidation path.

Assuming an esr60 nomination was also intended here. Grafts cleanly, fwiw.

Comment 17

[Kelsey Gilbert [:jgilbert]]

Sure! For some reason I thought esr60 was unaffected.

Comment 18

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

Comment on attachment 9027822 [details]
Bug 1467406 - Remove broken default-fb invalidation path.

Let's take this for beta and see how things go. I don't think we have any pressing need to get this into ESR but we can reconsider that for next cycle if you feel strongly about it.

Comment 19

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f4fe9800063b

Comment 20

[Kelsey Gilbert [:jgilbert]]

I don't seem to be authorized to re-request esr60 consideration.

Comment 21

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

Comment on attachment 9027822 [details]
Bug 1467406 - Remove broken default-fb invalidation path.

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: 

User impact if declined: 

Fix Landed on Version: 

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): 

String or UUID changes made by this patch:

Comment 22

[Julien Cristau [:jcristau]]

Comment on attachment 9027822 [details]
Bug 1467406 - Remove broken default-fb invalidation path.

webgl fix, included in 64.0, approved for 60.5.0esr

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/a60524dd4cff