[Webkit-unassigned] [Bug 63064] Apply the ParallelJobs support to FEMorphology

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Zoltan Herczeg <zherczeg at webkit.org>  2011-06-21 23:28:40 PST ---
(From update of attachment 97979)
View in context: https://bugs.webkit.org/attachment.cgi?id=97979&action=review

> Source/WebCore/ChangeLog:10
> +        cores if the architecture supports.

You should mention the gain somewhere (and the benchmark if it is publicly availible).

> Source/WebCore/platform/graphics/filters/FEMorphology.cpp:110
> +    const int rowByteWidth = width * 4;

Strange name for me. I liked the effectWidth better, or maybe scanlineWidth?

> Source/WebCore/platform/graphics/filters/FEMorphology.cpp:115
> +        yEnd = height;

I would not prefer these "ifs" here, why not simply pass these values as arguments?

> Source/WebCore/platform/graphics/filters/FEMorphology.cpp:119
> +        int extStartY = max(0, y - radiusY);

We don't use abbreviations is WebKit, keep the original name or make it extremaStartY

> Source/WebCore/platform/graphics/filters/FEMorphology.cpp:120
> +        int extEndY = min(height - 1, y + radiusY);

Ditto.

> Source/WebCore/platform/graphics/filters/FEMorphology.h:61
> +    };

It seems to me that radiusX and radiusY are constants, so they should go here, and not passed as arguments to platformApply* .

> Source/WebCore/platform/graphics/filters/FEMorphology.h:64
> +        static const int s_minimalArea = 300 * 300;

We should mention here that this is empirical data.

> Source/WebCore/platform/graphics/filters/FEMorphology.h:78
> +        void platformApplyGeneric(PaintingData*, const int radiusX, const int radiusY, const int yStart=-1, const int yEnd=-1);

The above two should be inline function.

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