[Webkit-unassigned] [Bug 49907] Better result passing in filter primitives

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Dirk Schulze <krit at webkit.org>  2010-11-29 15:01:16 PST ---
(From update of attachment 74995)
View in context: https://bugs.webkit.org/attachment.cgi?id=74995&action=review

Great patch. Just some minor notes. Could you write a manual perf test which shows the affect of your changes please? You can place the test in WebCore/manual-test. If it could measure the timings it would be a great tool for performance patches in the future, but it doesn't need it. Beth wrote a perf test for SVG clipping. It is placed there too. Maybe you can take a look at it.

> WebCore/platform/graphics/filters/FilterEffect.cpp:25
> +

IIRC we don't add newlines here.

>> WebCore/platform/graphics/filters/FilterEffect.cpp:106
>> +inline void FilterEffect::copyImageBytes(ImageData* src, ImageData* dst, const IntRect& rect)
> 
> Avoid abbrevations, while it is convienent here, I'd prefer source and destination.

I guess you're following the style in ImageBuffer*.cpp, but maybe Niko is right here.

> WebCore/platform/graphics/filters/FilterEffect.cpp:110
> +    if (rect.x() < 0 || rect.y() < 0 || (rect.x() + rect.width()) > m_absolutePaintRect.width() || (rect.y() + rect.height()) > m_absolutePaintRect.height())

(rect.x() + rect.width())
See Nikos comment about right(), the same for the vertical position, just bottom()

>> WebCore/platform/graphics/filters/FilterEffect.cpp:159
>> +            unsigned char* end = src + (m_absolutePaintRect.width() * m_absolutePaintRect.height() * 4);
> 
> Again, src -> source, dst -> destination etc.

Would mean to write copyImageBytes(m_unmultipliedImageResult.get(), dst, rect); twice. Bu maybe it's still better to read. So...

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