[Webkit-unassigned] [Bug 114639] [CSS Shaders] Extract validation logic out of CustomFilterValidatedProgram into a CustomFilterValidator class

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=114639





--- Comment #3 from Alexandru Chiculita <achicu at adobe.com>  2013-05-10 17:03:12 PST ---
(From update of attachment 200998)
View in context: https://bugs.webkit.org/attachment.cgi?id=200998&action=review

Thanks Max! I think this is a step in the right direction.

> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:45
> +    : m_programInfo(program->programInfo())

Let's remove this programInfo and just store the validated program info. See below.

> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:50
> +    m_isInitialized = CustomFilterValidator::validateShaderStrings(program, m_validatedVertexShader, m_validatedFragmentShader);

I think this might look better. This might also fix the questions about static methods & constructors.
CustomFilterValidator customFilterValidator();
if (customFilterValidator.validate(program)) {
    m_programInfo = CustomFilterProgramInfo(customFilterValidator.vertexShader(), customFilterValidator.fragmentShader(), programInfo.programType(), programInfo.mixSettings(), programInfo.meshType());
    m_initialized = true;
}

We would need a follow up patch to remove ProgramInfo and only store the required settings for the platform layer.

> Source/WebCore/platform/graphics/filters/CustomFilterValidator.cpp:104
> +static String blendFunctionString(BlendMode blendMode)

Let's add a bug to break this up in smaller functions. I would like to see these two functions (blendFunctionString and compositeFunctionString) extracted to a separate file. They seem independent from the validation part of the other class. The code might be useful in other parts in the future (ie. blending & compositing).

> Source/WebCore/platform/graphics/filters/CustomFilterValidator.cpp:593
> +        // FIXME: Report the validation errors.

Having this as an object might help do this patch in the future.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.