Bug 71240 – Switch background/border image back to Int

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Emil A Eklund]

We went a bit too far with the LayoutRect/Point/Size conversion a while ago, it turns out that we want to keep image background and border rendering aligned to int boundaries to match up with device pixels.

[Emil A Eklund]

Created attachment 113091 [details]
Patch

[Emil A Eklund]

Created attachment 113113 [details]
Patch

[Daniel Bates]

Comment on attachment 113113 [details]
Patch

Attachment 113113 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/10245500

[Emil A Eklund]

Created attachment 113197 [details]
Patch

[Darin Adler]

Comment on attachment 113197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113197&action=review

This is a mix of two changes. About 2/3 of this is changing background and border back to int, and about 1/3 is changes for the newer version of LayoutUnit where implicit conversion from int is not allowed. It would be better if the change log told the truth about that.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:441
> -    LayoutUnit xPos = (isBox() ? toRenderBox(this)->left() : 0);
> -    
> +    LayoutUnit xPos = (isBox() ? toRenderBox(this)->left() : LayoutUnit(0));

This change is not part of changing background and border back to int.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:475
> -    LayoutUnit yPos = (isBox() ? toRenderBox(this)->top() : 0);
> +    LayoutUnit yPos = (isBox() ? toRenderBox(this)->top() : LayoutUnit(0));

Neither is this.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:657
> -    LayoutUnit bLeft = includeLeftEdge ? borderLeft() : 0;
> -    LayoutUnit bRight = includeRightEdge ? borderRight() : 0;
> -    LayoutUnit pLeft = includeLeftEdge ? paddingLeft() : 0;
> -    LayoutUnit pRight = includeRightEdge ? paddingRight() : 0;
> +    LayoutUnit bLeft = includeLeftEdge ? borderLeft() : LayoutUnit(0);
> +    LayoutUnit bRight = includeRightEdge ? borderRight() : LayoutUnit(0);
> +    LayoutUnit pLeft = includeLeftEdge ? paddingLeft() : LayoutUnit(0);
> +    LayoutUnit pRight = includeRightEdge ? paddingRight() : LayoutUnit(0);

Neither is this.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:679
> -        LayoutRect clipRect = LayoutRect(scrolledPaintRect.x() + bLeft + (includePadding ? pLeft : 0),
> -                                   scrolledPaintRect.y() + borderTop() + (includePadding ? paddingTop() : 0),
> -                                   scrolledPaintRect.width() - bLeft - bRight - (includePadding ? pLeft + pRight : 0),
> -                                   scrolledPaintRect.height() - borderTop() - borderBottom() - (includePadding ? paddingTop() + paddingBottom() : 0));
> +        LayoutRect clipRect = LayoutRect(scrolledPaintRect.x() + bLeft + (includePadding ? pLeft : LayoutUnit(0)),
> +                                   scrolledPaintRect.y() + borderTop() + (includePadding ? paddingTop() : LayoutUnit(0)),
> +                                   scrolledPaintRect.width() - bLeft - bRight - (includePadding ? pLeft + pRight : LayoutUnit(0)),
> +                                   scrolledPaintRect.height() - borderTop() - borderBottom() - (includePadding ? paddingTop() + paddingBottom() : LayoutUnit(0)));

Neither is this.

[Emil A Eklund]

(In reply to comment #5)
> (From update of attachment 113197 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113197&action=review
> 
> This is a mix of two changes. About 2/3 of this is changing background and border back to int, and about 1/3 is changes for the newer version of LayoutUnit where implicit conversion from int is not allowed. It would be better if the change log told the truth about that.

Good point, I'll update the ChangeLog to be explicit about this and try to avoid making unrelated changes going forward.

Thanks.

[Emil A Eklund]

Created attachment 113248 [details]
Patch for landing

[Emil A Eklund]

I reverted all changes unrelated to background and border images instead. Seemed like the better option.

[WebKit Review Bot]

Comment on attachment 113248 [details]
Patch for landing

Clearing flags on attachment: 113248

Committed r99019: <http://trac.webkit.org/changeset/99019>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.