[webkit-reviews] review denied: [Bug 38683] SVG FilterEffects need more detailed DRT information : [Attachment 55294] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Dirk Schulze <krit at webkit.org>'s
request for review:
Bug 38683: SVG FilterEffects need more detailed DRT information
https://bugs.webkit.org/show_bug.cgi?id=38683

Attachment 55294: Patch
https://bugs.webkit.org/attachment.cgi?id=55294&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
WebCore/platform/graphics/filters/FEBlend.cpp:163
 +	case FEBLEND_MODE_DARKEN:
I prefer to write this sort of code in terms of helper functions which convert
from the ENUM to a char*.  That would simplify this code slightly and make it
easier to re-use.

WebCore/platform/graphics/filters/FEColorMatrix.cpp:202
 +	    ts << "UNKNOWN";
Again here. :)

WebCore/platform/graphics/filters/FEColorMatrix.cpp:226
 +	if (!m_values.isEmpty()) {
I would have also made this a separate function... but I'm all crazy about
using lots of little inlines. :)

WebCore/platform/graphics/filters/FEColorMatrix.h:54
 +	    TextStream& externalRepresentation(TextStream& ts, int indent)
const;
no "ts" needed.

WebCore/platform/graphics/filters/FEComponentTransfer.cpp:220
 +	ts << "{red:   type=\"" << m_redFunc.type << "\" slope=\""
Should use a helper function. :)


only looked through the first little bit, but there is more cleanup needed
here.  The copy/paste with the m_redFunc stuff is r-. :(