366865 - width distribution of spanning cells suffers from rounding error

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Width distribution of column-spanning cells suffers from rounding error since we distribute each cell using the same math operations.  We should instead cumulatively rebuild the ratios after each distribution to a column so that we don't end up a pixel or two off after the cumulative rounding error for each column.

This breaks my first reftest for bug 363329 at some DPI/font metrics combinations.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: work in progress

There's something wrong with the ComputeColumnIntrinsicWidths changes here that I haven't figured out yet...

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

It took way too long to figure out what was wrong with the first loop, because gdb was lying to me about the state of the variables when I hit the assertion.  It turned out it was the assertion that was wrong.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 253875 [details] [diff] [review]
patch

This looks somewhat complicated but really isn't all that bad.  It's modifying two loops whose purpose is to distribute widths among columns based on the ratio of some parameter(s) of the columns.  It's modifying those so that instead of computing the ratio once at the beginning of the loop, we recompute it each time based on only the remaining columns so that we don't accumulate rounding error.  This means we will distribute exactly the amount we were asked to distribute, whereas in the old code we sometimes distributed a little more or less.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And reftests pass (module the one test from bug 1156 that's been failing for me).

Comment 5

[Bernd]

sorry for the delay, the review  is now my top priority will do it over the weekend

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk.

Comment 7

[Jeff Walden [:Waldo]]

Already tested by the failing tests which triggered the filing of this bug.