Summary: | shaderSource needs to preserve original source | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kenneth Russell <kbr> | ||||||
Component: | WebGL | Assignee: | Zhenyao Mo <zmo> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cmarrin, zmo | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | 52390 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Kenneth Russell
2011-01-20 12:45:16 PST
Created attachment 80232 [details]
Patch
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 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. Created attachment 80276 [details]
Patch
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? (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 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.
Committed r76814: <http://trac.webkit.org/changeset/76814> |