[webkit-reviews] review denied: [Bug 128049] Subpixel rendering: Introduce device pixel snapping helper functions. : [Attachment 222901] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Zalan Bujtas
<zalan at apple.com>'s request for review:
Bug 128049: Subpixel rendering: Introduce device pixel snapping helper
functions.
https://bugs.webkit.org/show_bug.cgi?id=128049

Attachment 222901: Patch
https://bugs.webkit.org/attachment.cgi?id=222901&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=222901&action=review


> Source/WebCore/platform/LayoutUnit.h:942
> +inline int snapSizeToPixel(LayoutUnit size, LayoutUnit location)

I don't like "size" because we have IntSize/FloatSize with have widths and
heights.

> Source/WebCore/platform/LayoutUnit.h:948
> +inline float snapSizeToDevicePixel(LayoutUnit size, LayoutUnit location,
float devicePixelRatio)

Ditto.

Do we ever use these on things that aren't rects?

> Source/WebCore/platform/graphics/GraphicsContext.cpp:91
> +    , m_paintPixelRatio(2)

How can you assume 2 here?

> Source/WebCore/platform/graphics/GraphicsContext.h:447
> +	   float paintPixelRatio() const { return m_paintPixelRatio; }

We may have to rethink this name. Reading it in isolation here doesn't
communicate what it's for.