Bug 133436 - Don't scroll to non-integral scroll offsets
Summary: Don't scroll to non-integral scroll offsets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-06-01 12:43 PDT by Simon Fraser (smfr)
Modified: 2014-06-01 16:07 PDT (History)
9 users (show)

See Also:

Attachments
Patch (2.39 KB, patch)
2014-06-01 12:45 PDT, Simon Fraser (smfr) 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2014-06-01 12:43:34 PDT 
Don't scroll to non-integral scroll offsets
Comment 1 Simon Fraser (smfr) 2014-06-01 12:45:53 PDT 
Created attachment 232351 [details]
Patch
Comment 2 Simon Fraser (smfr) 2014-06-01 12:46:17 PDT 
<rdar://problem/17088336>
Comment 3 zalan 2014-06-01 13:24:59 PDT 
Comment on attachment 232351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232351&action=review

> Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:324
> +    // FIXME: when we support half-pixel scroll positions on Retina displays, this will need to round to half pixels.

half-pixel -> device pixel

> Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:325
> +    FloatPoint roundedPosition(roundf(scrollPosition.x()), roundf(scrollPosition.y()));

I am wondering whether the input devices that send integral values do integer truncation or rounding. (and whether we should mimic that behavior)
Comment 4 Darin Adler 2014-06-01 14:49:04 PDT 
Comment on attachment 232351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232351&action=review

>> Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:325
>> +    FloatPoint roundedPosition(roundf(scrollPosition.x()), roundf(scrollPosition.y()));
> 
> I am wondering whether the input devices that send integral values do integer truncation or rounding. (and whether we should mimic that behavior)

Is this the single correct bottleneck for the rounding? What about the setScrollPositionWithoutContentEdgeConstraints and setScrollLayerPosition functions?
Comment 5 Simon Fraser (smfr) 2014-06-01 15:34:47 PDT 
(In reply to comment #4)
> (From update of attachment 232351 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232351&action=review
> 
> >> Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:325
> >> +    FloatPoint roundedPosition(roundf(scrollPosition.x()), roundf(scrollPosition.y()));
> > 
> > I am wondering whether the input devices that send integral values do integer truncation or rounding. (and whether we should mimic that behavior)
> 
> Is this the single correct bottleneck for the rounding? What about the setScrollPositionWithoutContentEdgeConstraints and setScrollLayerPosition functions?

This is the bottleneck, yes.
Comment 6 WebKit Commit Bot 2014-06-01 16:07:51 PDT 
Comment on attachment 232351 [details]
Patch

Clearing flags on attachment: 232351

Committed r169519: <http://trac.webkit.org/changeset/169519>
Comment 7 WebKit Commit Bot 2014-06-01 16:07:55 PDT 
All reviewed patches have been landed.  Closing bug.