Bug 137689

Summary:

gl.detachShader breaks shader program

Product:

WebKit

Reporter:

huscar

Component:

WebGL

Assignee:

Dean Jackson <dino>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, dino, graouts, jonlee, kondapallykalyan, noam, rleider, roger_fong, sam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

OS X 10.9

See Also:

https://bugs.webkit.org/show_bug.cgi?id=98204

Attachments:

Description	Flags
Testcase	none
Testcase	none
Patch	sam: review+

Description huscar 2014-10-14 04:50:14 PDT

Calling gl.detachShader after linking results in a program that doesn't draw anything (without error messages to the console). AFAIK this should not happen and using detachShader and deleteShader is encouraged to reduce memory usage.
Here's a good test from when firefox had similar problems https://bugzilla.mozilla.org/show_bug.cgi?id=867253

Comment 1 Radar WebKit Bug Importer 2017-08-22 16:57:50 PDT

<rdar://problem/34025056>

Comment 2 Dean Jackson 2017-08-23 18:15:28 PDT

I am unable to reproduce this. See the attached test case which both detaches and deletes the shaders after linking the program, but before they are used in a draw call (and before they are used to get attribute and uniform locations).

Comment 3 Dean Jackson 2017-08-23 18:15:51 PDT

Created attachment 318952 [details]
Testcase

Comment 4 Dean Jackson 2017-08-23 18:17:38 PDT

Also, the original demo on flohofwoe doesn't seem to work any more, in any browser. It also downloads rather than running directly.

Comment 5 Dean Jackson 2017-09-07 15:20:07 PDT

Oops. My test case is bad. This is really broken.

Comment 6 Dean Jackson 2017-09-07 17:57:14 PDT

Created attachment 320217 [details]
Testcase

Better testcase. The triangle should be red.

Comment 7 Dean Jackson 2017-09-08 19:05:03 PDT

Created attachment 320329 [details]
Patch

Comment 8 Sam Weinig 2017-09-08 20:20:05 PDT

Comment on attachment 320329 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320329&action=review

> Source/WebCore/ChangeLog:20
> +        This fixes the bug, but the whole area is still a bit messy. For one,
> +        we're keeping around all the shader information even after it is
> +        no longer used.

Seems like we should have a bug tracking this issue.

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:996
> +    LinkedShaderMap::iterator result = m_linkedShaderMap.find(program);

Use auto here?

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1053
> +    LinkedShaderMap::iterator result = m_linkedShaderMap.find(program);

auto?

Comment 9 Dean Jackson 2017-09-09 15:52:52 PDT

(In reply to Sam Weinig from comment #8)
> 
> Seems like we should have a bug tracking this issue.
> 

https://bugs.webkit.org/show_bug.cgi?id=98204

Comment 10 Dean Jackson 2017-09-09 15:55:00 PDT

Committed r221831: <http://trac.webkit.org/changeset/221831>