[webkit-reviews] review denied: [Bug 128460] Subpixel rendering: Make GraphicsLayerClient::paintContents's clip rect subpixel based. : [Attachment 223794] 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 128460: Subpixel rendering: Make GraphicsLayerClient::paintContents's clip
rect subpixel based.
https://bugs.webkit.org/show_bug.cgi?id=128460

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

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


r- because of my questions.

>
Source/WebCore/platform/graphics/texmap/coordinated/CompositingCoordinator.cpp:
258
> +    m_client->paintLayerContents(graphicsLayer, graphicsContext,
IntRect(clipRect));

enclosingIntRect()? If you plan to change this later, it should get a FIXME I
think.

> Source/WebCore/rendering/RenderLayer.cpp:2652
> -static LayoutRect cornerRect(const RenderLayer* layer, const LayoutRect&
bounds)
> +static IntRect cornerRect(const RenderLayer* layer, const IntRect& bounds)

Not sure why you changed these; the corner/scrollbar rects need to be device
pixel aligned at some point, so won't that involve going back to layout units?

> Source/WebCore/rendering/RenderLayerBacking.cpp:2198
> +    IntRect
pixelSnappedRectForIntegralPositionedItems(pixelSnappedIntRect(dirtyRect));

We normally initialize with =, so pixelSnappedRectForIntegralPositionedItems =
pixelSnappedIntRect(dirtyRect);

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2613
> +    IntRect
pixelSnappedRectForIntegralPositionedItems(pixelSnappedIntRect(LayoutRect(clip)
));

Ditto.

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextGL.cpp:403
> +    core(m_webView)->mainFrame().view()->paint(&context,
IntRect(rectToPaint));

enclosing?

> Source/WebKit/win/WebView.cpp:6736
> +    frame->view()->paint(&context, IntRect(inClip));

enclosing?

> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:278
> +	   m_webPage->drawRect(graphicsContext, IntRect(clipRect));

enclosing?

> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:284
> +	       m_webPage->drawPageOverlay(pageOverlayLayer.key,
graphicsContext, IntRect(clipRect));

ditto.

> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:196
> +	       m_webPage->drawPageOverlay(overlayAndLayer.key, graphicsContext,
IntRect(clipRect));

ditto

> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:363
> +	       m_webPage->drawPageOverlay(it->key, graphicsContext,
IntRect(clipRect));

ditto.