[webkit-reviews] review denied: [Bug 14185] 'round' not implemented in border-image : [Attachment 42585] patch v2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Benjamin Otte <otte at gnome.org>'s
request for review:
Bug 14185: 'round' not implemented in border-image
https://bugs.webkit.org/show_bug.cgi?id=14185

Attachment 42585: patch v2
https://bugs.webkit.org/attachment.cgi?id=42585&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Thanks for tackling this.

>      if (hRule == Image::RepeatTile)
>	   scaleX = scaleY;
> +    else if (hRule == Image::RoundTile)
> +	   scaleX = dstRect.width() / (srcRect.width() * roundf(dstRect.width()
/ (srcRect.width() * scaleY)));
> +
>      if (vRule == Image::RepeatTile)
>	   scaleY = scaleX;
> +    else if (vRule == Image::RoundTile)
> +	   scaleY = dstRect.height() / (srcRect.height() *
roundf(dstRect.height() / (srcRect.height() * scaleX)));

I think these should use switch statements now. It helps document that we have
cases to cover all values.

Since you're adding a new regression test, you need to include regression test
results as well. Also, a good test documents what the result is supposed to
look like so people, not just computers, can judge whether it is a success or
failure. Ideally tests can be judged for correctness without requiring
pixel-test results, but I suppose that may be impossible in this case.

review- because the patch has a new test, but no test results