[Aura] Fix for a flash of black frame when resizing with the new resize logic.

[Aura] Fix for a flash of black frame when resizing with the new resize logic.

BUG=124671, 161008
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192950

[Ben Goodger (Google), 2013-04-08 21:20:06]

lgtm

[commit-bot: I haz the power, 2013-04-08 21:23:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skaslev@chromium.org/13560016/1

[commit-bot: I haz the power, 2013-04-08 23:39:05]

Change committed as 192950

[piman, 2013-04-09 00:11:49]

https://chromiumcodereview.appspot.com/13560016/diff/1/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://chromiumcodereview.appspot.com/13560016/diff/1/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:761:
previous_damage_.op(RectToSkIRect(gfx::Rect(size)), SkRegion::kUnion_Op);
Why is this needed?
When we resize, the new frame should have full damage. We DCHECK for it. So the
previous damage should not have any effect.

[slavi, 2013-04-09 00:24:14]

On 2013/04/09 00:11:49, piman wrote:
>
https://chromiumcodereview.appspot.com/13560016/diff/1/content/browser/render...
> File content/browser/renderer_host/render_widget_host_view_aura.cc (right):
> 
>
https://chromiumcodereview.appspot.com/13560016/diff/1/content/browser/render...
> content/browser/renderer_host/render_widget_host_view_aura.cc:761:
> previous_damage_.op(RectToSkIRect(gfx::Rect(size)), SkRegion::kUnion_Op);
> Why is this needed?
> When we resize, the new frame should have full damage. We DCHECK for it. So
the
> previous damage should not have any effect.

Here's a sketch of the issue: After resize, the content_layer get's resized and
cleared. If I haven't received a new frame in SwapSoftwareFrame, in OnPaint only
the previous_damage_ will be painted, since it doesn't know that the
content_layer just got cleared and the whole frame should be repainted. 

The issue was very apparent with the new faster resize logic by Carlos -- the
first frame after resize was always black (or almost black). This patch fixes
the issue.

[piman, 2013-04-09 00:37:49]

On Mon, Apr 8, 2013 at 5:24 PM, <skaslev@chromium.org> wrote:

> On 2013/04/09 00:11:49, piman wrote:
>
> https://chromiumcodereview.**appspot.com/13560016/diff/1/**
>
content/browser/renderer_host/**render_widget_host_view_aura.**cc<https://chromiumcodereview.appspot.com/13560016/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc>
>
>> File content/browser/renderer_host/**render_widget_host_view_aura.**cc
>> (right):
>>
>
>
> https://chromiumcodereview.**appspot.com/13560016/diff/1/**
> content/browser/renderer_host/**render_widget_host_view_aura.**
>
cc#newcode761<https://chromiumcodereview.appspot.com/13560016/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc#newcode761>
>
>> content/browser/renderer_host/**render_widget_host_view_aura.**cc:761:
>> previous_damage_.op(**RectToSkIRect(gfx::Rect(size))**,
>> SkRegion::kUnion_Op);
>> Why is this needed?
>> When we resize, the new frame should have full damage. We DCHECK for it.
>> So
>>
> the
>
>> previous damage should not have any effect.
>>
>
> Here's a sketch of the issue: After resize, the content_layer get's
> resized and
> cleared. If I haven't received a new frame in SwapSoftwareFrame, in
> OnPaint only
> the previous_damage_ will be painted, since it doesn't know that the
> content_layer just got cleared and the whole frame should be repainted.
>

Hmm, I think the problem is here.
We can't choose to only paint previous_damage_, because the layer can cause
any arbitrary invalidation. I should have paid more attention to that on
the previous CL. OnPaint will be called with the proper clip rect set, so
that it's perfectly fine to paint the entire bitmap (Skia will clip
correctly and do the minimal amount of work). Also, it's not quite right to
reset previous_damage_ in OnPaint - OnPaint can be called multiple times
for a given renderer frame.

Also, I think there's confusion about what previous_damage_ means.
previous_damage_ is used for GL partial swaps, which assumes
double-buffering, so that we can copy to the new front buffer only the area
that was changed between 2 frames ago and the previous frame, that hasn't
change between the previous frame and the new one - i.e. for the case of a
constant damage rect, copying nothing.
You're implementing something different for software - that's fine - but
you shouldn't be modifying the behavior of the GL partial swaps. I'm not
sure SetSize isn't called redundantly, for example.


>
> The issue was very apparent with the new faster resize logic by Carlos --
> the
> first frame after resize was always black (or almost black). This patch
> fixes
> the issue.
>
>
https://codereview.chromium.**org/13560016/<https://codereview.chromium.org/1...
>