[webkit-reviews] review denied: [Bug 28268] adjustLineToPixelBoundaries used in platform/GraphicsContext drawLine needs refactoring finished. : [Attachment 34761] Patch to move adjustLineToPixelBoundaries to GraphicsContext.cpp and cleanup existing uses of function.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 13 11:42:49 PDT 2009


Eric Seidel <eric at webkit.org> has denied Mike Fenton
<mike.fenton at torchmobile.com>'s request for review:
Bug 28268: adjustLineToPixelBoundaries used in platform/GraphicsContext
drawLine needs refactoring finished.
https://bugs.webkit.org/show_bug.cgi?id=28268

Attachment 34761: Patch to move adjustLineToPixelBoundaries to
GraphicsContext.cpp and cleanup existing uses of function.
https://bugs.webkit.org/attachment.cgi?id=34761&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
+	 static void adjustLineToPixelBoundaries(FloatPoint& p1, FloatPoint&
p2, float strokeWidth, const StrokeStyle& penStyle);

"penStyle" name is not needed.

KHTML has little to do with WebKit anymore:
+    // works out.  For example, with a border width of 3, KHTML will pass us
(y1+y2)/2, e.g.,
That should be "WebKIt"

If you were a committer, I would have you fix those tiny nits when you landed,
but since you aren't (yet), please post a revised patch.

It's not clear when this function should be used.  I suspect that CoreGraphics
takes care of all this automatically internally?  Is this function used to work
around missing features in graphics libraries, or is this something that all
ports should be using all the time?

Please add some comments to the function declaration to help others understand
when (if ever) this should be used.  If it's only needed by Qt and CAiro,
consider wrapping it in an #ifdef

Otherwise looks fine.

Thanks!


More information about the webkit-reviews mailing list