Bug 136319 - Subpixel layout: Rename LayoutRect's device pixel snapping functions.
Summary: Subpixel layout: Rename LayoutRect's device pixel snapping functions.
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
Depends on:
Blocks: 136275
  Show dependency treegraph
Reported: 2014-08-27 15:47 PDT by zalan
Modified: 2014-08-29 10:00 PDT (History)
3 users (show)

See Also:

Patch (142.64 KB, patch)
2014-08-27 15:53 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (142.79 KB, patch)
2014-08-27 20:23 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (142.85 KB, patch)
2014-08-27 20:41 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2014-08-27 15:47:53 PDT
to the new naming style of snapRectToDevicePixels, enclosingRectToDevicePixels, snappedIntRect, enclosingIntRect
Comment 1 zalan 2014-08-27 15:53:15 PDT
Created attachment 237267 [details]
Comment 2 Simon Fraser (smfr) 2014-08-27 16:13:26 PDT
Comment on attachment 237267 [details]

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);

Comment 3 zalan 2014-08-27 20:23:57 PDT
Created attachment 237282 [details]
Comment 4 zalan 2014-08-27 20:25:34 PDT
wait for bug #136314
Comment 5 zalan 2014-08-27 20:41:37 PDT
Created attachment 237284 [details]
Comment 6 WebKit Commit Bot 2014-08-27 21:24:45 PDT
Comment on attachment 237284 [details]

Clearing flags on attachment: 237284

Committed r173047: <http://trac.webkit.org/changeset/173047>
Comment 7 WebKit Commit Bot 2014-08-27 21:24:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Darin Adler 2014-08-29 10:00:23 PDT
(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:


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.