Bug 103398 - OpenCL version of FEColorMatrix
Summary: OpenCL version of FEColorMatrix
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 70099
Blocks:
  Show dependency tree / graph
 
Reported: 2012-11-27 04:41 PST by Tamas Czene
Modified: 2012-12-13 07:07 PST (History)
4 users (show)

See Also:

Attachments
draft patch (11.90 KB, patch)
2012-11-27 23:42 PST, Tamas Czene 		no flags Details | Formatted Diff | Diff
proposed patch (15.47 KB, patch)
2012-12-11 08:03 PST, Tamas Czene 		zherczeg: review-
zherczeg: commit-queue-
Details | Formatted Diff | Diff
svg file (1.37 KB, image/svg+xml)
2012-12-11 23:57 PST, Tamas Czene 		no flags Details
proposed patch (15.45 KB, patch)
2012-12-13 04:22 PST, Tamas Czene 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tamas Czene 2012-11-27 04:41:12 PST 
FEColorMatrix filter need to be implemented on OpenCL.
Comment 1 Tamas Czene 2012-11-27 23:42:58 PST 
Created attachment 176413 [details]
draft patch
Comment 2 Tamas Czene 2012-12-11 08:03:38 PST 
Created attachment 178807 [details]
proposed patch

Benchmark results:

Qt Linux:
GPU: NVIDIA GeForce GT 440
CUDA Cores 96

CPU: Intel(R) Core(TM) i5-2320 CPU @ 3.00GHz
4 Core

Software: 3 fps
OpenCL GPU: 15 fps
Comment 3 Zoltan Herczeg 2012-12-11 09:51:26 PST 
Could you attach the source code of the benchmark as well?
Comment 4 Tamas Czene 2012-12-11 23:57:26 PST 
Created attachment 178983 [details]
svg file
Comment 5 Zoltan Herczeg 2012-12-13 00:53:16 PST 
Comment on attachment 178807 [details]
proposed patch

The patch looks good, only a few changes:

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

> Source/WebCore/ChangeLog:4
> +        OpenCL version of FEColorMatrix.
> +        https://bugs.webkit.org/show_bug.cgi?id=103398

Where is the "revewed by" line?

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:52
> +    values[5]  * sourcePixel.x + values[6]  * sourcePixel.y + values[7]  * sourcePixel.z + values[8]  * sourcePixel.w + values[9],
> +    values[10] * sourcePixel.x + values[11] * sourcePixel.y + values[12] * sourcePixel.z + values[13] * sourcePixel.w + values[14],
> +    values[15] * sourcePixel.x + values[16] * sourcePixel.y + values[17] * sourcePixel.z + values[18] * sourcePixel.w + values[19])

+4 space indentation

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:60
> +    sourcePixel.x * components[3] + sourcePixel.y * components[4] + sourcePixel.z * components[5],  
> +    sourcePixel.x * components[6] + sourcePixel.y * components[7] + sourcePixel.z * components[8],  
> +    sourcePixel.w)

ditto.

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:112
> +        colorMatrix = 0;

return. 0 is an invalid value. And an ASSERT_NOT_REACHED()

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:141
> +    absolutePaintRect().x() - in->absolutePaintRect().location().x(),
> +    absolutePaintRect().y() - in->absolutePaintRect().location().y());

Indentation again.
Comment 6 Tamas Czene 2012-12-13 04:22:30 PST 
Created attachment 179248 [details]
proposed patch
Comment 7 Zoltan Herczeg 2012-12-13 06:47:03 PST 
Comment on attachment 179248 [details]
proposed patch

r=me nice patch.
Comment 8 WebKit Review Bot 2012-12-13 07:07:43 PST 
Comment on attachment 179248 [details]
proposed patch

Clearing flags on attachment: 179248

Committed r137591: <http://trac.webkit.org/changeset/137591>
Comment 9 WebKit Review Bot 2012-12-13 07:07:47 PST 
All reviewed patches have been landed.  Closing bug.