| Summary: | [iOS][WK2] Round the UIScrollView content size to device pixel | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||
| Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | Keywords: | InRadar | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Benjamin Poulain
2014-05-30 21:54:49 PDT
Created attachment 232314 [details]
Patch
Comment on attachment 232314 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232314&action=review > Source/WebKit2/ChangeLog:11 > + This patch ensure the content size as seen by the API is rounded to device pixels. rounded or floored? > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:455 > +static inline CGFloat floorFloatToPixels(CGFloat input, float deviceScaleFactor) We've been using roundToDevicePixel, floorToDevicePixel terms in WebCore. Having 'float' in the function name doesn't add any useful information, while it's missing what the rounding unit is. Any explanation why we floor instead of round or ceil? (I guess, it's because you don't want garbage at the edges, but it'd be helpful to explain it in the changelog) Comment on attachment 232314 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232314&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:460 > +static CGSize roundScrollViewCountentSize(const WebKit::WebPageProxy& page, const CGSize& contentSize) "Countent" -> "Content" Is there a reason to pass contentSize by const reference instead of by value here? It seems likely it'll all be inlined anyway. Everything else looks good to me, thanks! > Is there a reason to pass contentSize by const reference instead of by value here? It seems likely it'll all be inlined anyway.
It is an habit. We pass stuff by const reference when they cannot be passed as registers. The "default" is to use const reference for non POD.
Here the CGSize can be passed by registers, so I should not have used const references.
Created attachment 232401 [details]
Patch
Comment on attachment 232401 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232401&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:460 > +static CGSize roundScrollViewContentSize(const WebKit::WebPageProxy& page, CGSize contentSize) Why not just pass in page.deviceScaleFactor()? (In reply to comment #7) > (From update of attachment 232401 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=232401&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:460 > > +static CGSize roundScrollViewContentSize(const WebKit::WebPageProxy& page, CGSize contentSize) > > Why not just pass in page.deviceScaleFactor()? Too many call sites, the repetition is not worth it. Committed r169549: <http://trac.webkit.org/changeset/169549> |