Closed Bug 750527 Opened 9 years ago Closed 9 years ago

NV GL driver messes up when relinking the currently-bound program

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: jgilbert, Assigned: jgilbert)

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → jgilbert
Status: NEW → ASSIGNED
Attachment #619714 - Flags: review?(bjacob)
Summary: NV GL drivers messes up when relinking the currently-bound program → NV GL driver messes up when relinking the currently-bound program
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)
Attachment #619714 - Flags: review-
Comment on attachment 619714 [details] [diff] [review]
Patch

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

Waiting for updated patch then. OK in principle.
Attachment #619714 - Flags: review?(bjacob)
Attached patch Patch (obsolete) — Splinter Review
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);
Attachment #619714 - Attachment is obsolete: true
Attachment #620134 - Flags: review?(bjacob)
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.
Attachment #620134 - Flags: review?(bjacob) → review-
Attached patch patchSplinter Review
Attachment #620134 - Attachment is obsolete: true
Attachment #621742 - Flags: review?(bjacob)
Attachment #621742 - Flags: review?(bjacob) → review+
Previous landing was a null push.

Actual push:
https://hg.mozilla.org/integration/mozilla-inbound/rev/60ac5e1aba15
https://hg.mozilla.org/mozilla-central/rev/60ac5e1aba15
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
FYI, this has been filed as NVIDIA Incident 110263 and is tracked on the WebGL WG's internal wiki.
You need to log in before you can comment on or make changes to this bug.