Inflate dirty rect by 1 pixel on a high DPI display

Inflate dirty rect by 1 pixel on a high DPI display

On a high dpi display, putImageData leaves visible artifacts around the
affected area's border.

This CL fixes it in the way that when there is antialias involved, the
dirty rect is inflated by 1 pixel.

Locally we can see this bug goes away with this fix on a retina display.
Note that this bug doesn't repro with our layout test. So a manual
test is added to prevent regression.

BUG=609820
Committed: https://crrev.com/0baf3521c90b2efda95e33c5ed26cc2c51423381
Cr-Commit-Position: refs/heads/master@{#394286}

[xidachen, 2016-05-16 14:43:55]

xidachen@chromium.org changed reviewers:
+ junov@chromium.org, senorblanco@chromium.org

[xidachen, 2016-05-16 14:43:56]

PTAL.

I tried to add a layout test, but the bug doesn't repro with a layout test.

[Justin Novosad, 2016-05-16 15:27:31]

On 2016/05/16 14:43:56, xidachen wrote:
> PTAL.
> 
> I tried to add a layout test, but the bug doesn't repro with a layout test.

There is a special API for high dpi layout tests:
testRunner.setBackingScaleFactor().
Dis you try using it?

[xidachen, 2016-05-16 15:40:59]

On 2016/05/16 15:27:31, Justin Novosad wrote:
> On 2016/05/16 14:43:56, xidachen wrote:
> > PTAL.
> > 
> > I tried to add a layout test, but the bug doesn't repro with a layout test.
> 
> There is a special API for high dpi layout tests:
> testRunner.setBackingScaleFactor().
> Dis you try using it?

Yes, please take a look at PS#2, where I put my layout test in there.

With that test, what we should see is a transparent black canvas at the end if
everything is correct. In chromium, there is a whole bunch of artifact left
behind without this CL. But when I run that under virtual/gpu/, or under the
normal mode, the result is a clear canvas. I don't understand that.

[Justin Novosad, 2016-05-16 15:56:49]

Could be due to mesa no behaving exactly the same as a real GPU, or the
compositor not behaving exactly the same way. I suspect mesa might not support
the partial swapbuffers extension, which is probably at the root of this issue.
Perhaps you should just put this in "ManualTests". If you do that, add a
paragraph at the top of the page that explains what the test does and what
should be observed.

https://codereview.chromium.org/1983863002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/canvas/canvas-ClearRect-hidpi.html
(right):

https://codereview.chromium.org/1983863002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/canvas-ClearRect-hidpi.html:8:
testRunner.setBackingScaleFactor(2, function() { init(); });
function() { init(); }   ->   init

[xidachen, 2016-05-16 16:53:16]

https://codereview.chromium.org/1983863002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/canvas/canvas-ClearRect-hidpi.html
(right):

https://codereview.chromium.org/1983863002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/canvas-ClearRect-hidpi.html:8:
testRunner.setBackingScaleFactor(2, function() { init(); });
On 2016/05/16 15:56:49, Justin Novosad wrote:
> function() { init(); }   ->   init

I tried this, but that doesn't work either. So I move the test to ManualTests/.

[Justin Novosad, 2016-05-16 17:08:54]

https://codereview.chromium.org/1983863002/diff/40001/third_party/WebKit/Manu...
File third_party/WebKit/ManualTests/canvas-ClearRect-hidpi.html (right):

https://codereview.chromium.org/1983863002/diff/40001/third_party/WebKit/Manu...
third_party/WebKit/ManualTests/canvas-ClearRect-hidpi.html:6: the result should
be a clear canvas with no artifact.
... after a few seconds.

[Justin Novosad, 2016-05-16 17:09:51]

On 2016/05/16 17:08:54, Justin Novosad wrote:
>
https://codereview.chromium.org/1983863002/diff/40001/third_party/WebKit/Manu...
> File third_party/WebKit/ManualTests/canvas-ClearRect-hidpi.html (right):
> 
>
https://codereview.chromium.org/1983863002/diff/40001/third_party/WebKit/Manu...
> third_party/WebKit/ManualTests/canvas-ClearRect-hidpi.html:6: the result
should
> be a clear canvas with no artifact.
> ... after a few seconds.

otherwise, lgtm

[xidachen, 2016-05-16 17:13:34]

Description was changed from

==========
Inflate dirty rect by 1 pixel on a high DPI display

On a high dpi display, putImageData leaves visible artifacts around the
affected area's border.

This CL fixes it in the way that when there is antialias involved, the
dirty rect is inflated by 1 pixel.

Locally we can see this bug goes away with this fix on a retina display.

BUG=609820
==========

to

==========
Inflate dirty rect by 1 pixel on a high DPI display

On a high dpi display, putImageData leaves visible artifacts around the
affected area's border.

This CL fixes it in the way that when there is antialias involved, the
dirty rect is inflated by 1 pixel.

Locally we can see this bug goes away with this fix on a retina display. 
Note that this bug doesn't repro with our layout test. So a manual
test is added to prevent regression.

BUG=609820
==========

[xidachen, 2016-05-16 17:13:41]

The CQ bit was checked by xidachen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-16 17:14:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1983863002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1983863002/60001

[xidachen, 2016-05-16 18:11:53]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2016-05-17 18:11:26]

The CQ bit was checked by xidachen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-17 18:11:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1983863002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1983863002/80001

[xidachen, 2016-05-17 18:58:16]

The CQ bit was checked by xidachen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-17 18:58:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1983863002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1983863002/100001

[xidachen, 2016-05-17 19:54:12]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2016-05-18 00:27:32]

The CQ bit was checked by xidachen@chromium.org

[xidachen, 2016-05-18 00:27:32]

The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org
Link to the patchset: https://codereview.chromium.org/1983863002/#ps100001
(title: "nullcheck on page()")

[commit-bot: I haz the power, 2016-05-18 00:28:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1983863002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1983863002/100001

[commit-bot: I haz the power, 2016-05-18 00:33:47]

Description was changed from

==========
Inflate dirty rect by 1 pixel on a high DPI display

On a high dpi display, putImageData leaves visible artifacts around the
affected area's border.

This CL fixes it in the way that when there is antialias involved, the
dirty rect is inflated by 1 pixel.

Locally we can see this bug goes away with this fix on a retina display. 
Note that this bug doesn't repro with our layout test. So a manual
test is added to prevent regression.

BUG=609820
==========

to

==========
Inflate dirty rect by 1 pixel on a high DPI display

On a high dpi display, putImageData leaves visible artifacts around the
affected area's border.

This CL fixes it in the way that when there is antialias involved, the
dirty rect is inflated by 1 pixel.

Locally we can see this bug goes away with this fix on a retina display. 
Note that this bug doesn't repro with our layout test. So a manual
test is added to prevent regression.

BUG=609820
==========

[commit-bot: I haz the power, 2016-05-18 00:33:49]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-18 00:36:32]

Description was changed from

==========
Inflate dirty rect by 1 pixel on a high DPI display

On a high dpi display, putImageData leaves visible artifacts around the
affected area's border.

This CL fixes it in the way that when there is antialias involved, the
dirty rect is inflated by 1 pixel.

Locally we can see this bug goes away with this fix on a retina display. 
Note that this bug doesn't repro with our layout test. So a manual
test is added to prevent regression.

BUG=609820
==========

to

==========
Inflate dirty rect by 1 pixel on a high DPI display

On a high dpi display, putImageData leaves visible artifacts around the
affected area's border.

This CL fixes it in the way that when there is antialias involved, the
dirty rect is inflated by 1 pixel.

Locally we can see this bug goes away with this fix on a retina display.
Note that this bug doesn't repro with our layout test. So a manual
test is added to prevent regression.

BUG=609820

Committed: https://crrev.com/0baf3521c90b2efda95e33c5ed26cc2c51423381
Cr-Commit-Position: refs/heads/master@{#394286}
==========

[commit-bot: I haz the power, 2016-05-18 00:36:33]

Patchset 6 (id:??) landed as
https://crrev.com/0baf3521c90b2efda95e33c5ed26cc2c51423381
Cr-Commit-Position: refs/heads/master@{#394286}