Commit 6c9a22a8 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

[TablesNG] Float rounding fix

This fixes a very subtle bug:

If table size is auto:
- table assignable size is computed as sum of its column max widths
- then, the size gets redistributed to columns
- in the old code, even though the sum of columns matched table
  width exactly, each column would still recompute its value.
- due to floating-point rounding error, a column might shrink
  or grow slightly.
- shrunk column would cause text to wrap, when it should not

I am having a hard time creating a reproducible test for this one.
I've only seen it once in my temporary test file, and fixed it.
Then I blew away my test, and decided to create a real test.
I could never make my fp math fail in the new test.
I am submitting a test, that might fail on some fp implementation,
but not on my linux box.

Bug: 958381
Change-Id: I3c5dbb5577b9c7b7f6f922974eb67fa743d947b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386285
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803618}
parent e2378696
......@@ -176,14 +176,23 @@ void DistributeInlineSizeToComputedInlineSizeAuto(
guess_size_total_increases[kMaxGuess];
LayoutUnit distributable_inline_size =
target_inline_size - guess_sizes[kSpecifiedGuess];
LayoutUnit rounding_error_inline_size = distributable_inline_size;
// When widths match exactly, this usually means that table width
// is auto, and that columns should be wide enough to accommodate
// content without wrapping.
// Instead of using floating-point math to compute final column
// width, we use max_inline_size.
// Using floating-point math can cause rounding errors, and uninintended
// line wrap.
bool is_exact_match = target_inline_size == guess_sizes[kMaxGuess];
LayoutUnit rounding_error_inline_size =
is_exact_match ? LayoutUnit() : distributable_inline_size;
NGTableTypes::Column* last_column = nullptr;
for (NGTableTypes::Column* column = start_column; column != end_column;
++column) {
if (column->percent) {
column->computed_inline_size =
column->ResolvePercentInlineSize(target_inline_size);
} else if (column->is_constrained) {
} else if (column->is_constrained || is_exact_match) {
column->computed_inline_size = *column->max_inline_size;
} else {
last_column = column;
......
......@@ -130,6 +130,34 @@ TEST_F(NGTableAlgorithmHelpersTest, DistributeColspanAutoSizeConstrained) {
EXPECT_EQ(*column_constraints[2].min_inline_size, 50);
}
TEST_F(NGTableAlgorithmHelpersTest, DistributeColspanAutoExactMaxSize) {
// If column widths sum match table widths exactly, column widths
// should not be redistributed at all.
// The error occurs if widths are redistributed, and column widths
// change due to floating point rounding.
LayoutUnit column_widths[] = {LayoutUnit(0.1), LayoutUnit(22.123456),
LayoutUnit(33.789012), LayoutUnit(2000.345678)};
NGTableTypes::Columns column_constraints;
column_constraints.Shrink(0);
column_constraints.push_back(NGTableTypes::Column{
LayoutUnit(0), column_widths[0], base::nullopt, false});
column_constraints.push_back(NGTableTypes::Column{
LayoutUnit(3.33333), column_widths[1], base::nullopt, false});
column_constraints.push_back(NGTableTypes::Column{
LayoutUnit(3.33333), column_widths[2], base::nullopt, false});
column_constraints.push_back(NGTableTypes::Column{
LayoutUnit(0), column_widths[3], base::nullopt, false});
LayoutUnit assignable_table_inline_size =
column_widths[0] + column_widths[1] + column_widths[2] + column_widths[3];
NGTableAlgorithmHelpers::SynchronizeAssignableTableInlineSizeAndColumns(
assignable_table_inline_size, LayoutUnit(), false, &column_constraints);
EXPECT_EQ(column_constraints[0].computed_inline_size, column_widths[0]);
EXPECT_EQ(column_constraints[1].computed_inline_size, column_widths[1]);
EXPECT_EQ(column_constraints[2].computed_inline_size, column_widths[2]);
EXPECT_EQ(column_constraints[3].computed_inline_size, column_widths[3]);
}
TEST_F(NGTableAlgorithmHelpersTest, ComputeGridInlineMinmax) {
NGTableTypes::Columns column_constraints;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment