|Summary:||shaderSource needs to preserve original source|
|Product:||WebKit||Reporter:||Kenneth Russell <kbr>|
|Component:||WebGL||Assignee:||Zhenyao Mo <zmo>|
|Version:||528+ (Nightly build)|
|Bug Depends on:||52390|
Description Kenneth Russell 2011-01-20 12:45:16 PST
After the removal of comments implemented in bug 52390, shaderSource needs to preserve the original source so that it can be returned from getShaderSource.
Comment 2 Zhenyao Mo 2011-01-26 14:09:06 PST
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.
Comment 3 Kenneth Russell 2011-01-26 14:41:33 PST
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.
Comment 5 Zhenyao Mo 2011-01-26 17:31:12 PST
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?
Comment 6 Kenneth Russell 2011-01-26 17:59:09 PST
(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?
Comment 7 Kenneth Russell 2011-01-27 10:45:18 PST
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.