[webkit-reviews] review granted: [Bug 134523] Subpixel rendering: Inline box decoration rounds to integral. : [Attachment 234231] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Zalan Bujtas <zalan at apple.com>'s
request for review:
Bug 134523: Subpixel rendering: Inline box decoration rounds to integral.
https://bugs.webkit.org/show_bug.cgi?id=134523

Attachment 234231: Patch
https://bugs.webkit.org/attachment.cgi?id=234231&action=review

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


EWS bots show tests failing.

> Source/WebCore/rendering/InlineFlowBox.cpp:1052
> -	   LayoutRect boundsRect(roundedFrameRect());
> +	   LayoutRect boundsRect = LayoutRect(topLeft(), size());

I don’t think this syntax is great:

    C a = C(b, c);

Two possibilities are what the existing code was doing:

    C a(b, c);

or if we prefer something that looks like assignment:

    C a = { b, c };

But also, it’s kind of annoying that InlineBox doesn’t have a function that
returns this. It has logicalFrameRect but for some reason it doesn’t have
frameRect. I think it’s really strange that the InlineBox class has x, y,
width, height, left, top, size, right, and even bottom, but no function that
just returns the whole thing as a rect.

> Source/WebCore/rendering/InlineFlowBox.cpp:1078
> +    LayoutRect rect = LayoutRect(topLeft(), size());

And again.

> Source/WebCore/rendering/InlineFlowBox.cpp:1287
> +    LayoutRect frameRect = LayoutRect(topLeft(), size());

And again.

> Source/WebCore/rendering/InlineFlowBox.cpp:1359
> +    LayoutRect frameRect = LayoutRect(topLeft(), size());

And again.