[webkit-reviews] review denied: [Bug 64126] Background leaks for two nested divs : [Attachment 100907] Proposed fix 0.1

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied  review:
Bug 64126: Background leaks for two nested divs
https://bugs.webkit.org/show_bug.cgi?id=64126

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
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.