cc: Add flag to disable future mask tiling.

cc: Add flag to disable future mask tiling.

This is to prevent using new mask tiling code path, the whole change will
be discarded once mask tiling is correct.

BUG=567293
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2681183002
Cr-Commit-Position: refs/heads/master@{#452491}
Committed: https://chromium.googlesource.com/chromium/src/+/c8c1c25e6a0e79935dcac8f6118fffdd8ec02257

[sunxd, 2017-02-08 22:54:00]

Description was changed from

==========
cc: Add flag to disable future mask tiling.

This is to prevent using new mask tiling code path, the whole change will
be discarded once mask tiling is correct.

BUG=567293
==========

to

==========
cc: Add flag to disable future mask tiling.

This is to prevent using new mask tiling code path, the whole change will
be discarded once mask tiling is correct.

BUG=567293
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[sunxd, 2017-02-08 22:54:06]

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

[commit-bot: I haz the power, 2017-02-08 22:54:41]

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

[sunxd, 2017-02-08 22:55:24]

sunxd@chromium.org changed reviewers:
+ enne@chromium.org, trchen@chromium.org

[sunxd, 2017-02-08 22:55:24]

In order to break the mask tiling patch into smaller ones and land them before
fixing aa issue and demoting issue, I'm considering land this flag first.

[commit-bot: I haz the power, 2017-02-09 00:09:39]

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

[commit-bot: I haz the power, 2017-02-09 00:09:40]

Dry run: This issue passed the CQ dry run.

[sunxd, 2017-02-10 21:59:33]

PTAL.

This patch is integrated with the one of changing mask types.

[enne (OOO), 2017-02-10 22:24:34]

https://codereview.chromium.org/2681183002/diff/40001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/2681183002/diff/40001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:701: mask_type_ ==
Layer::LayerMaskType::MULTI_TEXTURE_MASK))
Maybe setting the mask type should just ignore this and set SINGLE_TEXTURE_MASK
when not supported and then you don't have to check it everywhere? 
Alternatively the caller of set mask type should do this?

I guess this is feedback for the last patch, but seeing these conditionals make
me wonder if that woudl be cleaner.

https://codereview.chromium.org/2681183002/diff/40001/cc/trees/layer_tree_set...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/2681183002/diff/40001/cc/trees/layer_tree_set...
cc/trees/layer_tree_settings.h:100: bool enable_mask_tiling = false;
= false?

[trchen, 2017-02-11 00:21:16]

https://codereview.chromium.org/2681183002/diff/40001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/2681183002/diff/40001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:701: mask_type_ ==
Layer::LayerMaskType::MULTI_TEXTURE_MASK))
+1 to enne@

I think it is cleaner to change layer.cc:422
< if (inputs_.filters.IsEmpty()) {
> if (inputs_.filters.IsEmpty() && (!layer_tree_host_ ||
layer_tree_host_->settings().enable_mask_tiling)) {

The null check is needed because the layer could be detached at the time. We
need to double check when it's actually attached.

void PictureLayer::SetLayerTreeHost(LayerTreeHost* host) {
  if (host && !host->settings().enable_mask_tiling && mask_type_ ==
LayerMaskType::MULTI_TEXTURE_MASK)
    mask_type_ = LayerMaskType::SINGLE_TEXTURE_MASK;
}

It's still gross, but grossness is more contained this way IMO. :/

[sunxd, 2017-02-13 15:30:47]

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

[commit-bot: I haz the power, 2017-02-13 15:31:05]

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, 2017-02-13 16:52:38]

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

[commit-bot: I haz the power, 2017-02-13 16:52:40]

Dry run: This issue passed the CQ dry run.

[sunxd, 2017-02-13 18:13:42]

PTAL

https://codereview.chromium.org/2681183002/diff/40001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/2681183002/diff/40001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:701: mask_type_ ==
Layer::LayerMaskType::MULTI_TEXTURE_MASK))
On 2017/02/11 00:21:16, trchen wrote:
> +1 to enne@
> 
> I think it is cleaner to change layer.cc:422
> < if (inputs_.filters.IsEmpty()) {
> > if (inputs_.filters.IsEmpty() && (!layer_tree_host_ ||
> layer_tree_host_->settings().enable_mask_tiling)) {
> 
> The null check is needed because the layer could be detached at the time. We
> need to double check when it's actually attached.
> 
> void PictureLayer::SetLayerTreeHost(LayerTreeHost* host) {
>   if (host && !host->settings().enable_mask_tiling && mask_type_ ==
> LayerMaskType::MULTI_TEXTURE_MASK)
>     mask_type_ = LayerMaskType::SINGLE_TEXTURE_MASK;
> }
> 
> It's still gross, but grossness is more contained this way IMO. :/

Yeah I think it looks better, if mask tiling is disabled, it makes sense as all
masks are single texture masks.

https://codereview.chromium.org/2681183002/diff/40001/cc/trees/layer_tree_set...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/2681183002/diff/40001/cc/trees/layer_tree_set...
cc/trees/layer_tree_settings.h:100: bool enable_mask_tiling = false;
On 2017/02/10 22:24:34, enne wrote:
> = false?

I think we want to disable mask tiling until we are sure it is correct. Also
this flag makes breaking the big CL easier.

[trchen, 2017-02-16 23:52:30]

lgtm except one nit. enne@ WDYT?

https://codereview.chromium.org/2681183002/diff/80001/cc/layers/picture_layer.cc
File cc/layers/picture_layer.cc (right):

https://codereview.chromium.org/2681183002/diff/80001/cc/layers/picture_layer...
cc/layers/picture_layer.cc:73: mask_type_ = LayerMaskType::SINGLE_TEXTURE_MASK;
nit: Move after line 77 then null-check for host can be skipped.

[enne (OOO), 2017-02-17 00:08:39]

lgtm

[sunxd, 2017-02-17 21:00:25]

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

[commit-bot: I haz the power, 2017-02-17 21:01:13]

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, 2017-02-17 23:06:56]

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

[commit-bot: I haz the power, 2017-02-17 23:06:57]

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

[sunxd, 2017-02-21 15:46:53]

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

[commit-bot: I haz the power, 2017-02-21 15:47:23]

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, 2017-02-21 15:57:04]

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

[commit-bot: I haz the power, 2017-02-21 15:57:05]

Dry run: Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)

[sunxd, 2017-02-22 22:35:19]

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

[commit-bot: I haz the power, 2017-02-22 22:36:12]

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, 2017-02-23 00:04:20]

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

[commit-bot: I haz the power, 2017-02-23 00:04:21]

Dry run: This issue passed the CQ dry run.

[sunxd, 2017-02-23 15:23:48]

The CQ bit was checked by sunxd@chromium.org

[sunxd, 2017-02-23 15:23:49]

The patchset sent to the CQ was uploaded after l-g-t-m from trchen@chromium.org,
enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2681183002/#ps140001
(title: "Correct my mistake in rebase.")

[commit-bot: I haz the power, 2017-02-23 15:24:04]

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, 2017-02-23 15:28:09]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1487863428779720,
"parent_rev": "7a38c5400e14c6175749d6ec96f275ef9b145fc3", "commit_rev":
"c8c1c25e6a0e79935dcac8f6118fffdd8ec02257"}

[commit-bot: I haz the power, 2017-02-23 15:28:45]

Description was changed from

==========
cc: Add flag to disable future mask tiling.

This is to prevent using new mask tiling code path, the whole change will
be discarded once mask tiling is correct.

BUG=567293
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Add flag to disable future mask tiling.

This is to prevent using new mask tiling code path, the whole change will
be discarded once mask tiling is correct.

BUG=567293
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2681183002
Cr-Commit-Position: refs/heads/master@{#452491}
Committed:
https://chromium.googlesource.com/chromium/src/+/c8c1c25e6a0e79935dcac8f6118f...
==========

[commit-bot: I haz the power, 2017-02-23 15:28:46]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/c8c1c25e6a0e79935dcac8f6118f...