602183 - Crash with point sprites on certain old ATI drivers

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Alice0775 White]

Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20101005 Firefox/4.0b7pre ID:20101005041637

Browser crashes with crash report when open http://web.chemdoodle.com/overview3D.php .

D2D, D3D9 on/off does not matter.

Graphics
Adapter Description : ATI Radeon HD 4300/4500 Series
Vendor ID : 1002
Device ID : 954f
Adapter RAM : 512
Adapter Drivers : aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64
Driver Version : 8.771.0.0
Driver Date : 8-25-2010
Direct2D Enabled : true
DirectWrite Enabled : true
GPU Accelerated Windows : 2/2 Direct3D 9

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open URL ( http://web.chemdoodle.com/overview3D.php )
3. Wait loading the page

Actual Results:
 Browser crashes with crash report.
 bp-7d742ddf-fe1e-43c5-9f78-1d02e2101006
 bp-2dd1e215-2a47-46a9-989a-3e35d2101006

Expected Results:
 Should not crash

I tried to find regression window in cached m-c hourly.
(with webgl.enabled_for_all_sites = true ):

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/0fa5626cec95
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100927 Firefox/4.0b7pre ID:20100927141643
Fails
http://hg.mozilla.org/mozilla-central/rev/9829aad9a09f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100927 Firefox/4.0b7pre ID:20100927144359
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0fa5626cec95&tochange=9829aad9a09f

Comment 1

[Alice0775 White]

In local build, I revert to changeset 2dc6383d3c6e, then the crash does not happen.
So, the following changeset causes the crash.
9829aad9a09f	Benoit Jacob — Bug 596034 - make sure gl_PointCoord is available in the shader language - r=vladimir, a=blocking-beta7

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Thanks for finding this!

So this is clearly a ATI driver bug: doing

    glEnable(GL_POINT_SPRITE);

causes the ATI driver to be crashy.

When you revert to before this was added, does this demo work for you (do you see the particles?)
    http://learningwebgl.com/lessons/example03/particles-01.html

Comment 3

[Alice0775 White]

(In reply to comment #2)
> Thanks for finding this!
> 
> So this is clearly a ATI driver bug: doing
> 
>     glEnable(GL_POINT_SPRITE);
> 
> causes the ATI driver to be crashy.
> 
> When you revert to before this was added, does this demo work for you (do you
> see the particles?)
>     http://learningwebgl.com/lessons/example03/particles-01.html

Today's m-c nightly dee1e01fd8ed: Fails, Only appears black rectangle
Build from changeset 9829aad9a09f :  Works, Fireworks exploded in the night sky
Build from changeset 2dc6383d3c6e :  Works, Fireworks exploded in the night sky

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Does the build from changeset 9829aad9a09f crash as originally reported in this bug?

Comment 5

[Alice0775 White]

> Today's m-c nightly dee1e01fd8ed: Fails, Only appears black rectangle
> Build from changeset 9829aad9a09f :  Works, Fireworks exploded in the night sky
> Build from changeset 2dc6383d3c6e :  Works, Fireworks exploded in the night sky

Sorry ,My mistake.

Today's m-c nightly dee1e01fd8ed:    Works, Fireworks exploded in the night sky
Build from changeset 9829aad9a09f :  Works, Fireworks exploded in the night sky
Build from changeset 2dc6383d3c6e :  Works, Fireworks exploded in the night sky

Comment 6

[Alice0775 White]

(In reply to comment #4)
> Does the build from changeset 9829aad9a09f crash as originally reported in this
> bug?

In my local build, Build from changeset 9829aad9a09f crashes when open ( http://web.chemdoodle.com/overview3D.php ).

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

OK, thanks, so now we know two things about changeset 9829aad9a09f (bug 596034):

  1.  On your system (ATI / Windows), it's not needed.
  2.  On your system (ATI / Windows), it triggers driver crashes.

So clearly, we should do 2 things:
  a. report to ATI (comment 2).
  b. until the driver is fixed, not do this on ATI.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Don't enable GL_POINT_SPRITE on ATI / Windows

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

(Note, this patch relies on new API introduced in bug 602289)

Comment 10

[Joe Drew (not getting mail)]

Comment on attachment 481307 [details] [diff] [review]
Don't enable GL_POINT_SPRITE on ATI / Windows

Just do the gl->IsATI() inside an #ifdef XP_WIN instead of adding IS_WINDOWS.

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Don't enable GL_POINT_SPRITE on ATI / Windows

carrying forward r+

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/cea1b6cc7d99

Comment 14

[jacob]

Hi There,
  This patch is pretty old, and it will cause the webGL conformance test http://www.khronos.org/registry/webgl/conformance-suites/1.0.1/conformance/glsl/variables/gl-pointcoord.html failed. So please remvoe this patch.

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

Hi Jacob, thanks for your input. Could you please indicate which driver versions need the patch and which don't? Is there a way that we can easily determine that from the GL_VERSION string, or in any other way?

Comment 16

[jacob]

Sorry for late reply. I thought this forum would send me E-mail automatically when there is any update. I don't think determining it from the GL_VERSION is a good idea.
The catalyst since 12.10 does not support HD 4300/4500 Series any more. So I think we can just delete the patch. But if firefox have to consider the compatibility to the old driver, you can get the driver version by geting "OpenGL version string". For example, there is a line when runing glxinfo "OpenGL version string: 4.2.12541 Compatibility Profile Context 13.12", "13.12" at the end of the line is our driver version.
By the way, can we just patch for HD 4300/4500?

Comment 17

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to jacob from comment #16)
> Sorry for late reply. I thought this forum would send me E-mail
> automatically when there is any update.

It does for me, but that may depend on account preferences.

 I don't think determining it from
> the GL_VERSION is a good idea.
> The catalyst since 12.10 does not support HD 4300/4500 Series any more. So I
> think we can just delete the patch. But if firefox have to consider the
> compatibility to the old driver, you can get the driver version by geting
> "OpenGL version string". For example, there is a line when runing glxinfo
> "OpenGL version string: 4.2.12541 Compatibility Profile Context 13.12",
> "13.12" at the end of the line is our driver version.
> By the way, can we just patch for HD 4300/4500?

OK. Given that this may only affect a small minority of users anymore, let's unconditionally reenable point sprites and then let's watch out for related bug reports and react accordingly.

Comment 18

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: reenable point sprites on ATI

Let's see how this now fares crash-wise. It seems that all we know is that it crashed for one person 3 years ago, so time to retry now.

Comment 19

[jacob]

That's great, thanks you very much.

(In reply to Benoit Jacob [:bjacob] from comment #17)
> (In reply to jacob from comment #16)
> > Sorry for late reply. I thought this forum would send me E-mail
> > automatically when there is any update.
> 
> It does for me, but that may depend on account preferences.
> 
>  I don't think determining it from
> > the GL_VERSION is a good idea.
> > The catalyst since 12.10 does not support HD 4300/4500 Series any more. So I
> > think we can just delete the patch. But if firefox have to consider the
> > compatibility to the old driver, you can get the driver version by geting
> > "OpenGL version string". For example, there is a line when runing glxinfo
> > "OpenGL version string: 4.2.12541 Compatibility Profile Context 13.12",
> > "13.12" at the end of the line is our driver version.
> > By the way, can we just patch for HD 4300/4500?
> 
> OK. Given that this may only affect a small minority of users anymore, let's
> unconditionally reenable point sprites and then let's watch out for related
> bug reports and react accordingly.

Comment 20

[Kelsey Gilbert [:jgilbert] (returns Nov01)]

Comment on attachment 760196 [details] [diff] [review]
reenable point sprites on ATI

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

Huh, this one slipped through.

Comment 21

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/796773a10db1

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/796773a10db1