Closed Bug 1268096 Opened 4 years ago Closed 4 years ago

WebGL point sprites malfunctions in Firefox 46 on Windows D3D11 ANGLE

Categories

(Core :: Canvas: WebGL, defect)

46 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox46 + wontfix
firefox47 + fixed
firefox48 + fixed
firefox49 + fixed

People

(Reporter: postfilter, Assigned: jgilbert)

References

Details

(Keywords: dev-doc-complete, regression, site-compat, Whiteboard: gfx-noted, needs-test)

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160421124000

Steps to reproduce:

Visit these pages:

http://alteredqualia.com/xg/examples/skylon.html
http://alteredqualia.com/xg/examples/deferred_particles_nebula.html


Actual results:

There are missing particle effects: stream of particles coming out of spacecraft, also particles nebula in second demo.


Expected results:

There should be particles.

---------

This is likely due to ANGLE regress. 

The same thing happened in Chrome few months ago (January-February 2016) - broken gl.POINTS primitives on Windows due to ANGLE changes. 

Chrome already got it fixed, but seems Firefox still uses older ANGLE version that still had this regress (both stable Firefox 46 and the most recent Firefox Nightly 49 have broken particles).

If this is indeed the case, fix should be relatively simple - use a newer ANGLE version.

See this Chrome issue for more details: https://bugs.chromium.org/p/chromium/issues/detail?id=586531
[Tracking Requested - why for this release]: A site-compat regression from Firefox 46, initially reported on Twitter: https://twitter.com/alteredq/status/725076739911737345
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jmuizelaar)
Tracking for 46, 47, 48 and 49.
Jeff, how bad is this for users? If we do a dot release in the next week, and you have a fix and it is testable and not too risky feeling, it could still get into 46.
Flags: needinfo?(jgilbert)
Approval Request Comment
[Feature/regressing bug #]: Bug 1232902
[User impact if declined]: Some WebGL content will be broken.
[Describe test coverage new/current, TreeHerder]: Tested by upstream and landed in Chrome.
[Risks and why]: Should be pretty low.
Flags: needinfo?(jmuizelaar)
Attachment #8746617 - Flags: approval-mozilla-release?
Attachment #8746617 - Flags: approval-mozilla-beta?
Attachment #8746617 - Flags: approval-mozilla-aurora?
Thanks, jrmuizel.
Assignee: nobody → jmuizelaar
Flags: needinfo?(jgilbert)
Keywords: dev-doc-needed
Keywords: leave-open
Duplicate of this bug: 1268001
I'm leaning towards letting this ride with 47 so we have more time for testing or finding any resulting issues. (rather than shipping as a ridealong with 46.0.1)
Attachment #8746617 - Flags: approval-mozilla-release? → approval-mozilla-release-
Hello AlteredQualia, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(postfilter)
Comment on attachment 8746617 [details] [diff] [review]
Port upstream change for point sprites.

Recent regression, Aurora48+, Beta47+
Attachment #8746617 - Flags: approval-mozilla-beta?
Attachment #8746617 - Flags: approval-mozilla-beta+
Attachment #8746617 - Flags: approval-mozilla-aurora?
Attachment #8746617 - Flags: approval-mozilla-aurora+
(In reply to Ritu Kothari (:ritu) from comment #12)
> Hello AlteredQualia, could you please verify this issue is fixed as expected
> on a latest Nightly build? Thanks!

Yup, it now works ok here with the latest Nightly 49.0a1 (2016-05-02).

Thank you.
Flags: needinfo?(postfilter)
Wes - I want to mark this as fixed in 49, but just checking to make sure it landed. 

The bug is also tagged leave-open - jgilbert do you still want to leave it open?
Flags: needinfo?(wkocher)
Flags: needinfo?(jgilbert)
It only didn't get marked resolved for 49 because of the leave-open. If that goes away, this can be resolved. So I'll defer to jgilbert. :)
Flags: needinfo?(wkocher)
Flags: needinfo?(jgilbert)
Keywords: leave-open
Is there a workaround?
It breaks the markers on our map, it gives a really bad experience for Firefox users on our website, e.g.

https://www.bienici.com/recherche/achat/france?camera=18_-1.5113552_48.6358157

There should be POIs on the map, none is displayed.
Please use Google Chrome.
(In reply to jazzzz from comment #19)
> Is there a workaround?
> It breaks the markers on our map, it gives a really bad experience for
> Firefox users on our website, e.g.
> 
> https://www.bienici.com/recherche/achat/france?camera=18_-1.5113552_48.
> 6358157
> 
> There should be POIs on the map, none is displayed.

Draw your own small quads.
FWIW, the way you use point-sprites is technically non-portable, since max point size can be 1.0.

Also, you should always test on Nightly/Dev Edition/Beta before the version becomes Release. Once a bug hits Release, it's generally going to be there for six weeks unless it's a major widespread problem, as stability of our Release channel is paramount.
FWIW, under the hood, ANGLE is converting your point sprites to quads, so doing it yourself will generally be more efficient.

Also, if you use a library, file a bug with that library.
I'll add a regression test for this.
Flags: needinfo?(jgilbert)
Whiteboard: gfx-noted, needs-test
(In reply to Jeff Gilbert [:jgilbert] from comment #22)
> FWIW, under the hood, ANGLE is converting your point sprites to quads, so
> doing it yourself will generally be more efficient.
> 
> Also, if you use a library, file a bug with that library.

Thanks, we'll convert our point sprites to quads.
Add a regression test.
Flags: needinfo?(jgilbert)
Attachment #8754123 - Flags: review?(jmuizelaar)
I'll take this to drive through the test.
Assignee: jmuizelaar → jgilbert
Attachment #8754123 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8754123 [details] [diff] [review]
0001-Bug-1268096-r-jrmuizel-Add-regression-test.patch

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: lack of regression test
[Describe test coverage new/current, TreeHerder]: this is the test
[Risks and why]: none
[String/UUID change made/needed]: none
Attachment #8754123 - Flags: approval-mozilla-beta?
Attachment #8754123 - Flags: approval-mozilla-aurora?
Comment on attachment 8754123 [details] [diff] [review]
0001-Bug-1268096-r-jrmuizel-Add-regression-test.patch

Test checkins do not need review by release management team. They are auto-approved.
Attachment #8754123 - Flags: approval-mozilla-beta?
Attachment #8754123 - Flags: approval-mozilla-aurora?
Should be simple, but to keep things clean, here's a try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=507ceac0e399

(In reply to Ritu Kothari (:ritu) from comment #29)
> Comment on attachment 8754123 [details] [diff] [review]
> 0001-Bug-1268096-r-jrmuizel-Add-regression-test.patch
> 
> Test checkins do not need review by release management team. They are
> auto-approved.

Awesome, great policy.
Flags: needinfo?(jgilbert)
Lost track of this?
Flags: needinfo?(jgilbert)
(In reply to Milan Sreckovic [:milan] from comment #32)
> Lost track of this?

I've been lax about commenting. Adding the test creates a perma-orange due to leaking the WebGLContextLossHandler object. (allegedly)
Landing the test is blocked on fixing this.
Flags: needinfo?(jgilbert)
Depends on: 1280507
Blocks: 1280508
Comment on attachment 8754123 [details] [diff] [review]
0001-Bug-1268096-r-jrmuizel-Add-regression-test.patch

Moved to bug 1280508.
Attachment #8754123 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
No longer depends on: 1280507
You need to log in before you can comment on or make changes to this bug.