[webkit-reviews] review denied: [Bug 5862] feDisplacementMap filter is not implemented : [Attachment 6354] Formatting changes

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Wed Feb 8 18:49:49 PST 2006


Eric Seidel <macdome at opendarwin.org> has denied Oliver Hunt
<ojh16 at student.canterbury.ac.nz>'s request for review:
Bug 5862: feDisplacementMap filter is not implemented
http://bugzilla.opendarwin.org/show_bug.cgi?id=5862

Attachment 6354: Formatting changes
http://bugzilla.opendarwin.org/attachment.cgi?id=6354&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
This patch looks fine.

Ideally, we would want to remove the data duplication in the kcanvas objects,
by making the KCanvasFEDisplacementMap object just fetch its needed data off of
it's element() instead of storing a second copy locally.

Also, there are a couple tabs in the cikernel file which need to be fixed
before we land.

also XChannelSelector() and m_XChannelSelector should be xChannelSelector() and
m_xChannelSelector per our style guidelines.  (Or just completely removed per
my above comment)

It also doesn't look like KCanvasFEDisplacementMap has a default constructor
which sets things to sane default values (also unecessary if its data members
are removed)

getVectorForChannel could be done using an array of floats of 0's, followed by
setting one at the correct offset to 1 and returning.  Your method is also
totally fine.

Since you don't have commit bit, I'm going to mark this as r-.	If you did, it
would be find to land, making those tweaks as you landed.  This does not need
another review, but a final patch should be posted.



More information about the webkit-reviews mailing list