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 treegraph
 
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

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.