[webkit-reviews] review requested: [Bug 68353] [Qt] Apply ParalellJobs for ImageBuffer::platformTransformColorSpace method : [Attachment 107994] [Qt] Apply ParalellJobs for ImageBuffer::platformTransformColorSpace method

[bugzilla-daemon at webkit.org]

Balazs Kelemen <kbalazs at webkit.org> has asked  for review:
Bug 68353: [Qt] Apply ParalellJobs for ImageBuffer::platformTransformColorSpace
method
https://bugs.webkit.org/show_bug.cgi?id=68353

Attachment 107994: [Qt] Apply ParalellJobs for
ImageBuffer::platformTransformColorSpace method
https://bugs.webkit.org/attachment.cgi?id=107994&action=review

------- Additional Comments from Balazs Kelemen <kbalazs at webkit.org>
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.