Bug 136319 – Subpixel layout: Rename LayoutRect's device pixel snapping functions.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[zalan]

to the new naming style of snapRectToDevicePixels, enclosingRectToDevicePixels, snappedIntRect, enclosingIntRect

[zalan]

Created attachment 237267 [details]
Patch

[Simon Fraser (smfr)]

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?

[zalan]

Created attachment 237282 [details]
Patch

[zalan]

wait for bug #136314

[zalan]

Created attachment 237284 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 237284 [details]
Patch

Clearing flags on attachment: 237284

Committed r173047: <http://trac.webkit.org/changeset/173047>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

(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.