[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





--- Comment #14 from Andreas Kling <kling at webkit.org>  2011-10-05 00:49:39 PST ---
(From update of attachment 109750)
View in context: https://bugs.webkit.org/attachment.cgi?id=109750&action=review

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:208
> +    const int optimalThreadCount = min(m_size.height(), areaOfImage / areaLimitForParallelJobs);

Could you explain this to me?
Why is this better than deriving the optimal thread count from the number of scanlines total?

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:213
> +

Unnecessary newline.

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:216
> +            for (int i = 0; i < parallelJobs.numberOfJobs(); i++) {

numberOfJobs() returns a size_t, consequently 'i' should be a size_t.
Also, WebKit style prefers prefix increment (++i) to postfix (i++.)

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:239
> +    ThreadParameters parameters;
> +    parameters.yStart = 0;
> +    parameters.yEnd = m_size.height();
> +    parameters.width = m_size.width();
> +    parameters.bits = bits;
> +    parameters.bytesPerLine = bytesPerLine;
> +    parameters.lookUpTable = &lookUpTable;
> +    transformColorWorker(&parameters);

I like that you're sharing the code between the threaded and non-threaded paths.
Could we refactor it slightly to look less thread-specific?
For example having a transformColorSpace(yStart, yEnd, width, bits, bytesPerLine, lookupTable) method instead of passing a ThreadParameters object.
Then the non-threading code doesn't need to know about ThreadParameters.

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