cc: nine patch: add occlusion support

cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/258c7d3e78c8f4b2b96c5ade9cddb2d0b1cbfdca
Cr-Commit-Position: refs/heads/master@{#408084}

[llandwerlin-old, 2016-06-22 20:58:29]

Description was changed from

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
==========

to

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[llandwerlin-old, 2016-06-23 12:24:31]

lionel.g.landwerlin@intel.com changed reviewers:
+ ajuma@chromium.org, sky@chromium.org

[llandwerlin-old, 2016-06-23 12:24:31]

ajuma@chromium.org: Could you review the changes in cc/?
sky@chromium.org: Could you review the changes in ash/ & ui/?

Thank you

[llandwerlin-old, 2016-06-23 12:28:50]

cc oshima@ would reviewed a previous version of this CL (and subsequent fix).

I have to add that I didn't find a clean way to verify that the rounded corner
are overdrawn properly. That required either to depend on the test classes from
cc/ in ui/ or storing the occlusion in the ui::Layer only for test purposes.
I would love to read your suggestions if you have some.

Thanks!

[ajuma, 2016-06-23 18:20:06]

Description was changed from

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ajuma, 2016-06-23 18:20:07]

ajuma@chromium.org changed reviewers:
+ aelias@chromium.org
- ajuma@chromium.org

[ajuma, 2016-06-23 18:23:56]

+aelias, who's a better reviewer for this since he reviewed the previous version
of it.

Also, I'd suggest uploading a rebased version of the previous patch as a patch
set here (with the new version as the following patch set) to make it easier to
see what's changed.

[aelias_OOO_until_Jul13, 2016-06-23 18:28:09]

On 2016/06/23 at 18:23:56, ajuma wrote:
> +aelias, who's a better reviewer for this since he reviewed the previous
version of it.
> 
> Also, I'd suggest uploading a rebased version of the previous patch as a patch
set here (with the new version as the following patch set) to make it easier to
see what's changed.

Yes please, I would like to avoid rereviewing from scratch.

[llandwerlin-old, 2016-06-24 12:14:52]

Patchset #1 (id:1) has been deleted

[llandwerlin-old, 2016-06-24 12:18:48]

On 2016/06/23 18:28:09, aelias wrote:
> On 2016/06/23 at 18:23:56, ajuma wrote:
> > +aelias, who's a better reviewer for this since he reviewed the previous
> version of it.
> > 
> > Also, I'd suggest uploading a rebased version of the previous patch as a
patch
> set here (with the new version as the following patch set) to make it easier
to
> see what's changed.
> 
> Yes please, I would like to avoid rereviewing from scratch.

Sure, PTAL.

[aelias_OOO_until_Jul13, 2016-06-28 00:20:45]

https://codereview.chromium.org/2083083004/diff/40001/cc/layers/nine_patch_la...
File cc/layers/nine_patch_layer_impl.cc (right):

https://codereview.chromium.org/2083083004/diff/40001/cc/layers/nine_patch_la...
cc/layers/nine_patch_layer_impl.cc:195: border_.x() == 0 ? 0 :
(layer_occlusion_.x() * image_aperture_.x() /
This divide-by-zero avoidance looks unprincipled.  Can you DCHECK() that these
border values are never zero instead?

If you are worried about divide-by-zero crashes, I might suggest replacing some
of the variables with floats (including RectF) instead of using only ints. 
Floats seem like they should have a more correct result if you're doing scaling
operations now too.

[llandwerlin-old, 2016-07-04 14:13:27]

Sorry for the delay, here is a new version using gfx::RectF.

https://codereview.chromium.org/2083083004/diff/40001/cc/layers/nine_patch_la...
File cc/layers/nine_patch_layer_impl.cc (right):

https://codereview.chromium.org/2083083004/diff/40001/cc/layers/nine_patch_la...
cc/layers/nine_patch_layer_impl.cc:195: border_.x() == 0 ? 0 :
(layer_occlusion_.x() * image_aperture_.x() /
On 2016/06/28 00:20:45, aelias wrote:
> This divide-by-zero avoidance looks unprincipled.  Can you DCHECK() that these
> border values are never zero instead?
> 
> If you are worried about divide-by-zero crashes, I might suggest replacing
some
> of the variables with floats (including RectF) instead of using only ints. 
> Floats seem like they should have a more correct result if you're doing
scaling
> operations now too.

The reason why there are checks for divide-by-zero is that some of the existing
unit tests are actually verifying that some borders might not be drawn.
I've tried to cover roughly the same cases in case some part of chromium uses
this. It seems ChromeOS doesn't, but it's not clear to me on Android.

Switching to RectF.

[aelias_OOO_until_Jul13, 2016-07-06 23:41:29]

OK, lgtm.

[llandwerlin-old, 2016-07-25 16:23:29]

On 2016/07/06 23:41:29, aelias_OOO_until_July_25 wrote:
> OK, lgtm.

sky@: ping?

[sky, 2016-07-25 19:11:53]

LGTM

[llandwerlin-old, 2016-07-27 08:56:35]

The CQ bit was checked by lionel.g.landwerlin@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 08:56:50]

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-07-27 08:56:52]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.blink
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-27 09:50:22]

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

[commit-bot: I haz the power, 2016-07-27 09:50:23]

Dry run: This issue passed the CQ dry run.

[llandwerlin-old, 2016-07-27 09:51:21]

The CQ bit was checked by lionel.g.landwerlin@intel.com

[llandwerlin-old, 2016-07-27 09:51:21]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2083083004/#ps80001
(title: "Rebase on ToT before commit")

[commit-bot: I haz the power, 2016-07-27 09:51:50]

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-07-27 09:51:52]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.blink
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-27 09:55:42]

Description was changed from

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-07-27 09:55:45]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-07-27 09:57:34]

Description was changed from

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: nine patch: add occlusion support

Reland of https://codereview.chromium.org/1983803003 with fix for
hidpi screens

When using a NinePatchLayer to draw shadows on windows on ChromeOS
part of patches which are completely transparent are drawn on top of
the window. This prevents for example videos to be promoted into
layers.

This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute
12 patches to paint the shadow rather than 8 (center omitted). This
prevents drawing on the window's content completely and allows layer
promotion algorithms to promote the windows's content as layer if
possible (in particular with videos).

BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/258c7d3e78c8f4b2b96c5ade9cddb2d0b1cbfdca
Cr-Commit-Position: refs/heads/master@{#408084}
==========

[commit-bot: I haz the power, 2016-07-27 09:57:35]

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