[webkit-reviews] review denied: [Bug 70989] Attribute and Uniform variable names need translation in shader : [Attachment 167012] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has denied Dean Jackson <dino at apple.com>'s
request for review:
Bug 70989: Attribute and Uniform variable names need translation in shader
https://bugs.webkit.org/show_bug.cgi?id=70989

Attachment 167012: Patch
https://bugs.webkit.org/attachment.cgi?id=167012&action=review

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=167012&action=review


>> Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:59
>> +	    return symbolType == SHADER_SYMBOL_TYPE_UNIFORM
> 
> I think you want:
> symbolType == SHADER_SYMBOL_TYPE_UNIFORM && (dataType == SH_SAMPLER_2D ||
dataType == ...)

Duh! You're right :(

>> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:89

>> +	bool vertexShaderValid =
validator->compileShaderSource(originalVertexShader.utf8().data(),
SHADER_TYPE_VERTEX, m_validatedVertexShader, vertexShaderLog, symbols);
> 
> The bot failure is because we're not passing in SH_ATTRIBUTES_UNIFORMS
anymore, which causes the ANGLE symbol query to fail. I think it makes sense to
always use this flag in compileShaderSource since that method returns a vector
of symbols now. On the ANGLE side, all this flag does is walk the shader's
intermediate parse tree, collect all the attributes and uniforms in a vector,
and make them available via the ANGLE API. It doesn't affect the validation
policy in any way, so it should be fine in all usages of compileShaderSource.

Indeed. I've made this change.