Ensure Surface size always matches window size on swap

Ensure Surface size always matches window size on swap

D3D11 scales the backbuffer contents if a swap happens while the window is being resized, so do a Finish when we get a notification the window size is changing, and wait until a Surface is received at the new size before swapping again.

Committed: https://crrev.com/616238a39895ecac9f6434194c75c32c3785298f
Cr-Commit-Position: refs/heads/master@{#306537}

[jbauman, 2014-12-01 23:02:48]

jbauman@chromium.org changed reviewers:
+ jamesr@chromium.org, piman@chromium.org

[piman, 2014-12-01 23:42:09]

LGTM for ui/compositor and content/

https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
cc/surfaces/display.cc:140: DidSwapBuffersComplete();
Do we need to respect order wrt surface callbacks below?

https://codereview.chromium.org/767443002/diff/20001/ui/compositor/compositor.h
File ui/compositor/compositor.h (right):

https://codereview.chromium.org/767443002/diff/20001/ui/compositor/compositor...
ui/compositor/compositor.h:190: // Disable swapping on this surface until it is
resized.
Nit: "Finishes all outstanding rendering and disable swapping...."

[jamesr, 2014-12-02 00:04:17]

https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
cc/surfaces/display.cc:62: renderer_->Finish();
add a comment here indicating why we need to finish()

we only really need to do this on windows, right? could we make this
configurable so we don't pay this cost on other platforms?

https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
cc/surfaces/display.cc:122: if (!frame_data->render_pass_list.empty())
hmm, maybe Aggregate() should return a null frame if it ended up with an empty
pass list

https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
cc/surfaces/display.cc:149: surface->RunDrawCallbacks();
we probably don't want to run these if we didn't actually draw, right?

[jbauman, 2014-12-02 23:16:47]

On 2014/12/01 23:42:09, piman (Very slow to review) wrote:
> LGTM for ui/compositor and content/
> 
> https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc
> File cc/surfaces/display.cc (right):
> 
>
https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
> cc/surfaces/display.cc:140: DidSwapBuffersComplete();
> Do we need to respect order wrt surface callbacks below?

Not at the moment, but I've moved the DidSwapBuffersComplete to happen after the
draw callbacks.
> 
>
https://codereview.chromium.org/767443002/diff/20001/ui/compositor/compositor.h
> File ui/compositor/compositor.h (right):
> 
>
https://codereview.chromium.org/767443002/diff/20001/ui/compositor/compositor...
> ui/compositor/compositor.h:190: // Disable swapping on this surface until it
is
> resized.
> Nit: "Finishes all outstanding rendering and disable swapping...."

Fixed.

[jbauman, 2014-12-02 23:25:10]

On 2014/12/02 00:04:17, jamesr wrote:
> https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc
> File cc/surfaces/display.cc (right):
> 
>
https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
> cc/surfaces/display.cc:62: renderer_->Finish();
> add a comment here indicating why we need to finish()
> 
> we only really need to do this on windows, right? could we make this
> configurable so we don't pay this cost on other platforms?

Ok, added a new flag to RendererSettings and only set it on windows.
> 
>
https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
> cc/surfaces/display.cc:122: if (!frame_data->render_pass_list.empty())
> hmm, maybe Aggregate() should return a null frame if it ended up with an empty
> pass list

Ok, changed the code to do that.
> 
>
https://codereview.chromium.org/767443002/diff/20001/cc/surfaces/display.cc#n...
> cc/surfaces/display.cc:149: surface->RunDrawCallbacks();
> we probably don't want to run these if we didn't actually draw, right?

Unfortunately we need to ack the previous frames or the browser compositor will
be swap throttled and won't swap new frames of the correct size.

[jamesr, 2014-12-03 00:21:43]

lgtm

[jbauman, 2014-12-03 01:26:49]

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

[jbauman, 2014-12-03 01:26:50]

sky@: ui/views OWNERS review.

[sky, 2014-12-03 02:27:56]

views LGTM

[jbauman, 2014-12-03 02:28:19]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2014-12-03 02:30:01]

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

[commit-bot: I haz the power, 2014-12-03 03:18:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-03 03:19:52]

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