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+

Description Simon Fraser (smfr) 2014-03-08 14:44:26 PST
Change scrollOffsetForFixedPosition() to do LayoutUnit math
Comment 1 Simon Fraser (smfr) 2014-03-08 14:46:18 PST
Created attachment 226237 [details]
Patch
Comment 2 zalan 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?
Comment 3 zalan 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?
Comment 4 Build Bot 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
Comment 5 Build Bot 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
Comment 6 Simon Fraser (smfr) 2014-03-12 10:16:58 PDT
Created attachment 226523 [details]
Patch
Comment 7 Simon Fraser (smfr) 2014-03-12 10:42:18 PDT
Created attachment 226526 [details]
Patch
Comment 8 zalan 2014-03-12 11:00:55 PDT
Looks good.
Comment 9 Beth Dakin 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() ?
Comment 10 Simon Fraser (smfr) 2014-03-12 11:15:57 PDT
https://trac.webkit.org/r165484