Work around unknown Mac Intel driver bug causing non-deterministic behavior in GPU readback from 2D

Work around unknown Mac Intel driver bug causing non-deterministic behavior in GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/e4acee67ea6e24c3e594719ca33b4beeb46b4bc6
Cr-Commit-Position: refs/heads/master@{#433094}

[Kai Ninomiya, 2016-11-17 02:35:04]

Description was changed from

==========
work around unknown Mac Intel driver bug causing nondeterministic behavior in
GPU readback from 2D canvas

BUG=665656
==========

to

==========
work around unknown Mac Intel driver bug causing nondeterministic behavior in
GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Kai Ninomiya, 2016-11-17 03:00:21]

Description was changed from

==========
work around unknown Mac Intel driver bug causing nondeterministic behavior in
GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
work around unknown Mac Intel driver bug causing nondeterministic behavior in
GPU readback from 2D canvas

// Need to update expectations
COMMIT=false

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Kai Ninomiya, 2016-11-17 03:25:20]

Description was changed from

==========
work around unknown Mac Intel driver bug causing nondeterministic behavior in
GPU readback from 2D canvas

// Need to update expectations
COMMIT=false

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
work around unknown Mac Intel driver bug causing nondeterministic behavior in
GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Kai Ninomiya, 2016-11-17 22:39:58]

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

[Kai Ninomiya, 2016-11-17 22:41:10]

kainino@chromium.org changed reviewers:
+ kbr@chromium.org, piman@chromium.org, zmo@chromium.org

[commit-bot: I haz the power, 2016-11-17 22:41:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kai Ninomiya, 2016-11-17 22:42:25]

The CQ bit was unchecked by kainino@chromium.org

[Zhenyao Mo, 2016-11-17 22:46:05]

lgtm with nits fixed

https://codereview.chromium.org/2503283007/diff/40001/content/public/common/w...
File content/public/common/web_preferences.h (right):

https://codereview.chromium.org/2503283007/diff/40001/content/public/common/w...
content/public/common/web_preferences.h:137: bool
force_software_readback_from_2d_canvas;
initialize this in constructor

https://codereview.chromium.org/2503283007/diff/40001/gpu/config/gpu_driver_b...
File gpu/config/gpu_driver_bug_workaround_type.h (right):

https://codereview.chromium.org/2503283007/diff/40001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_workaround_type.h:204:
GPU_OP(FORCE_SOFTWARE_READBACK_FROM_2D_CANVAS,             \
alphabetic order

[Kai Ninomiya, 2016-11-17 22:50:19]

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

[commit-bot: I haz the power, 2016-11-17 22:51:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kai Ninomiya, 2016-11-17 22:57:13]

kainino@chromium.org changed reviewers:
+ tsepez@chromium.org

[Kai Ninomiya, 2016-11-17 22:57:13]

tsepez: PTAL content/public/common/common_param_traits_macros.h

[Tom Sepez, 2016-11-17 22:58:05]

lgtm

[Kai Ninomiya, 2016-11-17 22:59:55]

kainino@chromium.org changed reviewers:
+ ben@chromium.org

[Kai Ninomiya, 2016-11-17 22:59:55]

ben@chromium.org: Please review changes in 
third_party/WebKit/public/web/WebSettings.h

[Ben Goodger (Google), 2016-11-17 23:02:57]

lgtm

[piman, 2016-11-17 23:06:44]

lgtm

[Ken Russell (switch to Gerrit), 2016-11-17 23:50:33]

lgtm

[esprehn, 2016-11-17 23:54:33]

Description was changed from

==========
work around unknown Mac Intel driver bug causing nondeterministic behavior in
GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Work around unknown Mac Intel driver bug causing non-deterministic behavior in
GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[esprehn, 2016-11-17 23:56:25]

esprehn@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2016-11-17 23:56:26]

Doesn't this cause a big perf regression when doing toDataURL() and toBlob() on
all mac machines?

[commit-bot: I haz the power, 2016-11-18 00:12:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 00:12:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Kai Ninomiya, 2016-11-18 00:26:25]

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

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

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 00:55:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 00:55:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Kai Ninomiya, 2016-11-18 00:58:55]

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

[commit-bot: I haz the power, 2016-11-18 01:00:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Russell (switch to Gerrit), 2016-11-18 01:04:19]

On 2016/11/17 23:56:26, esprehn wrote:
> Doesn't this cause a big perf regression when doing toDataURL() and toBlob()
on
> all mac machines?

It doesn't affect those code paths. Those already do a readback of the canvas's
contents, and they already aren't fast.

This only impacts uploads of 2D-rendered HTMLCanvasElements to WebGL textures.
It does make that code path slower on Macs with Intel GPUs, but we haven't yet
figured out exactly what the graphics driver bug is that affects this path.

We'll continue to investigate this in the next release, but this work is needed
in order to pass the WebGL 2.0 conformance tests on macOS.

(I see that your confusion might be around the flag name -- it could possibly be
called "forceSoftwareReadbackDuring2DCanvasUploadToWebGL" or similar -- we would
be happy to rename it, but since there's only one use in the code base it should
be pretty easy to figure out what it does.)

[esprehn, 2016-11-18 01:08:56]

On 2016/11/18 at 01:04:19, kbr wrote:
> On 2016/11/17 23:56:26, esprehn wrote:
> > Doesn't this cause a big perf regression when doing toDataURL() and toBlob()
on
> > all mac machines?
> 
> It doesn't affect those code paths. Those already do a readback of the
canvas's contents, and they already aren't fast.
> 
> This only impacts uploads of 2D-rendered HTMLCanvasElements to WebGL textures.
It does make that code path slower on Macs with Intel GPUs, but we haven't yet
figured out exactly what the graphics driver bug is that affects this path.
> 
> We'll continue to investigate this in the next release, but this work is
needed in order to pass the WebGL 2.0 conformance tests on macOS.
> 
> (I see that your confusion might be around the flag name -- it could possibly
be called "forceSoftwareReadbackDuring2DCanvasUploadToWebGL" or similar -- we
would be happy to rename it, but since there's only one use in the code base it
should be pretty easy to figure out what it does.)

My concern is actually with about:gpu being confusing when it lists this
workaround since it's not at all clear it's about webgl.

[Kai Ninomiya, 2016-11-18 01:36:32]

On 2016/11/18 01:08:56, esprehn wrote:
> On 2016/11/18 at 01:04:19, kbr wrote:
> > On 2016/11/17 23:56:26, esprehn wrote:
> > > Doesn't this cause a big perf regression when doing toDataURL() and
toBlob()
> on
> > > all mac machines?
> > 
> > It doesn't affect those code paths. Those already do a readback of the
> canvas's contents, and they already aren't fast.
> > 
> > This only impacts uploads of 2D-rendered HTMLCanvasElements to WebGL
textures.
> It does make that code path slower on Macs with Intel GPUs, but we haven't yet
> figured out exactly what the graphics driver bug is that affects this path.
> > 
> > We'll continue to investigate this in the next release, but this work is
> needed in order to pass the WebGL 2.0 conformance tests on macOS.
> > 
> > (I see that your confusion might be around the flag name -- it could
possibly
> be called "forceSoftwareReadbackDuring2DCanvasUploadToWebGL" or similar -- we
> would be happy to rename it, but since there's only one use in the code base
it
> should be pretty easy to figure out what it does.)
> 
> My concern is actually with about:gpu being confusing when it lists this
> workaround since it's not at all clear it's about webgl.

Thanks esprehn, good point. I will clarify at least the description text for
about:gpu in a follow-on CL.

[esprehn, 2016-11-18 01:40:00]

On 2016/11/18 at 01:36:32, kainino wrote:
> On 2016/11/18 01:08:56, esprehn wrote:
> > ...
> > 
> > My concern is actually with about:gpu being confusing when it lists this
> > workaround since it's not at all clear it's about webgl.
> 
> Thanks esprehn, good point. I will clarify at least the description text for
about:gpu in a follow-on CL.

Thanks! :D

[Zhenyao Mo, 2016-11-18 03:26:43]

The CQ bit was unchecked by zmo@chromium.org

[Zhenyao Mo, 2016-11-18 03:26:51]

The CQ bit was checked by zmo@chromium.org

[Zhenyao Mo, 2016-11-18 03:26:51]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/2503283007/#ps60001
(title: "address comments")

[commit-bot: I haz the power, 2016-11-18 03:28:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 04:07:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 04:07:22]

Internal error: failed to checkout. Please try again.

[Kai Ninomiya, 2016-11-18 04:08:33]

The CQ bit was checked by kainino@chromium.org

[commit-bot: I haz the power, 2016-11-18 04:09:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 04:17:43]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-18 04:21:17]

Description was changed from

==========
Work around unknown Mac Intel driver bug causing non-deterministic behavior in
GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Work around unknown Mac Intel driver bug causing non-deterministic behavior in
GPU readback from 2D canvas

BUG=665656
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/e4acee67ea6e24c3e594719ca33b4beeb46b4bc6
Cr-Commit-Position: refs/heads/master@{#433094}
==========

[commit-bot: I haz the power, 2016-11-18 04:21:19]

Patchset 4 (id:??) landed as
https://crrev.com/e4acee67ea6e24c3e594719ca33b4beeb46b4bc6
Cr-Commit-Position: refs/heads/master@{#433094}