WIP - mask a view's layer by its view coordinates

WIP - mask a view's layer by its view coordinates

BUG=lots

[Evan Stade, 2016-11-22 00:41:14]

estade@chromium.org changed reviewers:
+ sky@chromium.org

[Evan Stade, 2016-11-22 00:41:14]

Hi Scott,

Throughout MD work we've often had to add layers to views, and this tends to
mess with inherent z-order/clipping assumptions. In the past the solution has
been to make the parent view use a layer and call SetMasksToBounds(true) on it,
but this is a bit of a band-aid as the parent can in turn suddenly have new
clipping issues.

What do you think of the approach in this patch (specifically
md_text_button.cc)? In short, create a masking layer which masks to the visible
bounds of the view. The idea is that I'd make this part of View so all you had
to do in the future would be something like:

SetPaintToLayer(true);
SetClipLayerToVisibleBounds(true);

crbug.com/659488 is one example of a bug this patch fixes (the traditional
alternative would be to make DownloadShelfItem paint to a layer and mask to
bounds).

[sky, 2016-11-22 01:02:27]

If you think we need this in a number of places than I'm fine with adding it to
view.

[Peter Kasting, 2016-11-22 01:12:22]

Would adding this mean we can get rid of a bunch of parent layers we no longer
need?

[Evan Stade, 2016-11-22 01:23:37]

On 2016/11/22 01:12:22, Peter Kasting wrote:
> Would adding this mean we can get rid of a bunch of parent layers we no longer
> need?

yes, it should. I count 4 places in c/b/u/views that would most likely use this
instead of parent layer clipping, plus more in ash/ and ui/views/, plus all
those places that are outstanding bugs.

[bruthig, 2016-11-22 01:44:09]

On 2016/11/22 01:23:37, Evan Stade wrote:
> On 2016/11/22 01:12:22, Peter Kasting wrote:
> > Would adding this mean we can get rid of a bunch of parent layers we no
longer
> > need?
> 
> yes, it should. I count 4 places in c/b/u/views that would most likely use
this
> instead of parent layer clipping, plus more in ash/ and ui/views/, plus all
> those places that are outstanding bugs.

Evan, do you think we'll be able to have less layers in total?  Or would we be
swapping a parent View painting to a layer for a mask layer that is attached to
a child?

[Evan Stade, 2016-11-22 16:26:50]

On 2016/11/22 01:44:09, bruthig wrote:
> On 2016/11/22 01:23:37, Evan Stade wrote:
> > On 2016/11/22 01:12:22, Peter Kasting wrote:
> > > Would adding this mean we can get rid of a bunch of parent layers we no
> longer
> > > need?
> > 
> > yes, it should. I count 4 places in c/b/u/views that would most likely use
> this
> > instead of parent layer clipping, plus more in ash/ and ui/views/, plus all
> > those places that are outstanding bugs.
> 
> Evan, do you think we'll be able to have less layers in total?  Or would we be
> swapping a parent View painting to a layer for a mask layer that is attached
to
> a child?

It's hard to say because adding a layer to a parent has a cascading effect
sometimes, requiring more and more layers to be added. On the other hand, if one
parent has two children and each child now needs a masking layer, that's one
more layer than before. But I also don't know if these layers are something we
can compare equally in terms of performance (a mask vs one that's actually
rendered on screen). I actually set out on this path thinking we wouldn't have
to repaint the view's layer at each step and that updating the mask layer would
be sufficient, but that doesn't seem to be the case. PS2 has an alternative that
doesn't create an extra layer (focus on the changes in view.cc --- there are
some inefficiencies and assumptions here but think of it as a POC).

[Evan Stade, 2016-11-29 02:46:20]

Description was changed from

==========
WIP - mask a view's layer by its view coordinates

BUG=lots
==========

to

==========
WIP - mask a view's layer by its view coordinates

BUG=lots
==========

[Evan Stade, 2016-11-29 02:46:29]

On 2016/11/22 16:26:50, Evan Stade wrote:
> On 2016/11/22 01:44:09, bruthig wrote:
> > On 2016/11/22 01:23:37, Evan Stade wrote:
> > > On 2016/11/22 01:12:22, Peter Kasting wrote:
> > > > Would adding this mean we can get rid of a bunch of parent layers we no
> > longer
> > > > need?
> > > 
> > > yes, it should. I count 4 places in c/b/u/views that would most likely use
> > this
> > > instead of parent layer clipping, plus more in ash/ and ui/views/, plus
all
> > > those places that are outstanding bugs.
> > 
> > Evan, do you think we'll be able to have less layers in total?  Or would we
be
> > swapping a parent View painting to a layer for a mask layer that is attached
> to
> > a child?
> 
> It's hard to say because adding a layer to a parent has a cascading effect
> sometimes, requiring more and more layers to be added. On the other hand, if
one
> parent has two children and each child now needs a masking layer, that's one
> more layer than before. But I also don't know if these layers are something we
> can compare equally in terms of performance (a mask vs one that's actually
> rendered on screen). I actually set out on this path thinking we wouldn't have
> to repaint the view's layer at each step and that updating the mask layer
would
> be sufficient, but that doesn't seem to be the case. PS2 has an alternative
that
> doesn't create an extra layer (focus on the changes in view.cc --- there are
> some inefficiencies and assumptions here but think of it as a POC).

Any opinions on the approach in ps2?

[sky, 2016-11-29 18:08:13]

https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc#newcod...
ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
Is this really enough? What happens if an animation occurs that effectively
moves the layer. Wouldn't the clip move with the layer, which isn't really waht
you want.

[Evan Stade, 2016-11-29 18:38:01]

https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc#newcod...
ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
On 2016/11/29 18:08:13, sky wrote:
> Is this really enough? What happens if an animation occurs that effectively
> moves the layer. Wouldn't the clip move with the layer, which isn't really
waht
> you want.

when the layer is moved that affects the view's transform (as per GetTransform),
which is taken into account in GetVisibleBounds.

[sky, 2016-11-29 20:27:09]

https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc#newcod...
ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
On 2016/11/29 18:38:01, Evan Stade wrote:
> On 2016/11/29 18:08:13, sky wrote:
> > Is this really enough? What happens if an animation occurs that effectively
> > moves the layer. Wouldn't the clip move with the layer, which isn't really
> waht
> > you want.
> 
> when the layer is moved that affects the view's transform (as per
GetTransform),
> which is taken into account in GetVisibleBounds.

That's assuming you end up here, right? If the layer is moved that is not
necessarily the case.

[Evan Stade, 2016-11-30 00:09:39]

On 2016/11/29 20:27:09, sky wrote:
> https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc
> File ui/views/view.cc (right):
> 
>
https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc#newcod...
> ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
> On 2016/11/29 18:38:01, Evan Stade wrote:
> > On 2016/11/29 18:08:13, sky wrote:
> > > Is this really enough? What happens if an animation occurs that
effectively
> > > moves the layer. Wouldn't the clip move with the layer, which isn't really
> > waht
> > > you want.
> > 
> > when the layer is moved that affects the view's transform (as per
> GetTransform),
> > which is taken into account in GetVisibleBounds.
> 
> That's assuming you end up here, right? If the layer is moved that is not
> necessarily the case.

If that doesn't trigger a VisibleBoundsChanged notification (and it looks like
it may not) we might need to introduce LayerDelegate::OnLayerTransformChanged,
similar to the currently extant OnLayerBoundsChanged.

[sky, 2016-11-30 00:13:47]

In thinking about this a bit more the only way to really address this
is making a parent have a layer. I don't see any other way that
doesn't have some limitation that is easy to miss.

  -Scott

On Tue, Nov 29, 2016 at 4:09 PM,  <estade@chromium.org> wrote:
> On 2016/11/29 20:27:09, sky wrote:
>> https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc
>> File ui/views/view.cc (right):
>>
>>
>
https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc#newcod...
>> ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
>> On 2016/11/29 18:38:01, Evan Stade wrote:
>> > On 2016/11/29 18:08:13, sky wrote:
>> > > Is this really enough? What happens if an animation occurs that
> effectively
>> > > moves the layer. Wouldn't the clip move with the layer, which isn't
>> > > really
>> > waht
>> > > you want.
>> >
>> > when the layer is moved that affects the view's transform (as per
>> GetTransform),
>> > which is taken into account in GetVisibleBounds.
>>
>> That's assuming you end up here, right? If the layer is moved that is not
>> necessarily the case.
>
> If that doesn't trigger a VisibleBoundsChanged notification (and it looks
> like
> it may not) we might need to introduce
> LayerDelegate::OnLayerTransformChanged,
> similar to the currently extant OnLayerBoundsChanged.
>
> https://codereview.chromium.org/2514303004/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Evan Stade, 2016-11-30 00:50:14]

On 2016/11/30 00:13:47, sky wrote:
> In thinking about this a bit more the only way to really address this
> is making a parent have a layer. I don't see any other way that
> doesn't have some limitation that is easy to miss.
> 
>   -Scott

I think in the limit that means that all views will have layers as we add more
and more layers for reasons like ink drop ripples or wanting shnazzy animations,
and parent layers to effect clipping. This is a constant source of gotchas
because everything can seem to work fine until you discover some corner case
where views start overlapping.

Anyway, I'm willing to set this aside and only come back to it if we have to.

> 
> On Tue, Nov 29, 2016 at 4:09 PM,  <mailto:estade@chromium.org> wrote:
> > On 2016/11/29 20:27:09, sky wrote:
> >> https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc
> >> File ui/views/view.cc (right):
> >>
> >>
> >
>
https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc#newcod...
> >> ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
> >> On 2016/11/29 18:38:01, Evan Stade wrote:
> >> > On 2016/11/29 18:08:13, sky wrote:
> >> > > Is this really enough? What happens if an animation occurs that
> > effectively
> >> > > moves the layer. Wouldn't the clip move with the layer, which isn't
> >> > > really
> >> > waht
> >> > > you want.
> >> >
> >> > when the layer is moved that affects the view's transform (as per
> >> GetTransform),
> >> > which is taken into account in GetVisibleBounds.
> >>
> >> That's assuming you end up here, right? If the layer is moved that is not
> >> necessarily the case.
> >
> > If that doesn't trigger a VisibleBoundsChanged notification (and it looks
> > like
> > it may not) we might need to introduce
> > LayerDelegate::OnLayerTransformChanged,
> > similar to the currently extant OnLayerBoundsChanged.
> >
> > https://codereview.chromium.org/2514303004/
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.
>

[sky, 2016-11-30 00:59:40]

On Tue, Nov 29, 2016 at 4:58 PM,  <danakj@chromium.org> wrote:
> On Tue, Nov 29, 2016 at 4:50 PM, <estade@chromium.org> wrote:
>>
>> On 2016/11/30 00:13:47, sky wrote:
>> > In thinking about this a bit more the only way to really address this
>> > is making a parent have a layer. I don't see any other way that
>> > doesn't have some limitation that is easy to miss.
>> >
>> > -Scott
>>
>> I think in the limit that means that all views will have layers as we add
>> more
>> and more layers for reasons like ink drop ripples or wanting shnazzy
>> animations,
>> and parent layers to effect clipping. This is a constant source of gotchas
>> because everything can seem to work fine until you discover some corner
>> case
>> where views start overlapping.
>
>
> The more layers there are the more memory we use also.

Is there a simple calculation for the cost of making a view have a
layer? I would imagine it depends upon the size of the view, but I'm
not positive there.

  -Scott

>
>>
>>
>> Anyway, I'm willing to set this aside and only come back to it if we have
>> to.
>>
>>
>> >
>> > On Tue, Nov 29, 2016 at 4:09 PM, <mailto:estade@chromium.org> wrote:
>> > > On 2016/11/29 20:27:09, sky wrote:
>> > >>
>> > >> https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc
>> > >> File ui/views/view.cc (right):
>> > >>
>> > >>
>> > >
>> >
>>
>>
https://codereview.chromium.org/2514303004/diff/20001/ui/views/view.cc#newcod...
>> > >> ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
>> > >> On 2016/11/29 18:38:01, Evan Stade wrote:
>> > >> > On 2016/11/29 18:08:13, sky wrote:
>> > >> > > Is this really enough? What happens if an animation occurs that
>> > > effectively
>> > >> > > moves the layer. Wouldn't the clip move with the layer, which
>> > >> > > isn't
>> > >> > > really
>> > >> > waht
>> > >> > > you want.
>> > >> >
>> > >> > when the layer is moved that affects the view's transform (as per
>> > >> GetTransform),
>> > >> > which is taken into account in GetVisibleBounds.
>> > >>
>> > >> That's assuming you end up here, right? If the layer is moved that is
>> > >> not
>> > >> necessarily the case.
>> > >
>> > > If that doesn't trigger a VisibleBoundsChanged notification (and it
>> > > looks
>> > > like
>> > > it may not) we might need to introduce
>> > > LayerDelegate::OnLayerTransformChanged,
>> > > similar to the currently extant OnLayerBoundsChanged.
>> > >
>> > > https://codereview.chromium.org/2514303004/
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "Chromium-reviews" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an
>> email
>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>> >
>>
>>
>>
>> https://codereview.chromium.org/2514303004/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2016-11-30 01:04:59]

On Tue, Nov 29, 2016 at 4:50 PM, <estade@chromium.org> wrote:

> On 2016/11/30 00:13:47, sky wrote:
> > In thinking about this a bit more the only way to really address this
> > is making a parent have a layer. I don't see any other way that
> > doesn't have some limitation that is easy to miss.
> >
> > -Scott
>
> I think in the limit that means that all views will have layers as we add
> more
> and more layers for reasons like ink drop ripples or wanting shnazzy
> animations,
> and parent layers to effect clipping. This is a constant source of gotchas
> because everything can seem to work fine until you discover some corner
> case
> where views start overlapping.
>

The more layers there are the more memory we use also.


>
> Anyway, I'm willing to set this aside and only come back to it if we have
> to.
>
>
> >
> > On Tue, Nov 29, 2016 at 4:09 PM, <mailto:estade@chromium.org> wrote:
> > > On 2016/11/29 20:27:09, sky wrote:
> > >> https://codereview.chromium.org/2514303004/diff/20001/ui/vie
> ws/view.cc
> > >> File ui/views/view.cc (right):
> > >>
> > >>
> > >
> >
> https://codereview.chromium.org/2514303004/diff/20001/ui/vie
> ws/view.cc#newcode826
> > >> ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
> > >> On 2016/11/29 18:38:01, Evan Stade wrote:
> > >> > On 2016/11/29 18:08:13, sky wrote:
> > >> > > Is this really enough? What happens if an animation occurs that
> > > effectively
> > >> > > moves the layer. Wouldn't the clip move with the layer, which
> isn't
> > >> > > really
> > >> > waht
> > >> > > you want.
> > >> >
> > >> > when the layer is moved that affects the view's transform (as per
> > >> GetTransform),
> > >> > which is taken into account in GetVisibleBounds.
> > >>
> > >> That's assuming you end up here, right? If the layer is moved that is
> not
> > >> necessarily the case.
> > >
> > > If that doesn't trigger a VisibleBoundsChanged notification (and it
> looks
> > > like
> > > it may not) we might need to introduce
> > > LayerDelegate::OnLayerTransformChanged,
> > > similar to the currently extant OnLayerBoundsChanged.
> > >
> > > https://codereview.chromium.org/2514303004/
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> >
>
>
>
> https://codereview.chromium.org/2514303004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2016-11-30 03:35:42]

On Tue, Nov 29, 2016 at 7:33 PM, <danakj@chromium.org> wrote:

> On Tue, Nov 29, 2016 at 4:59 PM, Scott Violet <sky@chromium.org> wrote:
>
>> On Tue, Nov 29, 2016 at 4:58 PM,  <danakj@chromium.org> wrote:
>> > On Tue, Nov 29, 2016 at 4:50 PM, <estade@chromium.org> wrote:
>> >>
>> >> On 2016/11/30 00:13:47, sky wrote:
>> >> > In thinking about this a bit more the only way to really address this
>> >> > is making a parent have a layer. I don't see any other way that
>> >> > doesn't have some limitation that is easy to miss.
>> >> >
>> >> > -Scott
>> >>
>> >> I think in the limit that means that all views will have layers as we
>> add
>> >> more
>> >> and more layers for reasons like ink drop ripples or wanting shnazzy
>> >> animations,
>> >> and parent layers to effect clipping. This is a constant source of
>> gotchas
>> >> because everything can seem to work fine until you discover some corner
>> >> case
>> >> where views start overlapping.
>> >
>> >
>> > The more layers there are the more memory we use also.
>>
>> Is there a simple calculation for the cost of making a view have a
>> layer? I would imagine it depends upon the size of the view, but I'm
>> not positive there.
>>
>
> Right, ignoring the possibility where a whole tile is a single color and
> we avoid this, each pixel of the layer takes up 4 bytes of memory. There's
> some additional data structure overhead but it's smaller unless your layers
> are like 2x2 or something :P  There's also complexity overhead as
> compositor algorithms are often O(...something .. #layers .. something...).
> And overdraw meaning more GL driver calls and memory bandwidth use, as each
> pixel will be drawn to N times when N layers overlap it (modulo culling
> when a layer or tile is fully hidden by an opaque thing above it).
>

^ All the above is only applying for layers with contents that draw. If
they are for book-keeping (clips or whatever), they have computational
complexity overhead and data structure bookkeeping overhead but no pixel
memory or raster time for those pixels, or GL drawing etc.


>
>
>>
>>   -Scott
>>
>> >
>> >>
>> >>
>> >> Anyway, I'm willing to set this aside and only come back to it if we
>> have
>> >> to.
>> >>
>> >>
>> >> >
>> >> > On Tue, Nov 29, 2016 at 4:09 PM, <mailto:estade@chromium.org> wrote:
>> >> > > On 2016/11/29 20:27:09, sky wrote:
>> >> > >>
>> >> > >> https://codereview.chromium.org/2514303004/diff/20001/ui/vie
>> ws/view.cc
>> >> > >> File ui/views/view.cc (right):
>> >> > >>
>> >> > >>
>> >> > >
>> >> >
>> >>
>> >> https://codereview.chromium.org/2514303004/diff/20001/ui/vie
>> ws/view.cc#newcode826
>> >> > >> ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
>> >> > >> On 2016/11/29 18:38:01, Evan Stade wrote:
>> >> > >> > On 2016/11/29 18:08:13, sky wrote:
>> >> > >> > > Is this really enough? What happens if an animation occurs
>> that
>> >> > > effectively
>> >> > >> > > moves the layer. Wouldn't the clip move with the layer, which
>> >> > >> > > isn't
>> >> > >> > > really
>> >> > >> > waht
>> >> > >> > > you want.
>> >> > >> >
>> >> > >> > when the layer is moved that affects the view's transform (as
>> per
>> >> > >> GetTransform),
>> >> > >> > which is taken into account in GetVisibleBounds.
>> >> > >>
>> >> > >> That's assuming you end up here, right? If the layer is moved
>> that is
>> >> > >> not
>> >> > >> necessarily the case.
>> >> > >
>> >> > > If that doesn't trigger a VisibleBoundsChanged notification (and it
>> >> > > looks
>> >> > > like
>> >> > > it may not) we might need to introduce
>> >> > > LayerDelegate::OnLayerTransformChanged,
>> >> > > similar to the currently extant OnLayerBoundsChanged.
>> >> > >
>> >> > > https://codereview.chromium.org/2514303004/
>> >> >
>> >> > --
>> >> > You received this message because you are subscribed to the Google
>> >> > Groups
>> >> > "Chromium-reviews" group.
>> >> > To unsubscribe from this group and stop receiving emails from it,
>> send
>> >> > an
>> >> email
>> >> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>> >> >
>> >>
>> >>
>> >>
>> >> https://codereview.chromium.org/2514303004/
>> >
>> >
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2016-11-30 03:40:31]

On Tue, Nov 29, 2016 at 4:59 PM, Scott Violet <sky@chromium.org> wrote:

> On Tue, Nov 29, 2016 at 4:58 PM,  <danakj@chromium.org> wrote:
> > On Tue, Nov 29, 2016 at 4:50 PM, <estade@chromium.org> wrote:
> >>
> >> On 2016/11/30 00:13:47, sky wrote:
> >> > In thinking about this a bit more the only way to really address this
> >> > is making a parent have a layer. I don't see any other way that
> >> > doesn't have some limitation that is easy to miss.
> >> >
> >> > -Scott
> >>
> >> I think in the limit that means that all views will have layers as we
> add
> >> more
> >> and more layers for reasons like ink drop ripples or wanting shnazzy
> >> animations,
> >> and parent layers to effect clipping. This is a constant source of
> gotchas
> >> because everything can seem to work fine until you discover some corner
> >> case
> >> where views start overlapping.
> >
> >
> > The more layers there are the more memory we use also.
>
> Is there a simple calculation for the cost of making a view have a
> layer? I would imagine it depends upon the size of the view, but I'm
> not positive there.
>

Right, ignoring the possibility where a whole tile is a single color and we
avoid this, each pixel of the layer takes up 4 bytes of memory. There's
some additional data structure overhead but it's smaller unless your layers
are like 2x2 or something :P  There's also complexity overhead as
compositor algorithms are often O(...something .. #layers .. something...).
And overdraw meaning more GL driver calls and memory bandwidth use, as each
pixel will be drawn to N times when N layers overlap it (modulo culling
when a layer or tile is fully hidden by an opaque thing above it).


>
>   -Scott
>
> >
> >>
> >>
> >> Anyway, I'm willing to set this aside and only come back to it if we
> have
> >> to.
> >>
> >>
> >> >
> >> > On Tue, Nov 29, 2016 at 4:09 PM, <mailto:estade@chromium.org> wrote:
> >> > > On 2016/11/29 20:27:09, sky wrote:
> >> > >>
> >> > >> https://codereview.chromium.org/2514303004/diff/20001/ui/vie
> ws/view.cc
> >> > >> File ui/views/view.cc (right):
> >> > >>
> >> > >>
> >> > >
> >> >
> >>
> >> https://codereview.chromium.org/2514303004/diff/20001/ui/vie
> ws/view.cc#newcode826
> >> > >> ui/views/view.cc:826: clip_recorder.ClipRect(GetVisibleBounds());
> >> > >> On 2016/11/29 18:38:01, Evan Stade wrote:
> >> > >> > On 2016/11/29 18:08:13, sky wrote:
> >> > >> > > Is this really enough? What happens if an animation occurs that
> >> > > effectively
> >> > >> > > moves the layer. Wouldn't the clip move with the layer, which
> >> > >> > > isn't
> >> > >> > > really
> >> > >> > waht
> >> > >> > > you want.
> >> > >> >
> >> > >> > when the layer is moved that affects the view's transform (as per
> >> > >> GetTransform),
> >> > >> > which is taken into account in GetVisibleBounds.
> >> > >>
> >> > >> That's assuming you end up here, right? If the layer is moved that
> is
> >> > >> not
> >> > >> necessarily the case.
> >> > >
> >> > > If that doesn't trigger a VisibleBoundsChanged notification (and it
> >> > > looks
> >> > > like
> >> > > it may not) we might need to introduce
> >> > > LayerDelegate::OnLayerTransformChanged,
> >> > > similar to the currently extant OnLayerBoundsChanged.
> >> > >
> >> > > https://codereview.chromium.org/2514303004/
> >> >
> >> > --
> >> > You received this message because you are subscribed to the Google
> >> > Groups
> >> > "Chromium-reviews" group.
> >> > To unsubscribe from this group and stop receiving emails from it, send
> >> > an
> >> email
> >> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> >> >
> >>
> >>
> >>
> >> https://codereview.chromium.org/2514303004/
> >
> >
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.