Bug 64126 – Turn antialiasing on only for rects which are are rotated or not scaled

• Component: Layout and Rendering
• Status: NEW

[Dmitry Gorbik]

Created attachment 100029 [details]
reduction

If we have two nested divs with different background color, and scaling is applied, border appears.

[Dmitry Gorbik]

Created attachment 100907 [details]
Proposed fix 0.1

Proposed fix 0.1

[Simon Fraser (smfr)]

Comment on attachment 100907 [details]
Proposed fix 0.1

View in context: https://bugs.webkit.org/attachment.cgi?id=100907&action=review

> LayoutTests/transforms/2d/antialiasing-pixel-cracks.html:10
> +			There should not be a border

We try to avoid text in pixel results because of platform differences. You can make it an HTML comment.

> Source/WebCore/ChangeLog:8
> +        we need to turn antialiasing in the cases when we do not need it: when

... turn off...

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:721
> +    if (!canEnableAntialiasing())
> +        CGContextSetShouldAntialias(platformContext(), false);

I think this is going to have unwanted side effects. You're changing a lot of behavior here.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:845
> +    if (!canEnableAntialiasing())
> +        CGContextSetShouldAntialias(platformContext(), false);

You should call canEnableAntialiasing() just once, and use a flag to reset it.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:851
> +    if (!canEnableAntialiasing())
> +        CGContextSetShouldAntialias(platformContext(), true);

These aren't going to nest correctly if you always set it back to true.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1159
> +bool GraphicsContext::canEnableAntialiasing(void)

How is this different from !AffineTransform::isIdentityOrTranslationOrFlipped()?

> Source/WebCore/platform/graphics/cg/ImageCG.cpp:188
> +    if (!ctxt->canEnableAntialiasing())
> +        CGContextSetShouldAntialias(ctxt->platformContext(), false);

I think this is going to have unwanted side effects. You're changing a lot of behavior here.

[Dmitry Gorbik]

Created attachment 101050 [details]
Proposed fix 0.2

[Dmitry Gorbik]

Created attachment 101053 [details]
Proposed fix 0.3

[Dmitry Gorbik]

Created attachment 101076 [details]
Proposed fix 0.4

[Dmitry Gorbik]

Created attachment 101221 [details]
Proposed fix 0.5

[Early Warning System Bot]

Comment on attachment 101221 [details]
Proposed fix 0.5

Attachment 101221 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9115441

[Gyuyoung Kim]

Comment on attachment 101221 [details]
Proposed fix 0.5

Attachment 101221 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9111492

[Gustavo Noronha (kov)]

Comment on attachment 101221 [details]
Proposed fix 0.5

Attachment 101221 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9110582

[WebKit Review Bot]

Comment on attachment 101221 [details]
Proposed fix 0.5

Attachment 101221 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9115445

[Dmitry Gorbik]

Created attachment 101237 [details]
Proposed fix 0.6

[Simon Fraser (smfr)]

Comment on attachment 101237 [details]
Proposed fix 0.6

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.

[Dmitry Gorbik]

Created attachment 103959 [details]
Proposed fix 0.7

[Andreas Kling]

Comment on attachment 103959 [details]
Proposed fix 0.7

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.