558867 - WebGL on GLX still asks for GLX_SAMPLE_BUFFERS even if antialiasing is disabled

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Kristan Høgsberg]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100412 Minefield/3.7a5pre
Build Identifier: trunk

If anti-aliasing is disabled, the fbconfig chooser code in nsGLPbufferGLX.cpp tries to trim the multi sample attributes off the list passed to glXChooseFBCOnfig() by inserting None.  However the None token is inserted too far down the list and GLX_SAMPLE_BUFFERS is still present in the list passed to the chooser.

Patch attached.

Reproducible: Always

Comment 1

[Kristan Høgsberg]

Attachment: Insert None token at the right position

Comment 2

[Sven Arvidsson]

This patch no longer applies it seems. 

(I can still get WebGL working, but only with software rendering).

Comment 3

[Christopher Blizzard (:blizzard)]

Ben, can you help these guys?

Comment 4

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

Disclaimer: I didn't write any of that code. CC'ing Vlad and Matt, at least one of them will know about that.

It seems that this has been fixed a while ago: we certainly aren't asking for GLX_SAMPLES or GLX_SAMPLE_BUFFERS, actually MXR can't find any occurences for these strings.

So actually I'm afraid we now have the opposite bug: we're never asking for sample buffers, we don't seem to be honoring the antialiasing preference at all! That can be fixed later though: It won't prevent anyone from using WebGL.

@ Sven / comment 2: are you sure that the reason why you can't get WebGL working, is related to sample buffers? Where in the source code do you think we are requesting sample buffers?

Everyone: this stuff is getting deeply changed by the patches in bug 571831, which also make WebGL work for many people who are currently having trouble (e.g. on linux with Free drivers).

Comment 5

[Kristan Høgsberg]

(In reply to comment #4)
> Disclaimer: I didn't write any of that code. CC'ing Vlad and Matt, at least one
> of them will know about that.
> 
> It seems that this has been fixed a while ago: we certainly aren't asking for
> GLX_SAMPLES or GLX_SAMPLE_BUFFERS, actually MXR can't find any occurences for
> these strings.
> 
> So actually I'm afraid we now have the opposite bug: we're never asking for
> sample buffers, we don't seem to be honoring the antialiasing preference at
> all! That can be fixed later though: It won't prevent anyone from using WebGL.
> 
> @ Sven / comment 2: are you sure that the reason why you can't get WebGL
> working, is related to sample buffers? Where in the source code do you think we
> are requesting sample buffers?
> 
> Everyone: this stuff is getting deeply changed by the patches in bug 571831,
> which also make WebGL work for many people who are currently having trouble
> (e.g. on linux with Free drivers).

I added support for pbuffers to the open source linux drivers so that we could support webgl, and since the intel drivers don't support multisample buffers, I further had to track down this bug to get it to work.

It's great that webgl is moving to FBOs, that's definitely the better way to go.  However this patch was dead-trivial and yet just sat here for 3 months, which is a little disappointing.  Did I not assign it right some other technical error?  I want to see webgl take of, and I want to help make sure it works on Intel chipsets (I work for Intel), but if this is the average turn-around time for mozilla bugs I'll be wasting my time.

Comment 6

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

(In reply to comment #5)
> I added support for pbuffers to the open source linux drivers so that we could
> support webgl, and since the intel drivers don't support multisample buffers, I
> further had to track down this bug to get it to work.

Ah OK, well having pbuffer support in the open source linux drivers will be useful anyway!

> 
> It's great that webgl is moving to FBOs, that's definitely the better way to
> go.  However this patch was dead-trivial and yet just sat here for 3 months,
> which is a little disappointing.

Sure, I do understand. (FWIW, 3 months ago, I was not yet working @mozilla. Since I started, I've tried looking at all existing webgl bugs, but somehow I missed this one).

>  Did I not assign it right some other
> technical error?

Actually you are not the only person in this situation --- having sent a simple useful patch without getting a reply. Let's study reasons for that so it doesn't happen again. I can see 2 common patterns in all these cases:

 A. the patch was not asked to get reviewed. Asking someone to review your patch almost ensures that person looks at the patch. Typically you'd ask whoever is listed as Contributor at the top of the file, or you'd use "hg annotate". Of course, the problem is that it's not reasonable to expect everyone sending patches to know about that.

 B. the patch is not timely, e.g. it touches code that is pending a rewrite anyway. In your case, the GLX support has been rewritten shortly after you sent this patch.

My solution to A and B in WebGL has been to subscribe to get notified about all WebGL bugs and try to answer them all ASAP no matter what; but here, your bug was reported before I started work @ Mozilla. (Besides, this is GLX not WebGL, but I'd still have answered it and CC'd GLX people).

>  I want to see webgl take of, and I want to help make sure it
> works on Intel chipsets (I work for Intel), but if this is the average
> turn-around time for mozilla bugs I'll be wasting my time.

Please don't be discouraged by this bad experience. As I said just above, this won't happen again for WebGL bugs as I'm now monitoring them. Please understand that before I started, Vlad was essentially the only WebGL developer (as far as I know), and he is busy with many things.

Comment 7

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

(Just for the record, reassigned to component: Graphics as I believe this is where system OpenGL stuff belongs --- since the code is under gfx/)

Comment 8

[Sven Arvidsson]

(In reply to comment #4)
> @ Sven / comment 2: are you sure that the reason why you can't get WebGL
> working, is related to sample buffers? Where in the source code do you think we
> are requesting sample buffers?

No, I'm not sure at all :)

All I know is that it seemed to work okay a couple of months ago, with the patch, and now it seems to be using software rendering, or at least is a lot slower. Anyway, I guess this belongs in another bug report, I'll investigate further and file a new one if needed.

Comment 9

[:aceman]

Maybe 'Canvas: WebGL' is a better fitting component. Bug 571831 is there too.

Comment 10

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

As I said in Comment 4, this has been fixed long ago --- not sure anymore why I didn't mark as fixed.

We're definitely not currently asking for GLX_SAMPLE_BUFFERS.