[webkit-reviews] review denied: [Bug 5748] KCanvas needs to be redesigned to fill & stroke at once : [Attachment 5555] take the time to cleanup the code while refactoring on suggestion from macdome

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sun Jan 8 14:48:01 PST 2006


Eric Seidel <macdome at opendarwin.org> has denied Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 5748: KCanvas needs to be redesigned to fill & stroke at once
http://bugzilla.opendarwin.org/show_bug.cgi?id=5748

Attachment 5555: take the time to cleanup the code while refactoring on
suggestion from macdome
http://bugzilla.opendarwin.org/attachment.cgi?id=5555&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
Again, looks great.  A couple comments:

No need to check canvasStyle() or path() first, just have:
KRenderingPaintServer *fillPaintServer =
canvasStyle()->fillPaintServer(canvasStyle()->renderStyle(), this);
and then check if non-null, if non-null, setActiveClient(this) and draw()

overrideFillPaintServer overrideStrokePaintServer shoudl assume type IMAGE, and
shoudl ASSERT that the passed in paint server is of that type.	Also, I woudl
encourage you to land a // FIXME: right next to those functions describing how
they were going away.

Otherwise this looks great.

You had mentioned there was a laytou test regression wrt Images.  We'll wait to
land until that's fixed.



More information about the webkit-reviews mailing list