exo: Allow updating cc::Surface viewport and crop state independently.

exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer. If
the viewport is changed but a new buffer isn't attached then the old
resource information will be re-sent with the new Surface. The
SurfaceFactory will handle refcounting uses of the buffer.

Committed: https://crrev.com/2fdc0733cd2b8ff82f6af591d480c930571aa46c
Cr-Commit-Position: refs/heads/master@{#398179}

[jbauman, 2016-06-02 21:25:28]

Description was changed from

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer.

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

to

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer.
==========

[jbauman, 2016-06-02 21:26:03]

jbauman@chromium.org changed reviewers:
+ reveman@chromium.org

[reveman, 2016-06-02 21:48:30]

https://codereview.chromium.org/2032173002/diff/1/components/exo/surface.cc
File components/exo/surface.cc (right):

https://codereview.chromium.org/2032173002/diff/1/components/exo/surface.cc#n...
components/exo/surface.cc:537: texture_mailbox_release_callback =
current_buffer_->ProduceTextureMailbox(
It's considered a client error to commit the same buffer when already used.
ProduceTextureMailbox changes the behavior of buffer and logs warnings when this
happens. It looks like this will now happen when a client is just changing the
viewport or crop state. Can we change this state without having to produce a new
texture mailbox somehow?

[jbauman, 2016-06-03 00:07:44]

Description was changed from

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer.
==========

to

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer. If
the viewport is changed but a new buffer isn't attached then the old
resource information will be re-sent with the new Surface. The
SurfaceFactory will handle refcounting uses of the buffer.
==========

[jbauman, 2016-06-03 00:09:05]

PTAL.

On 2016/06/02 21:48:30, reveman wrote:
> https://codereview.chromium.org/2032173002/diff/1/components/exo/surface.cc
> File components/exo/surface.cc (right):
> 
>
https://codereview.chromium.org/2032173002/diff/1/components/exo/surface.cc#n...
> components/exo/surface.cc:537: texture_mailbox_release_callback =
> current_buffer_->ProduceTextureMailbox(
> It's considered a client error to commit the same buffer when already used.
> ProduceTextureMailbox changes the behavior of buffer and logs warnings when
this
> happens. It looks like this will now happen when a client is just changing the
> viewport or crop state. Can we change this state without having to produce a
new
> texture mailbox somehow?

Good point. Added support for holding on to the old texture mailbox in this
case.

[reveman, 2016-06-03 19:06:51]

https://codereview.chromium.org/2032173002/diff/20001/components/exo/surface.cc
File components/exo/surface.cc (right):

https://codereview.chromium.org/2032173002/diff/20001/components/exo/surface....
components/exo/surface.cc:553: if (has_pending_contents_ ||
changed_viewport_or_crop_) {
Do we need this if statement and the new changed_viewport_or_crop_ member? It's
rare that we get a commit but haven't changed contents or clipping and I'm not
sure it's worth optimizing for it at this time.

https://codereview.chromium.org/2032173002/diff/20001/components/exo/surface....
components/exo/surface.cc:667: if (current_resource_.id)
nit: please include this in the DCHECK, DCHECK(!current_resource_.id || ...

[jbauman, 2016-06-07 00:09:58]

PTAL.

On 2016/06/03 19:06:51, reveman wrote:
>
https://codereview.chromium.org/2032173002/diff/20001/components/exo/surface.cc
> File components/exo/surface.cc (right):
> 
>
https://codereview.chromium.org/2032173002/diff/20001/components/exo/surface....
> components/exo/surface.cc:553: if (has_pending_contents_ ||
> changed_viewport_or_crop_) {
> Do we need this if statement and the new changed_viewport_or_crop_ member?
It's
> rare that we get a commit but haven't changed contents or clipping and I'm not
> sure it's worth optimizing for it at this time.

Ok, Done.
> 
>
https://codereview.chromium.org/2032173002/diff/20001/components/exo/surface....
> components/exo/surface.cc:667: if (current_resource_.id)
> nit: please include this in the DCHECK, DCHECK(!current_resource_.id || ...

Done.

[reveman, 2016-06-07 00:25:54]

lgtm

[jbauman, 2016-06-07 00:50:44]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2016-06-07 00:51:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2032173002/80001

[commit-bot: I haz the power, 2016-06-07 00:55:47]

Description was changed from

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer. If
the viewport is changed but a new buffer isn't attached then the old
resource information will be re-sent with the new Surface. The
SurfaceFactory will handle refcounting uses of the buffer.
==========

to

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer. If
the viewport is changed but a new buffer isn't attached then the old
resource information will be re-sent with the new Surface. The
SurfaceFactory will handle refcounting uses of the buffer.
==========

[commit-bot: I haz the power, 2016-06-07 00:55:48]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-07 00:57:20]

Description was changed from

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer. If
the viewport is changed but a new buffer isn't attached then the old
resource information will be re-sent with the new Surface. The
SurfaceFactory will handle refcounting uses of the buffer.
==========

to

==========
exo: Allow updating cc::Surface viewport and crop state independently.

This was added for the texture-layer code but not the surface layer. If
the viewport is changed but a new buffer isn't attached then the old
resource information will be re-sent with the new Surface. The
SurfaceFactory will handle refcounting uses of the buffer.

Committed: https://crrev.com/2fdc0733cd2b8ff82f6af591d480c930571aa46c
Cr-Commit-Position: refs/heads/master@{#398179}
==========

[commit-bot: I haz the power, 2016-06-07 00:57:21]

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