[Linux] Show preview contents for minimized windows

[Linux] Show preview contents for minimized windows

When minimizing a window, we hide the contents to prevent
unnecessary redrawing. However, this means that when the frame is
redrawn after the hiding, it is blank. Several desktop environments rely
on the window having contents when minimized, drawing the window
off-screen to be able to show preview content in overviews (Unity,
Gnome Shell).

This patch stops all drawing operations before hiding the
window, making sure that the content before hiding is retained and can
be shown by the window manager as a preview, but still avoiding unnecessary
redrawing of minimized windows.

BUG=156346
Committed: https://crrev.com/d2ff06116540022bb9d835eec843bcb70ada67d0
Cr-Commit-Position: refs/heads/master@{#293909}

[Arjan van Leeuwen, 2014-09-05 13:22:14]

arjanl@opera.com changed reviewers:
+ danakj@chromium.org, erg@chromium.org

[danakj, 2014-09-05 15:03:15]

https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc#...
ui/compositor/compositor.cc:296: void Compositor::SetVisible(bool visible) {
Why don't you just call this?

[Arjan van Leeuwen, 2014-09-05 15:18:04]

On 2014/09/05 15:03:15, danakj wrote:
> https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc
> File ui/compositor/compositor.cc (right):
> 
>
https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc#...
> ui/compositor/compositor.cc:296: void Compositor::SetVisible(bool visible) {
> Why don't you just call this?

It is :). SetVisible is (still) being called, but it is the SetVisible() call
that caused the bug in the first place. See the description of the change:

When minimizing a window, we hide the contents to prevent unnecessary redrawing
(this is the SetVisible call). However, this means that when the frame is
redrawn after the hiding, it is blank.

SetVisible(false) will cause a draw, which will paint a white background
(because the contents are hidden). The problem is that minimized windows can
still be visualized by previews from the window manager. The Block call is there
to make sure that the last painted buffer is being kept, not overwritten with a
blank one.

So we first block drawing calls, and then we call SetVisible(false). That way we
keep the optimization (we're not painting any new content while the window is
minimized), but we still see something useful when the window is being
visualized by the window manager to show a preview (the last content that was
drawn before the window was hidden instead of a blank screen).

If there are existing ways of keeping the last buffer (without introducing a new
function for blocking drawing calls) then I could use those instead, just didn't
see any.

[danakj, 2014-09-05 15:21:02]

On 2014/09/05 15:18:04, arjanl wrote:
> On 2014/09/05 15:03:15, danakj wrote:
> > https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc
> > File ui/compositor/compositor.cc (right):
> > 
> >
>
https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc#...
> > ui/compositor/compositor.cc:296: void Compositor::SetVisible(bool visible) {
> > Why don't you just call this?
> 
> It is :). SetVisible is (still) being called, but it is the SetVisible() call
> that caused the bug in the first place. See the description of the change:
> 
> When minimizing a window, we hide the contents to prevent unnecessary
redrawing
> (this is the SetVisible call). However, this means that when the frame is
> redrawn after the hiding, it is blank.
> 
> SetVisible(false) will cause a draw, which will paint a white background
> (because the contents are hidden). The problem is that minimized windows can
> still be visualized by previews from the window manager. The Block call is
there
> to make sure that the last painted buffer is being kept, not overwritten with
a
> blank one.
> 
> So we first block drawing calls, and then we call SetVisible(false). That way
we
> keep the optimization (we're not painting any new content while the window is
> minimized), but we still see something useful when the window is being
> visualized by the window manager to show a preview (the last content that was
> drawn before the window was hidden instead of a blank screen).
> 
> If there are existing ways of keeping the last buffer (without introducing a
new
> function for blocking drawing calls) then I could use those instead, just
didn't
> see any.

I'm confused, cc should not be drawing when it's not visible.

https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/single_th...
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/single_th...

So why would cc still composite?

[Arjan van Leeuwen, 2014-09-05 16:02:46]

On 2014/09/05 15:21:02, danakj wrote:
> On 2014/09/05 15:18:04, arjanl wrote:
> > On 2014/09/05 15:03:15, danakj wrote:
> > >
https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc
> > > File ui/compositor/compositor.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/542113004/diff/1/ui/compositor/compositor.cc#...
> > > ui/compositor/compositor.cc:296: void Compositor::SetVisible(bool visible)
{
> > > Why don't you just call this?
> > 
> > It is :). SetVisible is (still) being called, but it is the SetVisible()
call
> > that caused the bug in the first place. See the description of the change:
> > 
> > When minimizing a window, we hide the contents to prevent unnecessary
> redrawing
> > (this is the SetVisible call). However, this means that when the frame is
> > redrawn after the hiding, it is blank.
> > 
> > SetVisible(false) will cause a draw, which will paint a white background
> > (because the contents are hidden). The problem is that minimized windows can
> > still be visualized by previews from the window manager. The Block call is
> there
> > to make sure that the last painted buffer is being kept, not overwritten
with
> a
> > blank one.
> > 
> > So we first block drawing calls, and then we call SetVisible(false). That
way
> we
> > keep the optimization (we're not painting any new content while the window
is
> > minimized), but we still see something useful when the window is being
> > visualized by the window manager to show a preview (the last content that
was
> > drawn before the window was hidden instead of a blank screen).
> > 
> > If there are existing ways of keeping the last buffer (without introducing a
> new
> > function for blocking drawing calls) then I could use those instead, just
> didn't
> > see any.
> 
> I'm confused, cc should not be drawing when it's not visible.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/single_th...
>
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/single_th...
> 
> So why would cc still composite?

Hmm. I'll have to get back to you on Monday to see where exactly the draw is
coming from. From the top of my head all I can say is that it's drawing the
background color of the root host layer (which is the 'white' window you
currently see in previews in Unity and Gnome Shell for minimized windows).

[danakj, 2014-09-05 16:20:29]

Ok thanks, look forward to more details.


On Fri, Sep 5, 2014 at 12:02 PM, <arjanl@opera.com> wrote:

> On 2014/09/05 15:21:02, danakj wrote:
>
>> On 2014/09/05 15:18:04, arjanl wrote:
>> > On 2014/09/05 15:03:15, danakj wrote:
>> > >
>>
> https://codereview.chromium.org/542113004/diff/1/ui/
> compositor/compositor.cc
>
>> > > File ui/compositor/compositor.cc (right):
>> > >
>> > >
>> >
>>
>
> https://codereview.chromium.org/542113004/diff/1/ui/
> compositor/compositor.cc#newcode296
>
>> > > ui/compositor/compositor.cc:296: void Compositor::SetVisible(bool
>> visible)
>>
> {
>
>> > > Why don't you just call this?
>> >
>> > It is :). SetVisible is (still) being called, but it is the SetVisible()
>>
> call
>
>> > that caused the bug in the first place. See the description of the
>> change:
>> >
>> > When minimizing a window, we hide the contents to prevent unnecessary
>> redrawing
>> > (this is the SetVisible call). However, this means that when the frame
>> is
>> > redrawn after the hiding, it is blank.
>> >
>> > SetVisible(false) will cause a draw, which will paint a white background
>> > (because the contents are hidden). The problem is that minimized
>> windows can
>> > still be visualized by previews from the window manager. The Block call
>> is
>> there
>> > to make sure that the last painted buffer is being kept, not overwritten
>>
> with
>
>> a
>> > blank one.
>> >
>> > So we first block drawing calls, and then we call SetVisible(false).
>> That
>>
> way
>
>> we
>> > keep the optimization (we're not painting any new content while the
>> window
>>
> is
>
>> > minimized), but we still see something useful when the window is being
>> > visualized by the window manager to show a preview (the last content
>> that
>>
> was
>
>> > drawn before the window was hidden instead of a blank screen).
>> >
>> > If there are existing ways of keeping the last buffer (without
>> introducing a
>> new
>> > function for blocking drawing calls) then I could use those instead,
>> just
>> didn't
>> > see any.
>>
>
>  I'm confused, cc should not be drawing when it's not visible.
>>
>
>
> https://code.google.com/p/chromium/codesearch#chromium/
> src/cc/trees/single_thread_proxy.cc&l=478
>
> https://code.google.com/p/chromium/codesearch#chromium/
> src/cc/trees/single_thread_proxy.cc&rcl=1409711490&l=503
>
>  So why would cc still composite?
>>
>
> Hmm. I'll have to get back to you on Monday to see where exactly the draw
> is
> coming from. From the top of my head all I can say is that it's drawing the
> background color of the root host layer (which is the 'white' window you
> currently see in previews in Unity and Gnome Shell for minimized windows).
>
> https://codereview.chromium.org/542113004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Arjan van Leeuwen, 2014-09-08 11:44:52]

On 2014/09/05 16:20:29, danakj wrote:
> Ok thanks, look forward to more details.
> 
> 
> >  I'm confused, cc should not be drawing when it's not visible.
> >>
> >
> >
> > https://code.google.com/p/chromium/codesearch#chromium/
> > src/cc/trees/single_thread_proxy.cc&l=478
> >
> > https://code.google.com/p/chromium/codesearch#chromium/
> > src/cc/trees/single_thread_proxy.cc&rcl=1409711490&l=503
> >
> >  So why would cc still composite?
> >>
> >
> > Hmm. I'll have to get back to you on Monday to see where exactly the draw
> > is
> > coming from. From the top of my head all I can say is that it's drawing the
> > background color of the root host layer (which is the 'white' window you
> > currently see in previews in Unity and Gnome Shell for minimized windows).

Sorry, I didn't notice we were referring to different SetVisible() functions
(aura windows vs. compositor). The Compositor::SetVisible() function wasn't
available when I created the patch, so I didn't try calling that. It definitely
works as you said, so I'll update the patch to call that instead.

[danakj, 2014-09-08 16:33:29]

Thanks! LGTM tho not an owner

[Elliot Glaysher, 2014-09-08 18:12:37]

owners stamp lgtm

[Arjan van Leeuwen, 2014-09-09 06:36:17]

The CQ bit was checked by arjanl@opera.com

[commit-bot: I haz the power, 2014-09-09 06:40:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/arjanl@opera.com/542113004/20001

[commit-bot: I haz the power, 2014-09-09 09:40:52]

Committed patchset #2 (id:20001) as d2b596c1fa0501345eb0fc48b3ddf0b4b850bbb5

[commit-bot: I haz the power, 2014-09-10 03:52:25]

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