[Webkit-unassigned] [Bug 68353] [Qt] Apply ParalellJobs for ImageBuffer::platformTransformColorSpace method

[bugzilla-daemon at webkit.org]

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


Balazs Kelemen <kbalazs at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #107994|review-                     |review?
               Flag|                            |




--- Comment #6 from Balazs Kelemen <kbalazs at webkit.org>  2011-09-20 06:48:44 PST ---
(From update of attachment 107994)
View in context: https://bugs.webkit.org/attachment.cgi?id=107994&action=review

Informal review - just some style nit, I didn't check it algoritmically.

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:170
> +static const int minimalAreaOfImage = 128 * 128; // Empirical data limit for parallel jobs
> +

This seems to be a confusing name. minimalAreaForWorkerThread?

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:180
> +static void worker(ThreadParam* tp)
> +{

1. worker does not say too much. Please chose a name for the function that suggests it's task.
2. tp is not a valid name, please use non-abbreviated variable names.

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:209
> +    if (1 != optimalThreadCount) {

It's not common in WebKit to write the constant first, please transpose the expression.

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