[webkit-reviews] review denied: [Bug 71225] [chromium] composited layers are blurry with a zoom-in page scale factor : [Attachment 113880] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Hin-Chung Lam
<hclam at google.com>'s request for review:
Bug 71225: [chromium] composited layers are blurry with a zoom-in page scale
factor
https://bugs.webkit.org/show_bug.cgi?id=71225

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113880&action=review


Nearly there!

> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:144
> +    return IntSize(bounds().width() * contentsScale(), bounds().height() *
contentsScale());

Are you sure this does the rounding we want?

> LayoutTests/ChangeLog:6
> +	   Added test cases that has position:fixed and page scale.

since you are only adding cr-linux expectations, you need to add = MISSING
expectations for windows and mac or the bots will go red when this lands.

> LayoutTests/compositing/geometry/fixed-position-composited-page-scale.html:23

> +	 eventSender.scalePageBy(2, 0, 0);

could you add a few tests with a scale < 1?

>
LayoutTests/platform/chromium-linux/compositing/geometry/fixed-position-composi
ted-page-scale-expected.txt:2
> +layer at (0,0) size 800x585
> +  RenderView at (0,0) size 800x585

do you need the render tree dump here, or is the test just about checking the
pixels? If the latter add this incantation to the test:

layoutTestController.dumpAsText(true);

so you don't need this stuff