Summary: | Subpixel layout: Rename LayoutRect's device pixel snapping functions. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | zalan <zalan> | ||||||||
Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, darin, simon.fraser | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 136275 | ||||||||||
Attachments: |
|
Description
zalan
2014-08-27 15:47:53 PDT
Created attachment 237267 [details]
Patch
Comment on attachment 237267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237267&action=review > Source/WebCore/platform/graphics/LayoutRect.h:205 > +WEBCORE_EXPORT IntRect enclosingIntRect(const LayoutRect&); > +WEBCORE_EXPORT LayoutRect enclosingLayoutRect(const FloatRect&); Do we want these to be inlined? if so, we should make an exported function that wraps them. > Source/WebCore/platform/graphics/LayoutRect.h:227 > +FloatRect enclosingRectToDevicePixels(const LayoutRect&, float pixelSnappingFactor); encloseRectToDevicePixels? Created attachment 237282 [details]
Patch
wait for bug #136314 Created attachment 237284 [details]
Patch
Comment on attachment 237284 [details] Patch Clearing flags on attachment: 237284 Committed r173047: <http://trac.webkit.org/changeset/173047> All reviewed patches have been landed. Closing bug. (In reply to comment #2) > encloseRectToDevicePixels? Sorry I missed the discussion of the new names for all these functions. I am not in favor of verb phrases to name functions that return values and have no side effects. To me it makes total sense to have a function names like these: enclosingRectOnDevicePixelBoundaries enclosedRectOnDevicePixelBoundaries But “enclose rect” doesn’t seem right as a function name to me if what the function does is return a rect that encloses another rect that’s passed in. But this one comment by me won’t substitute for a bit of discussion of terminology. I won’t stand in your way in rationalizing these and removing some of the confusing and duplicate function, but I am not sure I approve of the new naming scheme. |