[webkit-reviews] review granted: [Bug 131144] Subpixel rendering: Move background images to device pixel boundaries. : [Attachment 228461] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Zalan Bujtas
<zalan at apple.com>'s request for review:
Bug 131144: Subpixel rendering: Move background images to device pixel
boundaries.
https://bugs.webkit.org/show_bug.cgi?id=131144

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

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


> Source/WebCore/platform/graphics/LayoutPoint.h:191
> +inline FloatSize pixelSnappedForPainting(const LayoutSize& s, const
LayoutPoint& p, float pixelSnappingFactor)
> +{
> +    return FloatSize(snapSizeToDevicePixel(s.width(), p.x(),
pixelSnappingFactor), snapSizeToDevicePixel(s.height(), p.y(),
pixelSnappingFactor));
> +}

Is this any different from snapping a rect with location p and size s and
reading the size?

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1051
> +void
RenderBoxModelObject::pixelSnappedBackgroundImageGeometryForPainting(Background
ImageGeometry& geometry) const

I would call this pixelSnapBackgroundImageGeometryForPainting().

pixelSnappedBackgroundImageGeometryForPainting would be like:
BackgroundImageGeometry pixelSnappedBackgroundImageGeometryForPainting(const
BackgroundImageGeometry&)

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1142
> +	   int nrTiles = std::max(1, roundToInt(positioningAreaSize.width() /
fillTileSize.width()));

Maybe nrTiles -> numTiles while you're here.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1154
> +	   int nrTiles = std::max(1, roundToInt(positioningAreaSize.height() /
fillTileSize.height()));

Ditto.

> LayoutTests/css3/background/background-repeat-round-border-expected.html:12
> -		   background-size: 111px 85px;
> +		   background-size: 111px 85.5px;

Hmm, should we have half pixels in non-Retina situations?