[webkit-reviews] review denied: [Bug 64126] Turn antialiasing on only for rects which are are rotated or not scaled : [Attachment 101237] Proposed fix 0.6

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Dmitry Gorbik
<socket.h at gmail.com>'s request for review:
Bug 64126: Turn antialiasing on only for rects which are are rotated or not
scaled
https://bugs.webkit.org/show_bug.cgi?id=64126

Attachment 101237: Proposed fix 0.6
https://bugs.webkit.org/attachment.cgi?id=101237&action=review

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


> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:724
> +    bool restoreAntialiasMode = false;
> +    if (shouldAntialias()) {
> +	   CGContextSetShouldAntialias(platformContext(), false);
> +	   restoreAntialiasMode = true;
> +    }

I think this is too general a fix. You'll affect all rendering here, including
canvas rendering.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1165
> +bool GraphicsContext::needsAntialiasing()

Should be const.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1175
> +    // Enable AA for any rotated objects.
> +    if (ctm.a() == -ctm.d() && ctm.b() == ctm.c() && !ctm.b())
> +	   return false;

The comment doesn't match the code.

> Source/WebCore/platform/graphics/cg/ImageCG.cpp:191
> +    bool restoreAntialiasMode = false;
> +    if (ctxt->shouldAntialias()) {
> +	   CGContextSetShouldAntialias(ctxt->platformContext(), false);
> +	   restoreAntialiasMode = true;
> +    }

Again, this is too general. You'll change canvas rendering.