Closed Bug 366865 Opened 13 years ago Closed 13 years ago

width distribution of spanning cells suffers from rounding error

Categories

(Core :: Layout: Tables, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

(Keywords: autotest-issue, Whiteboard: [patch])

Attachments

(1 file, 1 obsolete file)

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.
No longer blocks: 363329
Flags: blocking1.9?
Attached patch work in progress (obsolete) — Splinter Review
There's something wrong with the ComputeColumnIntrinsicWidths changes here that I haven't figured out yet...
Attached patch patchSplinter Review
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.
Attachment #253844 - Attachment is obsolete: true
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.
Attachment #253875 - Flags: superreview?(roc)
Attachment #253875 - Flags: review?(bernd_mozilla)
And reftests pass (module the one test from bug 1156 that's been failing for me).
OS: Linux → All
Hardware: PC → All
Whiteboard: [patch]
Attachment #253875 - Flags: superreview?(roc) → superreview+
sorry for the delay, the review  is now my top priority will do it over the weekend
Attachment #253875 - Flags: review?(bernd_mozilla) → review+
Checked in to trunk.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Already tested by the failing tests which triggered the filing of this bug.
Flags: in-testsuite-
Flags: blocking1.9?
Duplicate of this bug: 370558
You need to log in before you can comment on or make changes to this bug.