Bug 56758

Summary: percentages are incorrectly rounded in WebKit
Product: WebKit Reporter: Rik Cabanier <cabanier>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Updated length class. Incorporated David Hyatt and Simon Fraser's review
simon.fraser: review+, simon.fraser: commit-queue+
updated pixel test
none
updated pixel test
none
removed xcode project changes none

Rik Cabanier
Reported 2011-03-21 11:15:57 PDT
WebCore stores percentages as fixed point with limited precision. Because of this, one of the layout tests (fast/css/percentage-non-integer.html) is not rendering correctly. The Length class should be updated to use float instead of fixed.
Attachments
Updated length class. Incorporated David Hyatt and Simon Fraser's review (56.15 KB, patch)
2011-03-21 11:16 PDT, Rik Cabanier 		simon.fraser: review+
simon.fraser: commit-queue+
DetailsFormatted DiffDiff
updated pixel test (56.15 KB, patch)
2011-03-21 14:12 PDT, Rik Cabanier 		no flags
DetailsFormatted DiffDiff
updated pixel test (56.72 KB, patch)
2011-03-21 15:05 PDT, Rik Cabanier 		no flags
DetailsFormatted DiffDiff
removed xcode project changes (56.17 KB, patch)
2011-03-21 15:16 PDT, Rik Cabanier 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Rik Cabanier
Comment 1 2011-03-21 11:16:55 PDT
Created attachment 86339 [details] Updated length class. Incorporated David Hyatt and Simon Fraser's review
Rik Cabanier
Comment 2 2011-03-21 11:40:18 PDT
From Simon's review: >> Source/WebCore/rendering/AutoTableLayout.cpp:235 >> + const float epsilon = 1 / 128.0f; > It's not obvious why you chose 1/128 here. I did it to keep the existing behavior intact. Percentages used to be fixed point and epsilon was set to 1 which is 1/128 of a percentage point. The whole 'epsilon' logic is flawed and should probably be removed...
Rik Cabanier
Comment 3 2011-03-21 14:12:02 PDT
Created attachment 86363 [details] updated pixel test
Rik Cabanier
Comment 4 2011-03-21 15:05:09 PDT
Created attachment 86370 [details] updated pixel test
Rik Cabanier
Comment 5 2011-03-21 15:09:56 PDT
*** Bug 56052 has been marked as a duplicate of this bug. ***
Rik Cabanier
Comment 6 2011-03-21 15:16:47 PDT
Created attachment 86374 [details] removed xcode project changes
James Robinson
Comment 7 2011-03-21 15:19:00 PDT
Comment on attachment 86374 [details] removed xcode project changes New test expectations look OK. R=me (smfr already r+'d the code changes)
WebKit Commit Bot
Comment 8 2011-03-21 17:24:17 PDT
Comment on attachment 86374 [details] removed xcode project changes Clearing flags on attachment: 86374 Committed r81625: <http://trac.webkit.org/changeset/81625>
WebKit Commit Bot
Comment 9 2011-03-21 17:24:22 PDT
All reviewed patches have been landed. Closing bug.
Adam Roben (:aroben)
Comment 10 2011-03-22 06:30:38 PDT
This change broke fast/css/percentage-non-integer.html on Windows: http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r81625%20(10623)/fast/css/percentage-non-integer-pretty-diff.html
Adam Roben (:aroben)
Comment 11 2011-03-22 06:53:29 PDT
It looks like the rounding on Windows is worse than on Mac. On Mac, the table cells increase in width by 2px each time. On Windows, they sometimes increase by other amounts.
Adam Roben (:aroben)
Comment 12 2011-03-22 07:01:37 PDT
I filed bug 56829 to cover the Windows failure.
Note You need to log in before you can comment on or make changes to this bug.