[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