Bug 52833 – shaderSource needs to preserve original source

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Kenneth Russell]

After the removal of comments implemented in bug 52390, shaderSource needs to preserve the original source so that it can be returned from getShaderSource.

[Zhenyao Mo]

Created attachment 80232 [details]
Patch

[Zhenyao Mo]

Note that shader source is cached in both WebGLRenderingContext (before comments removal) and GraphicsContext3D (before ANGLE).

In this patch the cached copy in GraphicsContext3D is released once the shader is compiled.  ASSERT_NOT_REACHED() is added in this level for getShaderSource() and getShaderiv(SHADER_SOURCE_LENGTH).  This is under the assumption that WebGLRenderingContext is the only place that calls these two functions.

Will need to do the same in chromium two ports to reduce memory usage.

[Kenneth Russell]

Comment on attachment 80232 [details]
Patch

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

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1220
> +        ASSERT_NOT_REACHED();

WebGLRenderingContext isn't the only client of this code; for example, the accelerated 2D canvas code may go through it, and Chromium's accelerated compositor is built on top of GraphicsContext3D. Please restore this functionality. The required (slightly redundant) caches are negligible as shaders are tiny.

I don't think any changes are needed in the Chromium port either. We really just need the additional caching to preserve any Unicode characters in comments for WebGL content.

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1257
> +    ASSERT_NOT_REACHED();

Same here.

[Zhenyao Mo]

Created attachment 80276 [details]
Patch

[Zhenyao Mo]

One question: how do we count the length if shader source is not all UTF8?

String::length() count a UTF16 char as 1.  However, do we want to count it as 2?

[Kenneth Russell]

(In reply to comment #5)
> One question: how do we count the length if shader source is not all UTF8?
> 
> String::length() count a UTF16 char as 1.  However, do we want to count it as 2?

That's a good question. I think the queries of the shader source length and info log length are useless in this API, since the queries of the logs return DOMStrings. Do you want to raise that on the working group email list?

[Kenneth Russell]

Comment on attachment 80276 [details]
Patch

The code changes look fine. Thanks for raising the issue of removing the useless query of shader source length on the WebGL working group list.

[Zhenyao Mo]

Committed r76814: <http://trac.webkit.org/changeset/76814>