Bug 96139 – Change FractionalLayoutUnit denominator to 64 to reduce precision loss when converting to floating point

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

[Emil A Eklund]

We currently use a denominator of 60 in FractionalLayoutUnit, this causes a loss of precision when converting to floating point.
By changing the denominator to 64 the values can better be represented as floating point (without loosing any precision for many values), this in turn allows us to remove the tolerance hack in the line break logic and avoids problems caused by this precision for web sites that do their own layout based on element measurements.

[Emil A Eklund]

Created attachment 162877 [details]
Patch

[WebKit Review Bot]

Comment on attachment 162877 [details]
Patch

Attachment 162877 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13800124

New failing tests:
fast/block/float/overhanging-tall-block.html
fast/sub-pixel/large-sizes.html

[Allan Sandfeld Jensen]

Good stuff. In addition it should also make it faster to do conversions from FractionalLayoutUnit to integer.

The two tests failing seems to be the ones that overflow the integers, so that was to be expected.

[Emil A Eklund]

(In reply to comment #3)
> Good stuff. In addition it should also make it faster to do conversions from FractionalLayoutUnit to integer.

Yeah, you have some nice permanence benefits as well. Bit shifting all the way.


> The two tests failing seems to be the ones that overflow the integers, so that was to be expected.

Right, I'll upload a new patch with fixes for those once we decide whether this is a good idea or not.

Thanks.

[Emil A Eklund]

Created attachment 163165 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 163165 [details]
Patch

What was the original logic behind 60?

[Emil A Eklund]

(In reply to comment #6)
> (From update of attachment 163165 [details])
> What was the original logic behind 60?

Evenly divisible by 2, 3, 4, 5, 6 and 10.

Given the default zoom levels this doesn't actually matter as much as we thought it would.

[WebKit Review Bot]

Comment on attachment 163165 [details]
Patch

Attachment 163165 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13806592

New failing tests:
fast/block/float/overhanging-tall-block.html
fast/sub-pixel/large-sizes.html

[Emil A Eklund]

Created attachment 165681 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 165681 [details]
Patch

Looks reasonable.

[Emil A Eklund]

Thanks Eric. I'll try to commit this when the tree is quiet to avoid painful merges.

[Levi Weintraub]

Super exciting. This should pretty much eliminate the perf hit sub-pixel causes :D

[Build Bot]

Comment on attachment 165681 [details]
Patch

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

New failing tests:
fast/block/float/overhanging-tall-block.html

[Allan Sandfeld Jensen]

(In reply to comment #13)
> (From update of attachment 165681 [details])
> Attachment 165681 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/14027320
> 
> New failing tests:
> fast/block/float/overhanging-tall-block.html

When I had a patch that changed these tests, I just marked them as FAIL, and then later fixed the baseline using the garden-o-matic tool. It seemed easier that way than setting up all the different environments needed yourself.

[Emil A Eklund]

Committed r129656: <http://trac.webkit.org/changeset/129656>