Don't anti-alias backgrounds of images.

Don't anti-alias backgrounds of images.

This fixes a pixel bleed issue on very high-DPI screens.

BUG=423834
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185161

[chrishtr, 2014-11-10 23:52:46]

chrishtr@chromium.org changed reviewers:
+ enne@chromium.org, leviw@chromium.org

[chrishtr, 2014-11-10 23:52:46]

I can verify the change manually with --force-device-scale-factor=3.5. Test
still doesn't seem to show it.

[leviw_travelin_and_unemployed, 2014-11-11 00:19:50]

Code change LGTM.

https://codereview.chromium.org/711163002/diff/40001/LayoutTests/paint/images...
File LayoutTests/paint/images/image-backgrounds-not-antialiased.html (right):

https://codereview.chromium.org/711163002/diff/40001/LayoutTests/paint/images...
LayoutTests/paint/images/image-backgrounds-not-antialiased.html:1: <style
type="text/css">
Nit: doctype.

Why not a ref test with the image sans background color?

[chrishtr, 2014-11-11 00:35:03]

On 2014/11/11 at 00:19:50, leviw wrote:
> Code change LGTM.
> 
>
https://codereview.chromium.org/711163002/diff/40001/LayoutTests/paint/images...
> File LayoutTests/paint/images/image-backgrounds-not-antialiased.html (right):
> 
>
https://codereview.chromium.org/711163002/diff/40001/LayoutTests/paint/images...
> LayoutTests/paint/images/image-backgrounds-not-antialiased.html:1: <style
type="text/css">
> Nit: doctype.
> 
> Why not a ref test with the image sans background color?

Done.

[mstensho (USE GERRIT), 2014-11-11 08:02:27]

mstensho@opera.com changed reviewers:
+ mstensho@opera.com

[mstensho (USE GERRIT), 2014-11-11 08:02:27]

https://codereview.chromium.org/711163002/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderImage.h (right):

https://codereview.chromium.org/711163002/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderImage.h:84: virtual void
paintBoxDecorationBackground(PaintInfo&, const LayoutPoint&);
override?

[enne (OOO), 2014-11-11 18:13:15]

https://codereview.chromium.org/711163002/diff/100001/LayoutTests/paint/image...
File LayoutTests/paint/images/image-backgrounds-not-antialiased.html (right):

https://codereview.chromium.org/711163002/diff/100001/LayoutTests/paint/image...
LayoutTests/paint/images/image-backgrounds-not-antialiased.html:7: height:
296px; /* Not important. */
Why is this not important? If you change the device scale factor to 3.5 and the
height to 295, does the bug repro in the test case?

[chrishtr, 2014-11-11 19:27:55]

On 2014/11/11 at 18:13:15, enne wrote:
>
https://codereview.chromium.org/711163002/diff/100001/LayoutTests/paint/image...
> File LayoutTests/paint/images/image-backgrounds-not-antialiased.html (right):
> 
>
https://codereview.chromium.org/711163002/diff/100001/LayoutTests/paint/image...
> LayoutTests/paint/images/image-backgrounds-not-antialiased.html:7: height:
296px; /* Not important. */
> Why is this not important? If you change the device scale factor to 3.5 and
the height to 295, does the bug repro in the test case?

Removed that comment. It does repro.

[chrishtr, 2014-11-11 19:34:21]

https://codereview.chromium.org/711163002/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderImage.h (right):

https://codereview.chromium.org/711163002/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderImage.h:84: virtual void
paintBoxDecorationBackground(PaintInfo&, const LayoutPoint&);
On 2014/11/11 at 08:02:27, mstensho wrote:
> override?

Done.

https://codereview.chromium.org/711163002/diff/160001/Source/core/paint/Image...
File Source/core/paint/ImagePainter.cpp (right):

https://codereview.chromium.org/711163002/diff/160001/Source/core/paint/Image...
Source/core/paint/ImagePainter.cpp:207: bool shouldAntialias =
!m_renderImage.isRenderImage();
For the record, I also spent a while this morning trying out a generalization of
this patch, by extending the bleed avoidance logic in BoxDecorationData.
That code contains various special cases that are similar in concept to what
this CL is attempting.

Unfortunately, it's hard to get this type of bleed avoidance cleanly into
BoxDecorationData, without adding a new bitfield member to represent "will
eventually have an obscured background". This is because
a) images load asynchronously, so there is a difference between "obscure now"
and "will end up obscured"
b) images may be partially transparent, so we don't even know that the
background will be obscured after load
c) even if we new after load, animated gifs can change transparency between
frames (or at least that's what the code assumes)

All this makes me more comfortable with this targeted approach.

[chrishtr, 2014-11-12 01:12:06]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-11-12 01:12:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/711163002/180001

[commit-bot: I haz the power, 2014-11-12 01:14:56]

Committed patchset #10 (id:180001) as 185161