[webkit-reviews] review granted: [Bug 76177] SVG group getBBox returns 0, 0, 0, 0 for a group of perpendicular paths : [Attachment 122241] Fix getBBox for perpendicular paths

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Philip Rogers <pdr at google.com>'s
request for review:
Bug 76177: SVG group getBBox returns 0,0,0,0 for a group of perpendicular paths
https://bugs.webkit.org/show_bug.cgi?id=76177

Attachment 122241: Fix getBBox for perpendicular paths
https://bugs.webkit.org/attachment.cgi?id=122241&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=122241&action=review


> Source/WebCore/platform/graphics/FloatRect.cpp:114
> +    float left = min(x(), other.x());
> +    float top = min(y(), other.y());
> +    float right = max(maxX(), other.maxX());
> +    float bottom = max(maxY(), other.maxY());

The "l, t, r, b" or "left, top, right, bottom" naming is obsolete for IntRect
and FloatRect. In new code we should name these things like x, y, maxX, and
maxY.

> Source/WebCore/platform/graphics/FloatRect.cpp:119
>  void FloatRect::uniteIfNonZero(const FloatRect& other)

Not new to this patch:

It would be clearer if the uniteIfNonZero function used the isZero function
rather than having a hand-written distinct implementation of “non-zero”. I am
concerned that this function’s definition of 0 is !x, while FloatSize’s
definition of 0 is fabs(x) < epsilon. It looks like Levi made that change to
FloatSize in July and I am not sure why it’s not needed here as well.