[webkit-reviews] review denied: [Bug 19991] WebKit needs cross-platform filter system : [Attachment 30842] subRegion Code + feFlood

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 1 14:54:56 PDT 2009


Eric Seidel <eric at webkit.org> has denied Dirk Schulze <krit at webkit.org>'s
request for review:
Bug 19991: WebKit needs cross-platform filter system
https://bugs.webkit.org/show_bug.cgi?id=19991

Attachment 30842: subRegion Code + feFlood
https://bugs.webkit.org/attachment.cgi?id=30842&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I think it's definitely time to move to a new bug.  Normally we try to do one
patch per bug. ;)

Why explicit constructor call?
 78	Color color = Color(colorWithOverrideAlpha(floodColor().rgb(),
floodOpacity()));


It seems like buffer creation of the proper size is a common thing that filters
will want to share code for.

Why are you translating before filling?  Why not just fill
FloatRect(FloatSize(), subRegion.size()) instead?

You should explain the
 68	if (!idIsEmpty && m_builtinEffects.contains(idImpl))

change in your ChangeLog.

I assume this is covered by existing tests (which aren't currently being run). 
When can we turn on filters again? ;)


More information about the webkit-reviews mailing list