[webkit-reviews] review granted: [Bug 42405] ANGLE integration for shader validator and compiler : [Attachment 64080] Stores data about shader (log, translated source, if valid) in HashMap instead of WebGLShader object
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 11 07:40:00 PDT 2010
Chris Marrin <cmarrin at apple.com> has granted Paul Sawaya <psawaya at apple.com>'s
request for review:
Bug 42405: ANGLE integration for shader validator and compiler
https://bugs.webkit.org/show_bug.cgi?id=42405
Attachment 64080: Stores data about shader (log, translated source, if valid)
in HashMap instead of WebGLShader object
https://bugs.webkit.org/attachment.cgi?id=64080&action=review
------- Additional Comments from Chris Marrin <cmarrin at apple.com>
> @@ -998,11 +1062,12 @@ void GraphicsContext3D::shaderSource(Pla
> ASSERT(shader);
>
> ensureContext(m_contextObj);
> - const CString& cs = string.utf8();
> - const char* s = cs.data();
> -
> - int length = string.length();
> - ::glShaderSource((GLuint) shader, 1, &s, &length);
> +
> + ShaderSourceEntry entry;
> +
> + entry.source = string;
> +
> + m_shaderSourceMap.set(shader, entry);
> }
>
> void GraphicsContext3D::stencilFunc(unsigned long func, long ref, unsigned
long mask)
> @@ -1352,20 +1417,35 @@ void GraphicsContext3D::getShaderiv(Plat
> String GraphicsContext3D::getShaderInfoLog(Platform3DObject shader)
> {
> ASSERT(shader);
> ensureContext(m_contextObj);
> GLint length;
> ::glGetShaderiv((GLuint) shader, GL_INFO_LOG_LENGTH, &length);
> -
> - GLsizei size;
> - GLchar* info = (GLchar*) fastMalloc(length);
> - if (!info)
> - return "";
> -
> - ::glGetShaderInfoLog((GLuint) shader, length, &size, info);
> - String s(info);
> - fastFree(info);
> - return s;
> +
> + HashMap<Platform3DObject, ShaderSourceEntry>::iterator result =
m_shaderSourceMap.find(shader);
> +
> + if (result == m_shaderSourceMap.end())
> + return "";
> +
> + ShaderSourceEntry entry = result->second;
> +
> + if (entry.isValid) {
> + GLint length;
> + ::glGetShaderiv((GLuint) shader, GL_INFO_LOG_LENGTH, &length);
Do you need to get INFO_LOG_LENGTH twice?
> +
> + GLsizei size;
> + GLchar* info = (GLchar*) fastMalloc(length);
> + if (!info)
> + return "";
> +
> + ::glGetShaderInfoLog((GLuint) shader, length, &size, info);
> + String s(info);
> + fastFree(info);
> + return s;
> + }
> + else {
> + return entry.log;
> + }
> }
>
> String GraphicsContext3D::getShaderSource(Platform3DObject shader)
WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:1447
+ }
The above lines are indented 5, they should be indented 4.
r+ with the above fixes
Also, this really needs testing. I really want this checked in, so existing
tests will do for now. Please submit a new bug for tests. You should add tests
that show the appropriate message return for a failed validation.
More information about the webkit-reviews
mailing list