[webkit-reviews] review denied: [Bug 23882] GraphicsContextSkia draws round rects as solid rects : [Attachment 27563] Fix for 23882

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 11 10:37:16 PST 2009


Eric Seidel <eric at webkit.org> has denied Scott Violet <sky at google.com>'s
request for review:
Bug 23882: GraphicsContextSkia draws round rects as solid rects
https://bugs.webkit.org/show_bug.cgi?id=23882

Attachment 27563: Fix for 23882
https://bugs.webkit.org/attachment.cgi?id=27563&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
This block still doesn't make sense to me, even after reading the bug:

+    if (topLeft.width() + topRight.width() > rect.width() ||
+	 bottomLeft.width() + bottomRight.width() > rect.width() ||
+	 topLeft.height() + bottomLeft.height() > rect.height() ||
+	 topRight.height() + bottomRight.height() > rect.height()) {
+	 // If the width/height along an axis is > than the width/height of the

+	 // rectangle, just draw a rect.
+	 fillRect(rect, color);
+	 return;
+    }

i.e. What behavior does that change?  Why are we changing it (to match CG's
behavior it seems).  What is "width/height long an axis"
Seems we need a better comment there to make more sense to the casual reader
(who knows less about our graphics code than even I do).

Otherwise this change looks great.


More information about the webkit-reviews mailing list