Use the bounding box of all descendants for localVisualRect if there is a mask.

Use the bounding box of all descendants for localVisualRect if there is a mask.

BUG=617785
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/3076aa222b62ac770f2c887bc11b7e246fe5b063
Cr-Commit-Position: refs/heads/master@{#433443}

[chrishtr, 2016-11-18 23:29:07]

Description was changed from

==========
none


none

BUG=
==========

to

==========
none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-11-18 23:30:17]

Description was changed from

==========
none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use the bounding box of all descendants for localVisualRect if there is a mask.

BUG=617785
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-11-18 23:32:26]

chrishtr@chromium.org changed reviewers:
+ wangxianzhu@chromium.org

[chrishtr, 2016-11-18 23:32:27]

Unfortunately, boxForFilterOrMask takes the union over all descendants, which is
expensive.
OTOH this is a clean solution I think. I'm assuming it is very uncommon to have
many masks
on the same page. WDYT?

[chrishtr, 2016-11-18 23:32:30]

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

[commit-bot: I haz the power, 2016-11-18 23:33:09]

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

[Xianzhu, 2016-11-18 23:51:20]

lgtm.

Just would like to consider other alternatives after this CL fixing the bug. How
about this method: first, include mask extents into self visual overflow; then
create a clip (like css clip to apply to all descendants) using the bounds of
the mask? This avoids the tree walk and expanding the visual rect too big.

[chrishtr, 2016-11-19 00:12:04]

On 2016/11/18 at 23:51:20, wangxianzhu wrote:
> lgtm.
> 
> Just would like to consider other alternatives after this CL fixing the bug.
How about this method: first, include mask extents into self visual overflow;
then create a clip (like css clip to apply to all descendants) using the bounds
of the mask? This avoids the tree walk and expanding the visual rect too big.

The problem is that the extent of the mask is already part of visual overflow,
but that does not suffice.
It's the area beyond the end of the mask image that is a problem, because
anything that intersects that region
must get clipped out. Conceptually, a mask image has an infinite extent and only
allows content to be rastered
that intersect the image. But the bounding box of descendants suffices.

[chrishtr, 2016-11-19 00:12:11]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2016-11-19 00:12:13]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2016-11-19 00:12:50]

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

[Xianzhu, 2016-11-19 00:17:08]

On 2016/11/19 00:12:04, chrishtr wrote:
> On 2016/11/18 at 23:51:20, wangxianzhu wrote:
> > lgtm.
> > 
> > Just would like to consider other alternatives after this CL fixing the bug.
> How about this method: first, include mask extents into self visual overflow;
> then create a clip (like css clip to apply to all descendants) using the
bounds
> of the mask? This avoids the tree walk and expanding the visual rect too big.
> 
> The problem is that the extent of the mask is already part of visual overflow,
> but that does not suffice.
> It's the area beyond the end of the mask image that is a problem, because
> anything that intersects that region
> must get clipped out. Conceptually, a mask image has an infinite extent and
only
> allows content to be rastered
> that intersect the image. But the bounding box of descendants suffices.

I also proposed to create a clip using the bounds of the mask. Would this work?

[chrishtr, 2016-11-19 00:25:47]

On 2016/11/19 at 00:17:08, wangxianzhu wrote:
> On 2016/11/19 00:12:04, chrishtr wrote:
> > On 2016/11/18 at 23:51:20, wangxianzhu wrote:
> > > lgtm.
> > > 
> > > Just would like to consider other alternatives after this CL fixing the
bug.
> > How about this method: first, include mask extents into self visual
overflow;
> > then create a clip (like css clip to apply to all descendants) using the
bounds
> > of the mask? This avoids the tree walk and expanding the visual rect too
big.
> > 
> > The problem is that the extent of the mask is already part of visual
overflow,
> > but that does not suffice.
> > It's the area beyond the end of the mask image that is a problem, because
> > anything that intersects that region
> > must get clipped out. Conceptually, a mask image has an infinite extent and
only
> > allows content to be rastered
> > that intersect the image. But the bounding box of descendants suffices.
> 
> I also proposed to create a clip using the bounds of the mask. Would this
work?

Oh. Yes I think it would..

[commit-bot: I haz the power, 2016-11-19 03:19:31]

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

[commit-bot: I haz the power, 2016-11-19 03:19:32]

Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[chrishtr, 2016-11-20 00:03:48]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-11-20 00:03:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2516793002/#ps20001
(title: "none")

[commit-bot: I haz the power, 2016-11-20 00:04:01]

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-20 02:25:45]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1479600228990370,
"parent_rev": ["0b86a9bfd1875acd7e2f9d704cedb805e9960147", null], "commit_rev":
["e5a4e2aee43e7cd4fad2ec350ea0f715d404a566", null]}

[commit-bot: I haz the power, 2016-11-20 02:26:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-20 02:27:57]

Description was changed from

==========
Use the bounding box of all descendants for localVisualRect if there is a mask.

BUG=617785
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use the bounding box of all descendants for localVisualRect if there is a mask.

BUG=617785
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/3076aa222b62ac770f2c887bc11b7e246fe5b063
Cr-Commit-Position: refs/heads/master@{#433443}
==========

[commit-bot: I haz the power, 2016-11-20 02:27:58]

Patchset 2 (id:??) landed as
https://crrev.com/3076aa222b62ac770f2c887bc11b7e246fe5b063
Cr-Commit-Position: refs/heads/master@{#433443}