[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