Bug 129981

Summary: Change scrollOffsetForFixedPosition() to do LayoutUnit math
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, buildbot, cmarcelo, commit-queue, dino, esprehn+autocc, glenn, jamesr, kondapallykalyan, luiz, rniwa, simon.fraser, thorton, tonikitoo, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
none
Patch
none
Patch bdakin: review+

Simon Fraser (smfr)
Reported 2014-03-08 14:44:26 PST
Change scrollOffsetForFixedPosition() to do LayoutUnit math
Attachments
Patch (17.42 KB, patch)
2014-03-08 14:46 PST, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (982.11 KB, application/zip)
2014-03-08 15:56 PST, Build Bot 		no flags
Details
Patch (519.07 KB, patch)
2014-03-12 10:16 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (519.63 KB, patch)
2014-03-12 10:42 PDT, Simon Fraser (smfr) 		bdakin: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2014-03-08 14:46:18 PST
Created attachment 226237 [details] Patch
alan
Comment 2 2014-03-08 15:44:58 PST
Comment on attachment 226237 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226237&action=review Why do we round/floor to layout unit? > Source/WebCore/rendering/RenderBoxModelObject.cpp:1121 > + LayoutPoint absoluteContainerOffset = flooredLayoutPoint(paintContainer->localToAbsolute(FloatPoint())); round vs floor?
alan
Comment 3 2014-03-08 15:45:00 PST
Comment on attachment 226237 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226237&action=review Why do we round/floor to layout unit? > Source/WebCore/rendering/RenderBoxModelObject.cpp:1121 > + LayoutPoint absoluteContainerOffset = flooredLayoutPoint(paintContainer->localToAbsolute(FloatPoint())); round vs floor?
Build Bot
Comment 4 2014-03-08 15:56:20 PST
Comment on attachment 226237 [details] Patch Attachment 226237 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6309407117803520 New failing tests: platform/mac-wk2/tiled-drawing/fixed-background/fixed-body-background-zoomed.html
Build Bot
Comment 5 2014-03-08 15:56:24 PST
Created attachment 226239 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Simon Fraser (smfr)
Comment 6 2014-03-12 10:16:58 PDT
Created attachment 226523 [details] Patch
Simon Fraser (smfr)
Comment 7 2014-03-12 10:42:18 PDT
Created attachment 226526 [details] Patch
alan
Comment 8 2014-03-12 11:00:55 PDT
Looks good.
Beth Dakin
Comment 9 2014-03-12 11:01:47 PDT
Comment on attachment 226526 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226526&action=review > Source/WebCore/page/FrameView.cpp:1542 > + viewportRect.setLocation(LayoutPoint() + scrollOffsetForFixedPosition()); Just out of curiosity, why is this better than toLayoutPoint() ?
Simon Fraser (smfr)
Comment 10 2014-03-12 11:15:57 PDT
https://trac.webkit.org/r165484
Note You need to log in before you can comment on or make changes to this bug.