[webkit-reviews] review denied: [Bug 9197] CSS3: Borders with border-radius and double, groove, or ridge styles should look better : [Attachment 32006] Re-written border code using clipping patches

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 4 19:33:56 PDT 2009


Eric Seidel <eric at webkit.org> has denied Alex Taylor <darwin at milliamp.org>'s
request for review:
Bug 9197: CSS3: Borders with border-radius and double, groove, or ridge styles
should look better
https://bugs.webkit.org/show_bug.cgi?id=9197

Attachment 32006: Re-written border code using clipping patches
https://bugs.webkit.org/attachment.cgi?id=32006&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Wow.  We can come up with some way to abstract some of this math.  This is way
too much copy/paste.

Huge sections of:
topLeftInnerRadius.setWidth(max(0, topLeftInnerRadius.width() -
style->borderLeftWidth()));
 825	     topLeftInnerRadius.setHeight(max(0, topLeftInnerRadius.height() -
style->borderTopWidth()));


This huge section of checks is repeated at least twice too:
	     && (bottomLeft.width() == 0 || bottomLeft.width() >=
style->borderLeftWidth())
 895		 && (bottomLeft.height() == 0 || bottomLeft.height() >=
style->borderBottomWidth())
 8

Style:
 736		 int innerBorderTopWidth = style->borderTopWidth()*2/3;
 737		 int innerBorderRightWidth = style->borderRightWidth()*2/3;
 738		 int innerBorderBottomWidth = style->borderBottomWidth()*2/3;
 739		 int innerBorderLeftWidth = style->borderLeftWidth()*2/3;
 740 
Can't we use IntRects here?

Bah!  copy/paste galore:
53	       if (style->borderTopWidth() % 3 == 1)
 754		     innerBorderTopWidth += 1;
 755		 if (style->borderRightWidth() % 3 == 1)
 756		     innerBorderRightWidth += 1;
 757		 if (style->borderBottomWidth() % 3 == 1)
 758		     innerBorderBottomWidth += 1;
 759		 if (style->borderLeftWidth() % 3 == 1)
 760		     innerBorderLeftWidth += 1;

I like this change... but It needs to be about half this size.	Using some nice
abstractions. :)  Way too much copy paste here to land this.


More information about the webkit-reviews mailing list