Last Comment Bug 750527 - NV GL driver messes up when relinking the currently-bound program
: NV GL driver messes up when relinking the currently-bound program
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Jeff Gilbert [:jgilbert]
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-30 15:14 PDT by Jeff Gilbert [:jgilbert]
Modified: 2012-10-23 10:39 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.03 KB, patch)
2012-04-30 15:16 PDT, Jeff Gilbert [:jgilbert]
jgilbert: review-
Details | Diff | Splinter Review
Patch (1.67 KB, patch)
2012-05-01 16:39 PDT, Jeff Gilbert [:jgilbert]
jacob.benoit.1: review-
Details | Diff | Splinter Review
patch (1.02 KB, patch)
2012-05-07 14:31 PDT, Jeff Gilbert [:jgilbert]
jacob.benoit.1: review+
Details | Diff | Splinter Review

Description Jeff Gilbert [:jgilbert] 2012-04-30 15:14:45 PDT
This is the conformance failure we're seeing on:
uniforms/uniform-location.html

This is fixed simply by rebinding the program after relink if it is the current program.
Comment 1 Jeff Gilbert [:jgilbert] 2012-04-30 15:16:29 PDT
Created attachment 619714 [details] [diff] [review]
Patch
Comment 2 Jeff Gilbert [:jgilbert] 2012-04-30 15:21:02 PDT
https://tbpl.mozilla.org/?tree=Try&rev=dedf969dcf53
Comment 3 Jeff Gilbert [:jgilbert] 2012-04-30 15:46:47 PDT
https://tbpl.mozilla.org/?tree=Try&rev=d8dc91c6c0ea
Comment 4 Jeff Gilbert [:jgilbert] 2012-04-30 16:44:14 PDT
Comment on attachment 619714 [details] [diff] [review]
Patch

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

Limit this to nVidia HW and annotate this workaround with this bug number, similar to the existing:
// bug 744888
        if (WorkAroundDriverBugs() &&
            !data &&
            Vendor() == VendorNVIDIA)
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2012-05-01 12:15:42 PDT
Comment on attachment 619714 [details] [diff] [review]
Patch

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

Waiting for updated patch then. OK in principle.
Comment 6 Jeff Gilbert [:jgilbert] 2012-05-01 16:39:17 PDT
Created attachment 620134 [details] [diff] [review]
Patch

Fixed up with bug comment and limited to NV hardware.
I also added a function fGetUIntegerv, which replaces this common meme:
GLuint name = 0;
fGetIntegerv(LOCAL_GL_PARAM, (GLint*)&name);

With:
GLuint name = 0;
fGetUIntegerv(LOCAL_GL_PARAM, &name);
Comment 7 Jeff Gilbert [:jgilbert] 2012-05-01 16:40:54 PDT
https://tbpl.mozilla.org/?tree=Try&rev=6b430c5babba
Comment 8 Benoit Jacob [:bjacob] (mostly away) 2012-05-07 05:49:25 PDT
Comment on attachment 620134 [details] [diff] [review]
Patch

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

::: content/canvas/src/WebGLContextGL.cpp
@@ +3347,5 @@
> +            updateInfoSucceeded &&
> +            gl->Vendor() == gl::GLContext::VendorNVIDIA)
> +        {
> +            GLuint curProgram = 0;
> +            gl->fGetUIntegerv(LOCAL_GL_CURRENT_PROGRAM, &curProgram);

Wait! if all you need to know is whether this program is the current one, you don't need to query the GL for that. You can simply compare the pointers to WebGLProgram objects. Compare |program| to |mCurrentProgram|.

::: gfx/gl/GLContext.h
@@ +1890,5 @@
>      }
>  
> +    void fGetUIntegerv(GLenum pname, GLuint* param) {
> +        fGetIntegerv(pname, (GLint*)param);
> +    }

Since this function is not in the real GL API, it shouldn't have this 'f' prefix, which comes from these functions originally being the raw function pointers loaded from GL libs.

But see other comment, it shouldn't be needed.
Comment 9 Jeff Gilbert [:jgilbert] 2012-05-07 14:31:44 PDT
Created attachment 621742 [details] [diff] [review]
patch
Comment 11 Jeff Gilbert [:jgilbert] 2012-05-30 17:01:48 PDT
Previous landing was a null push.

Actual push:
https://hg.mozilla.org/integration/mozilla-inbound/rev/60ac5e1aba15
Comment 12 Ed Morley [:emorley] 2012-05-31 05:55:34 PDT
https://hg.mozilla.org/mozilla-central/rev/60ac5e1aba15
Comment 13 Kenneth Russell 2012-10-23 10:39:43 PDT
FYI, this has been filed as NVIDIA Incident 110263 and is tracked on the WebGL WG's internal wiki.

Note You need to log in before you can comment on or make changes to this bug.