Summary: | LayoutUnit should round half consistently, not away from zero | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Levi Weintraub <leviw> | ||||||||||
Component: | Layout and Rendering | Assignee: | Levi Weintraub <leviw> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | allan.jensen, buildbot, darin, eae, eric, ojan.autocc, rniwa, simon.fraser, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Levi Weintraub
2013-01-17 17:44:58 PST
Created attachment 183334 [details]
Patch
Attachment 183334 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/platform/chromium-linux/fast/regions/overflow-size-change-in-variable-width-regions-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5]
Total errors found: 1 in 10 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 183334 [details]
Patch
I assume this makes us more like the pre-subpixel behavior?
Comment on attachment 183334 [details] Patch Attachment 183334 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15949106 New failing tests: fast/regions/overflow-size-change-with-stacking-context.html fast/regions/overflow-size-change-in-variable-width-regions.html fast/regions/overflow-moving-below-floats-in-variable-width-regions.html Comment on attachment 183334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183334&action=review > Source/WebCore/platform/LayoutUnit.h:231 > + return saturatedSubtraction(rawValue(), 1 + kEffectiveFixedPointDenominator / 2) / kEffectiveFixedPointDenominator; For non-saturated you do 1 + value - (denominator/2) yet for saturated you do value - 1 + (denominator/2). How about doing the operations in the same order for both? return (saturatedSubtraction(rawValue(), kEffectiveFixedPointDenominator / 2) + 1) / kEffectiveFixedPointDenominator; and return (m_value - (kEffectiveFixedPointDenominator / 2) + 1) / kEffectiveFixedPointDenominator; perhaps? > Source/WebCore/rendering/RenderLayer.cpp:-3655 > - if (this == localPaintingInfo.rootLayer) Nice! Glad to see this go! Also, you'll need to update Tools/TestWebKitAPI/Tests/WebCore/LayoutUnit.cpp Created attachment 183354 [details]
Suggested api test changes
Comment on attachment 183334 [details] Patch Attachment 183334 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15943286 New failing tests: fast/regions/overflow-size-change-with-stacking-context.html fast/regions/overflow-size-change-in-variable-width-regions.html fast/regions/overflow-moving-below-floats-in-variable-width-regions.html Comment on attachment 183334 [details] Patch Attachment 183334 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15940471 New failing tests: fast/regions/overflow-size-change-with-stacking-context.html fast/regions/overflow-size-change-in-variable-width-regions.html fast/regions/overflow-moving-below-floats-in-variable-width-regions.html Created attachment 183510 [details]
Patch for landing
Comment on attachment 183510 [details] Patch for landing Rejecting attachment 183510 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: xpectations:1286: Path does not exist. [test/expectations] [5] Total errors found: 6 in 2 files Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/platform/chromium/TestExpectat..." exit_code: 1 s:1286: expecting "[", "#", or end of line instead of "fast/regions/overflow-size-change-with-stacking-context.html" [test/expectations] [5] LayoutTests/platform/mac/TestExpectations:1286: Path does not exist. [test/expectations] [5] Total errors found: 6 in 2 files Full output: http://queues.webkit.org/results/15941771 Created attachment 183521 [details]
Patch for landing
Comment on attachment 183521 [details] Patch for landing Clearing flags on attachment: 183521 Committed r140192: <http://trac.webkit.org/changeset/140192> All reviewed patches have been landed. Closing bug. |