Changed type of border-width longhands from unsigned to float.

Changed type of border-width longhands from unsigned to float.

An unsigned value for border-width results in the computed style being rounded, where rounding should not occur until the actual value (https://drafts.csswg.org/css-cascade-3/#actual).

The unsigned border-widths are being truncated, resulting in a visible one pixel error, e.g. https://jsfiddle.net/0j4wptzt/.

This bug is present in chrome and firefox, not in safari and IE.

BUG=227192

[Bugs Nash, 2016-01-11 05:51:18]

bugsnash@chromium.org changed reviewers:
+ alancutter@chromium.org

[Bugs Nash, 2016-01-11 05:53:02]

This is not completed. Would like a quick once-over to ensure I'm on the right
track before fixing the  3000+ tests this change breaks.

[alancutter (OOO until 2018), 2016-01-11 06:19:07]

Summary of in person discussion:

 - We should fix the visual bug in a separate patch by rounding in the style
builder instead of flooring. This ought to be a one liner.

 - When storing float in ComputedStyle we should make the getter return a
clamped version for all the existing layout and paint calls and provide a
special float getter just for getComputedStyle(). This patch currently pushes
the float throughout the layout and paint systems  only for it to get clamped
somewhere deeper in the pipeline, we can save a lot of churn and test rebasing
if we don't touch those sub systems and continue to return unsigned.

[mstensho (USE GERRIT), 2016-01-11 10:12:58]

mstensho@opera.com changed reviewers:
+ mstensho@opera.com

[mstensho (USE GERRIT), 2016-01-11 10:12:58]

Why not use LayoutUnit instead of float?

[Bugs Nash, 2016-01-11 21:56:36]

On 2016/01/11 at 10:12:58, mstensho wrote:
> Why not use LayoutUnit instead of float?

What would be the benefit of using LayoutUnit instead of float?

[Bugs Nash, 2016-01-11 22:09:56]

Description was changed from

==========
Changed type of border-with longhands from unsigned to float.

BUG=227192
==========

to

==========
Changed type of border-width longhands from unsigned to float.

An unsigned value for border-width results in the computed style being rounded,
where rounding should not occur until the actual value
(https://drafts.csswg.org/css-cascade-3/#actual).

The unsigned border-widths are being truncated, resulting in a visible one pixel
error, e.g. https://jsfiddle.net/0j4wptzt/.

This bug is present in chrome and firefox, not in safari and IE.

BUG=227192
==========

[mstensho (USE GERRIT), 2016-01-12 08:04:46]

On 2016/01/11 21:56:36, Bugs Nash wrote:
> On 2016/01/11 at 10:12:58, mstensho wrote:
> > Why not use LayoutUnit instead of float?
> 
> What would be the benefit of using LayoutUnit instead of float?

That's what the layout engine [1] normally uses to store widths, heights,
positions, padding, margin, etc.

[1] I'm talking about your changes in core/layout/ only

[Bugs Nash, 2016-01-12 21:49:26]

On 2016/01/12 at 08:04:46, mstensho wrote:
> On 2016/01/11 21:56:36, Bugs Nash wrote:
> > On 2016/01/11 at 10:12:58, mstensho wrote:
> > > Why not use LayoutUnit instead of float?
> > 
> > What would be the benefit of using LayoutUnit instead of float?
> 
> That's what the layout engine [1] normally uses to store widths, heights,
positions, padding, margin, etc.
> 
> [1] I'm talking about your changes in core/layout/ only


Ah, I see. With Alan's suggested changes I shouldn't need to touch core/layout
anyway. But thanks I'll keep that in mind.

[alancutter (OOO until 2018), 2016-01-20 06:06:24]

https://codereview.chromium.org/1574933002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSProperties.in (right):

https://codereview.chromium.org/1574933002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSProperties.in:152: border-top-width
interpolable, initial=initialBorderWidth, converter=convertLineWidth<float>
We should modify convertLineWidth() to clamp to a minimum of 0 instead of
-std::numeric_limits<float>::max().

https://codereview.chromium.org/1574933002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/AnimatedStyleBuilder.cpp
(right):

https://codereview.chromium.org/1574933002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/AnimatedStyleBuilder.cpp:102:
template<typename T> T animatableLineWidthClamp(const AnimatableValue* value, T
min = defaultMinimumForClamp<T>(), T max = defaultMaximumForClamp<T>())
No need for min/max parameters. Line widths are always non-negative so the min 0
can be hard coded and max is defaulted anyway.

https://codereview.chromium.org/1574933002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1574933002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:448: return
std::max(clientHeight(), static_cast<LayoutUnit>((layoutOverflowRect().maxY() -
borderTop())));
Use LayoutUnit() instead of static_cast<LayoutUnit>().

[Bugs Nash, 2016-02-02 01:18:18]

bugsnash@chromium.org changed reviewers:
+ leviw@chromium.org

[Bugs Nash, 2016-02-02 01:24:38]

Levi can you please have a first parse of this patch? There are >3000 broken
tests and I'm spending a lot of time on these. Can you please check this before
I spend too much more time on these?

[leviw_travelin_and_unemployed, 2016-02-02 02:20:18]

I suspect there are lots of implicit conversions to int going on that you'll
still need to track down.

https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:439: return
std::max(clientWidth(), static_cast<LayoutUnit>((layoutOverflowRect().maxX() -
borderLeft())));
Echo-ing the static_cast comment (use the constructor instead)

https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right):

https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutTableCell.h:153: styleLogicalHeight
+= (computedCSSPaddingBefore() + computedCSSPaddingAfter()).floor() +
borderBefore().toFloat() + borderAfter().toFloat();
At the moment, these should be added to padding and floored at the end. We still
use integers for sizing boxes in Tables.

https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/BoxPainter.cpp (right):

https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/BoxPainter.cpp:363: int bLeft =
includeLeftEdge ? obj.borderLeft().toInt() : 0;
Floor?

[Bugs Nash, 2016-02-02 03:32:17]

On 2016/02/02 at 02:20:18, leviw wrote:
> I suspect there are lots of implicit conversions to int going on that you'll
still need to track down.

Yep, I was planning on first just changing what I had to to fix the bug and make
it compile, then later seeing what other types needed changing. I've left a
couple of TODOs where I've already stumbled across some

> 
>
https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):
> 
>
https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutBox.cpp:439: return
std::max(clientWidth(), static_cast<LayoutUnit>((layoutOverflowRect().maxX() -
borderLeft())));
> Echo-ing the static_cast comment (use the constructor instead)
> 
>
https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right):
> 
>
https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutTableCell.h:153:
styleLogicalHeight += (computedCSSPaddingBefore() +
computedCSSPaddingAfter()).floor() + borderBefore().toFloat() +
borderAfter().toFloat();
> At the moment, these should be added to padding and floored at the end. We
still use integers for sizing boxes in Tables.
> 
>
https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/paint/BoxPainter.cpp (right):
> 
>
https://codereview.chromium.org/1574933002/diff/30001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/paint/BoxPainter.cpp:363: int bLeft =
includeLeftEdge ? obj.borderLeft().toInt() : 0;
> Floor?

[alancutter (OOO until 2018), 2016-03-03 04:39:01]

Replying to remove this CL from my name on the review dashboard, please ignore.

[mstensho (USE GERRIT), 2016-08-09 10:42:49]

Please continue on this CL or close it.

[Bugs Nash, 2016-08-09 21:17:24]

On 2016/08/09 at 10:42:49, mstensho wrote:
> Please continue on this CL or close it.

Closed, thanks for pointing that out