[Webkit-unassigned] [Bug 99829] OpenCL version of SourceAlpha, SourceGraphics and FETurbulence filter effects

[bugzilla-daemon at webkit.org]

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


Zoltan Herczeg <zherczeg at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #175437|review?                     |review+
               Flag|                            |




--- Comment #13 from Zoltan Herczeg <zherczeg at webkit.org>  2012-11-22 23:56:41 PST ---
(From update of attachment 175437)
Thanks for all comments, and thanks for the patch. r=me with a few comments:

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

> Source/WebCore/ChangeLog:9
> +        This patch contains the OpenCL implementation of SourceAlpha, SourceGraphic, FETurbulence and a simple OpenCL environment.
> +        This is only included in Qt linux build.

Please mention here the speedup. And a little more verbose about the patch.

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.cpp:38
> +    static bool initFailed = false;

This name should int alreadyInitialized

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.cpp:52
> +    size_t deviceBufferSize = -1;

Here deviceBufferSize is unsigned...

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.cpp:67
> +    if (deviceBufferSize <= 0)

And you compare it with <= 0. I think you could simply set it to 0, and change this comparison to !deviceBufferSize.

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.cpp:70
> +    devices = reinterpret_cast<cl_device_id *>(fastMalloc(deviceBufferSize));

cl_device_id* without space

> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.cpp:120
> +void FilterContextOpenCL::openCLTransformColorSpace(OpenCLHandle& source, IntRect sourceSize, ColorSpace srcColorSpace, ColorSpace dstColorSpace)

This is just a note, but perhaps you could try sometimes the performance of doing the computation in the kernel without passing an array.

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLHandle.h:30
> +    OpenCLHandle() : m_openclMemory(0) { }

openCLImage and m_openclMemory names are not consistent. Use openCL everywhere (there are some other identifiers where this renaming is needed).

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