[Webkit-unassigned] [Bug 107444] Add error checking into OpenCL version of SVG filters.

[bugzilla-daemon at webkit.org]

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


Zoltan Herczeg <zherczeg at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #186615|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #10 from Zoltan Herczeg <zherczeg at webkit.org>  2013-02-06 03:29:43 PST ---
(From update of attachment 186615)
I think we are getting closer!

View in context: https://bugs.webkit.org/attachment.cgi?id=186615&action=review

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:489
> +        &clImageFormat, m_absolutePaintRect.width(), m_absolutePaintRect.height(), 0, source, &errorCode);
> +
> +    if (errorCode) {

This newline is unnecessary.

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.cpp:163
> +    if (inError() || m_transformColorSpaceCompileStatus != openclNotCompiledYet)
> +        return;

This transformColorSpaceCompileStatus should be a boolean. Name: transformColorSpaceWasCompiled. And the value should be set to true right after this check. The inError() can be checked after after that (this is faster since only one condition needs to be tested in most cases). Or at least swap the two sides of the || operation.

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.cpp:177
> +    m_transformColorSpaceCompileStatus = openclCompileSuccessful;

Setting this value case is too late here. See above.

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.h:141
> +                if (m_error)
> +                    return 0;
>                  return handle;

I think this could be shortened to return !m_error ? handle : 0;

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:78
>      m_colorMatrixProgram = compileProgram(colorMatrixKernelProgram);
> -    if (!m_colorMatrixProgram)
> -        return false;
> +    if (!m_colorMatrixProgram)  {
> +        setInError();
> +        return;
> +    }

There are many such tests in the code. They could be shortened as:
if (!(m_colorMatrixProgram = compileProgram(colorMatrixKernelProgram))) {
    setInError();
    return;
}

Or perhaps I have an even better idea. The setInError() calls could be replaced by an inline function, called checkError

bool isFailed(bool value) {
    if (!value)
        setInError();
}

if (isFailed((m_colorMatrixProgram = compileProgram(colorMatrixKernelProgram))))
    return;

We might add various isFailed methods if gcc expect them.

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