Merge lp:~vanvugt/unity/regionalDamage into lp:unity

Proposed by Daniel van Vugt on 2012-06-27
Status: Merged
Approved by: Tim Penhey on 2012-07-04
Approved revision: 2469
Merged at revision: 2470
Proposed branch: lp:~vanvugt/unity/regionalDamage
Merge into: lp:unity
Diff against target: 549 lines (+209/-74)
8 files modified
launcher/AbstractLauncherIcon.h (+1/-0)
launcher/Launcher.cpp (+11/-0)
launcher/Launcher.h (+4/-0)
launcher/LauncherIcon.cpp (+3/-0)
panel/PanelController.cpp (+15/-0)
panel/PanelController.h (+1/-0)
plugins/unityshell/src/unityshell.cpp (+166/-68)
plugins/unityshell/src/unityshell.h (+8/-6)
To merge this branch: bzr merge lp:~vanvugt/unity/regionalDamage
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Needs Information on 2012-07-06
Tim Penhey (community) 2012-06-27 Approve on 2012-07-04
Sam Spilsbury (community) 2012-06-27 Approve on 2012-06-27
jenkins (community) continuous-integration 2012-06-27 Approve on 2012-06-27
Daniel van Vugt Abstain on 2012-06-27
Review via email: mp+112283@code.launchpad.net

This proposal supersedes a proposal from 2012-06-26.

Commit message

Stop Unity from redrawing the shell on every frame (ie. when it doesn't need
to). It had a severe impact on graphics performance. (LP: #988079)

This especially improves OpenGL application performance and multi-monitor
desktop performance. Because unity was previously slowing down compiz
rendering by 20-40% for each monitor added to the system. This slowdown no
longer occurs as only damaged areas of the unity shell are repainted. Now
unity will not have any impact on compiz rendering performance for most
video frames.

Coincidentally, this fixes a number of other bugs (linked).

Description of the change

For me, this branch eliminates the 25-30% slowdown (compared to regular compiz) when unity is running with a single monitor. With 2 monitors, this branch eliminates the 75%(!) slowdown experienced when unity is running. Extrapolate and hypothesize as you like.

Even simple things like scrolling in a web browser are nicer with this branch. Scrolling is smoother and faster when you don't have to render the shell on every frame.

In multi-monitor tests, unity was slowing down my system to the point of only rendering 30 FPS with two monitors. That's half what it should be. With this branch, I get 60 FPS like I should.

WARNING: This fix can only work if no window is redrawing _under_ a panel or launcher. If you have something drawing under the panel, launcher, tooltip or quicklist, then the shell will need to redraw and you won't get any performance benefit. Fixing that is a much bigger job and should be considered separate.

To post a comment you must log in.
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

227 + /*
228 + * The shell is hidden if there exists any window that fully covers
229 + * the output and is in front of all Nux windows on that output.
230 + * We could also check CompositeWindow::opacity() but that would be slower
231 + * and almost always pointless.
232 + */
233 + if (w->isMapped() &&
234 + w->isViewable() &&
235 + !w->inShowDesktopMode() && // Why must this != isViewable?
236 + w->geometry().contains(output))

What might be faster here is using the occlusion detection passes from core. That will take into account whether or not the window is truly occluded. Usually you can get this by checking gw->clip () in GLWindow.

This code won't work for windows that are semitransparent or have an alpha channel, which will work a little weird.

Andrea Azzarone (azzar1) wrote : Posted in a previous version of this proposal

Does it fix bug 977984 too?

Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

I guess you can even remove this now

const std::string REDRAW_IDLE = "redraw-idle";

in: if (launcher_controller_.get()) no need for .get().

Overall looks good...

PS: pay attention to style, i.e.:
 bool UnityScreen::shellIsHidden(const CompOutput &output) -> bool UnityScreen::ShellIsHidden(CompOutput const& output)
 const std::vector<Window> &nuxwins(nux::XInputWindow::NativeHandleList()); -> std::vector<Window> const& nuxwins(nux::XInputWindow::NativeHandleList());

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Marco, I think your style is wrong :)
In C and C++, & and * are part of the variable, not part of the type:
    A b, *c, d, &e(b);
That is why they usually go next to the variable and not the type.

Also, the Google C++ style guide that Unity is meant to use agrees:
    http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Reference_Arguments
but then it allows both styles :)
    http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Reference_Expressions

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Andrea, no I don't think this will fix bug 977984.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Sam, gw->priv->clip only seems to get updated after glPaintOutputs which looks too late. How does one use clip() normally?

shellIsHidden is designed to be fast. And it is AFAIK. Yes, I did already add a comment about transparent windows. But we're only talking about fullscreen windows here. And I don't think the case of transparent or alpha fullscreen windows is one that we should need to waste time handling. If you make a fullscreen window transparent then you won't see the shell behind it. Does that matter?

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Actually, it does matter. The whole point of this proposal is to avoid rendering the shell because it hogs so much GPU. So I think we should not be rendering the shell if it is stacked below a transparent fullscreen window.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Just noticed a bug with NVIDIA:
Un-fullscreening a window does not damage the screen (!?) so the shell doesn't repaint until something changes or you use it. Not sure about the most elegant fix. It looks like a compiz bug, but I haven't yet proven it.

Daniel van Vugt (vanvugt) : Posted in a previous version of this proposal
review: Needs Fixing
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

> Marco, I think your style is wrong :)
> In C and C++, & and * are part of the variable, not part of the type:
> A b, *c, d, &e(b);
> That is why they usually go next to the variable and not the type.

I know, and that's how I used to do before, but the Unity style guide is different, see:
http://bazaar.launchpad.net/~unity-team/unity/trunk/view/head:/guides/cppguide.xml
(recently updated by Tim) http://bazaar.launchpad.net/~unity-team/unity/trunk/revision/2378

So here we use:
PointerType* ptr;
Object const& obj = ...;

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal
Download full text (4.2 KiB)

> Sam, gw->priv->clip only seems to get updated after glPaintOutputs which looks
> too late. How does one use clip() normally?

The clip region is updated after the occlusion pass. You are right, this happens within paintOutputRegion and not paintOutputs like I thought.

I feel like there are two options here:

1. Framebuffer object rebinds are not condition on whether or not the shell is repainting, but just as it is now with BackgroundEffectHelper::HasDirtyHelpers (). That leaves the problem of windows on top of the shell. At the moment, the damage collection code examines all damage events, but we don't care about stuff underneath the shell. As such, you could probably ignore any damage events coming in between PaintDisplay and glPaintOutput (damageRegionSetEnabled (false)). I don't think this will handle the case of fullscreen windows on top of the shell very well however. In compiz at least, we'd need a concept of layered damage, which is not trivial as it requires all damage to be tied to a particular object.

At this point, you can determine whether or not to paint the shell when you actually hit a nux window. This again, depends on being able to see nux windows in the paint stack (see lp:~smspillaz/unity/unity.less-paint-insanity). I guess at the moment, you could use the heuristic, and then Window -> CompWindow the nux window and check the clip region there to see if it is empty.

2. Second option is a bit trickier, but might also work. You'll need to effectively make unity do the occlusion pass. What you would do here is loop all the windows and do something like:

class UnityWindow :
....
{
....
private:

    CompRegion clip_;
};

void DetectOcclusionsOnUnityWindows (const GLMatrix &matrix,
                                     CompOutput *output)
{
GLWindowPaintAttrib a;
CompRegion tmpRegion (*output);

/* Top to bottom */
for (CompWindow *w : screen->reverseWindows ())
{
    UnityWindow *uw = UnityWindow::get (w);

    /* We only care about windows we actually clipped */
    clip = infiniteRegion;

    if (window->alpha ())
        continue;

    if (!window->isViewable ())
        continue;

    /* GLWindow::glPaint will return true if this window should be regarded
     * as occluding other windows. PAINT_WINDOW_OCCLUSION_DETECTION_MASK will
     * not actually paint the window, but will allow plugins to modify the state
     * of the window as if it was being painted, so we will know whether it should
     * occlude other windows */
    if (gw->glPaint (a, matrix, tmpRegion, PAINT_WINDOW_OCCLUSION_DETECTION_MASK))
        tmpRegion -= w->region (); // Not inputRect () or outputRect (): Decorations are ARGB windows

    uw->clip_ = tmpRegion;
}
}

Now, you can loop the list of nux windows (probably safe to store that internally, a nux window is a nux window for life, just detect it on the UnityWindow ctor) and see if they all have empty clip regions. If so, don't paint the shell.

You can also use the region detection to determine what parts of the shell to "damage" as well.

iirc, this is something similar to what the blur plugin does. Have a look at that for other ideas. This will also fix problems to do with ...

Read more...

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I am keeping shellIsHidden because it is fast and simple. It doesn't re-enter compiz and doesn't require any CompRegion logic. It just uses rectangles.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> I am keeping shellIsHidden because it is fast and simple. It doesn't re-enter
> compiz and doesn't require any CompRegion logic. It just uses rectangles.

My gripe with shellIsHidden is that it is inaccurate and will run into lots of edge cases. It doesn't make visual sense when you have windows with alpha regions, transparency or transformations preventing the shell from painting if the bounding area occludes the shell. Its pretty much defeats the purpose of having alpha regions, shape regions and transformations in the first place. You can easily run into cases where the shell won't paint even when it should, and I think that's bad. Unless you have a way to handle those cases, or a very strong argument as to why we shouldn't handle those cases, I honestly think its a blocking factor here.

The OpenGL plugin already uses similar looking logic to do its occlusion detection, and there is very little (if any) performance penalty for it. There is hardly any CompRegion allocation at all, because the variables are stored locally. In fact, every time I've callgrinded compiz, paintOutputRegion has not appeared as a hotspot anywhere. The re-entrancy into compiz is also very inexpensive because most plugins will have glPaint disabled if they are not running any animations.

I would suggest at least trying to make it work.

Other bits of review:

121 +std::vector<nux::View*> Controller::Impl::GetPanelViews() const
122 +{
123 + std::vector<nux::View*> views;
124 + views.reserve(windows_.size());
125 + for (auto window: windows_)
126 + views.push_back(ViewForWindow(window));
127 + return views;
128 +}
129 +

I think it probably makes more sense to have the panel controller keep track of the views in its own vector so that you can just return a const ref to the vector. Since we're going to be calling this function every time we call compizDamageNux, the cost of doing the initial tracking for outweighs the cost of the query here.

270 + w->isViewable() &&
271 + !w->inShowDesktopMode() && // Why must this != isViewable?

You probably wanted

w->state () & CompWindowStateHiddenMask

260 + // Loop through windows from back to front
261 + for (CompWindow* w : screen->windows ())
262 + {

If you still want to use this, it would be easier to go front to back, I had to read the code like 8 times to figure out how it worked.

// front to back
for (CompWindow *w : screen->reverseWindows ())
{
 if (w->isMapped() &&
  w->isViewable() &&
  !w->inShowDesktopMode() && // Why must this != isViewable?
  w->geometry().contains(output))
    return true; // a window that covers the whole output discovered at top first
 else
   for (Window id : nuxwins)
     if (w->id () == id && output->intersects (w->geometry ())
       return false; // a nux windows discovered first
}

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Sam, if you want people to pay attention, use fewer words. I'll get to this later.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> Sam, if you want people to pay attention, use fewer words. I'll get to this
> later.

I apologize for my verbosity. Some things are complicated. This is.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

ALPHA AND TRANSPARENCY
I can only find one edge case so far. Using the obs plugin make a fullscreen window translucent. I would like to be able to detect this using CompositeWindow opacity() and alpha(), but there are four problems with that:
  1. opacity() returns OPAQUE even after updateOpacity and even when the window is translucent. Bug?
  2. updateOpacity() triggers window damage. Not a good idea. Bug?
  3. updateOpacity() requires a round trip.
  4. alpha() is not implemented. Does it have a use?
Actually, obs doesn't use composite opacity at all. It stores opacity privately and I don't want to create a dependency of unityshell on obs. I know you've said the occlusion code is designed for all this, but I don't yet understand it. And I'm not going to use it until I understand what it's meant to do.

PANEL CONTROLLER
I would prefer to not modify the panel code at all. Also, I think the current design does the best at information hiding. I think it's only creating a vector that is typically size 1 or 2 anyway. So performance should not be an issue.

HIDDEN WINDOW DETECTION
Fixed as smspillaz suggested.

SHELLISHIDDEN
Fixed (simplified) as smspillaz suggested.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal
Download full text (3.9 KiB)

Looks pretty good now :)

427 + if (isNuxWindow(window))
428 + {
429 + if (mask & PAINT_WINDOW_OCCLUSION_DETECTION_MASK)
430 + {
431 + uScreen->nuxRegion += window->geometry();
432 + uScreen->nuxRegion -= uScreen->overShell;
433 + }
434 + return false;
435 + }
436 + else if (mask & PAINT_WINDOW_OCCLUSION_DETECTION_MASK &&
437 + !(mask & PAINT_WINDOW_TRANSLUCENT_MASK) &&
438 + window->state() & CompWindowStateFullscreenMask)
439 + // && !window->alpha() <-- doesn't work. False positives.
440 + {
441 + uScreen->overShell += window->geometry();
442 + uScreen->overShell -= uScreen->nuxRegion;
443 + }

I can't quite tell based on the context of the diff, but its probably safer to put this after the glPaint call, eg

bool retval = gWindow->glPaint (attrib, transform, region, mask);

And then something like:

bool nux_window = isNuxWindow (window);

if (mask & PAINT_WINDOW_OCCLUSION_DETECTION_MASK)
{
  if (nux_window)
  {
    uScreen->nuxRegion += window->geometry();
    uScreen->nuxRegion -= uScreen->overShell;
  }
  else
  {
    if (window->alpha () && attrib.opacity != opaque)
      mask |= PAINT_WINDOW_TRANSLUCENT_MASK;

    unsigned int noOcclusionMask = PAINT_WINDOW_TRANSLUCENT_MASK | PAINT_WINDOW_TRANSFORMED_MASK | PAINT_WINDOW_NO_CORE_INSTANCE_MASK;

    if (!(mask & noOcclusionMask == noOcclusionMask))
    {
      uScreen->overShell += window->geometry();
      uScreen->overShell -= uScreen->nuxRegion;
    }
  }
}

if (nux_window)
  return false;

return retval;

In that sample, I didn't check to see if the window was a fullscreen window before adding it to the occlusion region. Reason being that it is possible that a nonfullscreen window can be validly stacked above the shell (think override redirect windows, onboard, some other corner cases involving fullscreen windows and nonfullscreen windows). Note also the flags for NO_CORE_INSTANCE_MASK and TRANSFORMED_MASK.

I think the code here for determining whether or not something is over the shell using the occlusion detection is the wrong way around. The occlusion pass happens from bottom to top, so what seems to happen is that for windows stacked top to bottom as such (nux windows prefixed with N)

N1 > N2 > N3 > 4 > 5 > 6

You'll end up with:

overShell = (6 + 5 + 4)
nuxRegion = ((N3 - overShell) + (N2 - overShell) + (N1 - overShell))

Since when we hit the isNuxWindow case at N3, we add N3 to nuxRegion, and then subtract overShell from it (even though those windows aren't actually over the shell).

If you have no windows underneath the shell and a fullscreen window on top like this:

F1 > N2 > N3 > N4 > 5 > 6 > 7

You'll end up with:

nuxRegion = N4 + N3 + N2
overShell = (7 + 6 + 5 + F1) - nuxRegion

So in that case, you'll have a fullscreen window over the shell, and the shell will paint. If you don't have a fullscreen window over the shell, then the parts underneath it will obscure the shell.

The solution in this case is probably to only start to collect the shell region, and clear the "overShell" region when you hit the first nux window in PAINT_WINDOW_OCCLUSION_DETECTION_MASK. Then once you hit another window, you can s...

Read more...

jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:2463
http://s-jenkins:8080/job/unity-ci/21/

review: Approve (continuous-integration)
Daniel van Vugt (vanvugt) :
review: Abstain
jenkins (martin-mrazik+qa) wrote :

PASSED: Continuous integration, rev:2469
http://s-jenkins:8080/job/unity-ci/29/

review: Approve (continuous-integration)
Sam Spilsbury (smspillaz) wrote :

Okay, looks good.

I would suggest getting some of the more complicated parts of this code under test, but I'm happy with it as is.

review: Approve
Tim Penhey (thumper) wrote :

> std::vector<nux::View*> const& panels(panel_controller_->GetPanelViews());

This is very bad. You are taking a reference to a temporary.
Remove the const& and the standard library move semantics will make
sure that this is efficient.

Also there isn't really a need to get the list outside the range based for loop.

You could just say:

for (nux::View* view : panel_controller_->GetPanelViews())

Similarly for the other for loops.

> std::vector<nux::Geometry> const& dirty = wt->GetDrawList();

Again here you are taking a reference to a temporary.

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

Those temporaries exist for the lifetime of the block they're created in, so it is safe.

Tim Penhey (thumper) wrote :

You are half right.

Temporaries exist for the duration of the expression they were created in.

Except if they are bound to a reference, in which case they live for the duration of the life of the reference, except in certain situation covered in section 12.2.5 of the standard.

I learned something today.

review: Approve
Daniel van Vugt (vanvugt) wrote :

Yes, I was only half right. I learned something too.

Daniel d'Andrada (dandrader) wrote :

It seems that this patch has caused a regression.

Use case:
 1 - have number of windows open
 2 - hold alt key
 3 - press tab key [switcher comes up]
 4 - click on some unselected icon

Expected outcome:
 Selected icon is instantly redrawn

Actual outcome:
 It frequently take several seconds for the switcher to be redrawn after SwitcherView calls QueueDraw()

Branches:
  lp:~dandrader/unity/switcher_click_test
  This this patch and the issue appears.

  lp:~dandrader/unity/switcher_click_test_older_trunk
  Does not have this patch and things work nice and dandy.

Background info:
I'm working on new gestures to switch between windows and interact with the window switcher (the alt-tab guy). The branches above have a snippet of my WIP code that enables a user to select a window by directly tapping/clicking over its icon on the switcher.

So is my code snipped doing something dumb or have this patch indeed caused a regresssion?

review: Needs Information
Daniel d'Andrada (dandrader) wrote :

Things came back to normal again with revision 2475.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/AbstractLauncherIcon.h'
2--- launcher/AbstractLauncherIcon.h 2012-06-06 15:40:19 +0000
3+++ launcher/AbstractLauncherIcon.h 2012-06-27 07:28:39 +0000
4@@ -218,6 +218,7 @@
5
6 sigc::signal<void, AbstractLauncherIcon::Ptr> needs_redraw;
7 sigc::signal<void, AbstractLauncherIcon::Ptr> remove;
8+ sigc::signal<void, nux::ObjectPtr<nux::View>> tooltip_visible;
9 sigc::signal<void> visibility_changed;
10
11 sigc::connection needs_redraw_connection;
12
13=== modified file 'launcher/Launcher.cpp'
14--- launcher/Launcher.cpp 2012-06-20 18:51:16 +0000
15+++ launcher/Launcher.cpp 2012-06-27 07:28:39 +0000
16@@ -1520,6 +1520,11 @@
17 }
18 }
19
20+nux::ObjectPtr<nux::View> Launcher::GetActiveTooltip() const
21+{
22+ return _active_tooltip;
23+}
24+
25 void Launcher::SetActionState(LauncherActionState actionstate)
26 {
27 if (_launcher_action_state == actionstate)
28@@ -1681,6 +1686,7 @@
29 EnsureAnimation();
30
31 icon->needs_redraw.connect(sigc::mem_fun(this, &Launcher::OnIconNeedsRedraw));
32+ icon->tooltip_visible.connect(sigc::mem_fun(this, &Launcher::OnTooltipVisible));
33 }
34
35 void Launcher::OnIconRemoved(AbstractLauncherIcon::Ptr icon)
36@@ -1757,6 +1763,11 @@
37 EnsureAnimation();
38 }
39
40+void Launcher::OnTooltipVisible(nux::ObjectPtr<nux::View> view)
41+{
42+ _active_tooltip = view;
43+}
44+
45 void Launcher::Draw(nux::GraphicsEngine& GfxContext, bool force_draw)
46 {
47
48
49=== modified file 'launcher/Launcher.h'
50--- launcher/Launcher.h 2012-06-15 02:03:31 +0000
51+++ launcher/Launcher.h 2012-06-27 07:28:39 +0000
52@@ -96,6 +96,8 @@
53 return _parent;
54 };
55
56+ nux::ObjectPtr<nux::View> GetActiveTooltip() const; // nullptr = no tooltip
57+
58 virtual void RecvMouseUp(int x, int y, unsigned long button_flags, unsigned long key_flags);
59 virtual void RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags);
60 virtual void RecvMouseDrag(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags);
61@@ -269,6 +271,7 @@
62 void OnOrderChanged();
63
64 void OnIconNeedsRedraw(AbstractLauncherIcon::Ptr icon);
65+ void OnTooltipVisible(nux::ObjectPtr<nux::View> view);
66
67 void OnOverlayHidden(GVariant* data);
68 void OnOverlayShown(GVariant* data);
69@@ -311,6 +314,7 @@
70
71 LauncherModel::Ptr _model;
72 nux::BaseWindow* _parent;
73+ nux::ObjectPtr<nux::View> _active_tooltip;
74 QuicklistView* _active_quicklist;
75
76 nux::HLayout* m_Layout;
77
78=== modified file 'launcher/LauncherIcon.cpp'
79--- launcher/LauncherIcon.cpp 2012-06-19 01:00:36 +0000
80+++ launcher/LauncherIcon.cpp 2012-06-27 07:28:39 +0000
81@@ -513,6 +513,7 @@
82 LoadTooltip();
83 _tooltip->ShowTooltipWithTipAt(tip_x, tip_y);
84 _tooltip->ShowWindow(!tooltip_text().empty());
85+ tooltip_visible.emit(_tooltip);
86 }
87
88 void
89@@ -534,6 +535,7 @@
90
91 if (_tooltip)
92 _tooltip->ShowWindow(false);
93+ tooltip_visible.emit(nux::ObjectPtr<nux::View>(nullptr));
94 }
95
96 bool LauncherIcon::OpenQuicklist(bool select_first_item, int monitor)
97@@ -653,6 +655,7 @@
98 {
99 if (_tooltip)
100 _tooltip->ShowWindow(false);
101+ tooltip_visible.emit(nux::ObjectPtr<nux::View>(nullptr));
102 }
103
104 bool
105
106=== modified file 'panel/PanelController.cpp'
107--- panel/PanelController.cpp 2012-06-15 02:03:31 +0000
108+++ panel/PanelController.cpp 2012-06-27 07:28:39 +0000
109@@ -49,6 +49,7 @@
110 void QueueRedraw();
111
112 std::vector<Window> GetTrayXids() const;
113+ std::vector<nux::View*> GetPanelViews() const;
114 std::vector<nux::Geometry> GetGeometries() const;
115
116 // NOTE: nux::Property maybe?
117@@ -106,6 +107,15 @@
118 return xids;
119 }
120
121+std::vector<nux::View*> Controller::Impl::GetPanelViews() const
122+{
123+ std::vector<nux::View*> views;
124+ views.reserve(windows_.size());
125+ for (auto window: windows_)
126+ views.push_back(ViewForWindow(window));
127+ return views;
128+}
129+
130 std::vector<nux::Geometry> Controller::Impl::GetGeometries() const
131 {
132 std::vector<nux::Geometry> geometries;
133@@ -325,6 +335,11 @@
134 return pimpl->GetTrayXids();
135 }
136
137+std::vector<nux::View*> Controller::GetPanelViews() const
138+{
139+ return pimpl->GetPanelViews();
140+}
141+
142 std::vector<nux::Geometry> Controller::GetGeometries() const
143 {
144 return pimpl->GetGeometries();
145
146=== modified file 'panel/PanelController.h'
147--- panel/PanelController.h 2012-05-06 23:48:38 +0000
148+++ panel/PanelController.h 2012-06-27 07:28:39 +0000
149@@ -41,6 +41,7 @@
150 void QueueRedraw();
151
152 std::vector<Window> GetTrayXids() const;
153+ std::vector<nux::View*> GetPanelViews() const;
154 std::vector<nux::Geometry> GetGeometries() const;
155
156 // NOTE: nux::Property maybe?
157
158=== modified file 'plugins/unityshell/src/unityshell.cpp'
159--- plugins/unityshell/src/unityshell.cpp 2012-06-26 18:34:03 +0000
160+++ plugins/unityshell/src/unityshell.cpp 2012-06-27 07:28:39 +0000
161@@ -100,7 +100,6 @@
162 const unsigned int SCROLL_UP_BUTTON = 7;
163
164 const std::string RELAYOUT_TIMEOUT = "relayout-timeout";
165-const std::string REDRAW_IDLE = "redraw-idle";
166 } // namespace local
167 } // anon namespace
168
169@@ -118,6 +117,7 @@
170 , super_keypressed_(false)
171 , newFocusedWindow(nullptr)
172 , doShellRepaint(false)
173+ , didShellRepaint(false)
174 , allowWindowPaint(false)
175 , damaged(false)
176 , _key_nav_mode_requested(false)
177@@ -909,6 +909,7 @@
178 }
179
180 doShellRepaint = false;
181+ didShellRepaint = true;
182 damaged = false;
183 }
184
185@@ -1211,7 +1212,15 @@
186 {
187 bool ret;
188
189- doShellRepaint = true;
190+ /*
191+ * Very important!
192+ * Don't waste GPU and CPU rendering the shell on every frame if you don't
193+ * need to. Doing so on every frame causes Nux to hog the GPU and slow down
194+ * ALL rendering. (LP: #988079)
195+ */
196+ bool force = forcePaintOnTop() || PluginAdapter::Default()->IsExpoActive();
197+ doShellRepaint = force || !(region.isEmpty() || wt->GetDrawList().empty());
198+
199 allowWindowPaint = true;
200 _last_output = output;
201 paint_panel_ = false;
202@@ -1227,13 +1236,21 @@
203 * attempts to bind it will only increment
204 * its bind reference so make sure that
205 * you always unbind as much as you bind */
206- _fbo->bind (nux::Geometry (output->x (), output->y (), output->width (), output->height ()));
207+ if (doShellRepaint)
208+ _fbo->bind (nux::Geometry (output->x (), output->y (), output->width (), output->height ()));
209 #endif
210
211+ // CompRegion has no clear() method. So this is the fastest alternative.
212+ aboveShell = CompRegion();
213+ nuxRegion = CompRegion();
214+
215 /* glPaintOutput is part of the opengl plugin, so we need the GLScreen base class. */
216 ret = gScreen->glPaintOutput(attrib, transform, region, output, mask);
217
218 #ifndef USE_MODERN_COMPIZ_GL
219+ if (doShellRepaint && !force && aboveShell.contains(*output))
220+ doShellRepaint = false;
221+
222 if (doShellRepaint)
223 paintDisplay(region, transform, mask);
224 #endif
225@@ -1286,16 +1303,32 @@
226 for (ShowdesktopHandlerWindowInterface *wi : ShowdesktopHandler::animating_windows)
227 wi->HandleAnimations (ms);
228
229+ // Workaround Nux bug LP: #1014610:
230 if (damaged)
231 {
232 damaged = false;
233- damageNuxRegions();
234+ nuxDamageCompiz();
235 }
236
237+ compizDamageNux(cScreen->currentDamage());
238+
239+ didShellRepaint = false;
240 }
241
242 void UnityScreen::donePaint()
243 {
244+ /*
245+ * It's only safe to clear the draw list if drawing actually occurred
246+ * (i.e. the shell was not obscured behind a fullscreen window).
247+ * If you clear the draw list and drawing has not occured then you'd be
248+ * left with all your views thinking they're queued for drawing still and
249+ * would refuse to redraw when you return from fullscreen.
250+ * I think this is a Nux bug. ClearDrawList should ideally also mark all
251+ * the queued views as draw_cmd_queued_=false.
252+ */
253+ if (didShellRepaint)
254+ wt->ClearDrawList();
255+
256 std::list <ShowdesktopHandlerWindowInterface *> remove_windows;
257
258 for (ShowdesktopHandlerWindowInterface *wi : ShowdesktopHandler::animating_windows)
259@@ -1316,42 +1349,100 @@
260 cScreen->donePaint ();
261 }
262
263+void UnityScreen::compizDamageNux(CompRegion const& damage)
264+{
265+ if (!launcher_controller_)
266+ return;
267+
268+ CompRect::vector const& rects(damage.rects());
269+ for (const CompRect &r : rects)
270+ {
271+ nux::Geometry geo(r.x(), r.y(), r.width(), r.height());
272+ BackgroundEffectHelper::ProcessDamage(geo);
273+ }
274+
275+ auto launchers = launcher_controller_->launchers();
276+ for (auto launcher : launchers)
277+ {
278+ if (!launcher->Hidden())
279+ {
280+ nux::Geometry geo = launcher->GetAbsoluteGeometry();
281+ CompRegion launcher_region(geo.x, geo.y, geo.width, geo.height);
282+ if (damage.intersects(launcher_region))
283+ launcher->QueueDraw();
284+ nux::ObjectPtr<nux::View> tooltip = launcher->GetActiveTooltip();
285+ if (!tooltip.IsNull())
286+ {
287+ nux::Geometry tip = tooltip->GetAbsoluteGeometry();
288+ CompRegion tip_region(tip.x, tip.y, tip.width, tip.height);
289+ if (damage.intersects(tip_region))
290+ tooltip->QueueDraw();
291+ }
292+ }
293+ }
294+
295+ std::vector<nux::View*> const& panels(panel_controller_->GetPanelViews());
296+ for (nux::View* view : panels)
297+ {
298+ nux::Geometry geo = view->GetAbsoluteGeometry();
299+ CompRegion panel_region(geo.x, geo.y, geo.width, geo.height);
300+ if (damage.intersects(panel_region))
301+ view->QueueDraw();
302+ }
303+
304+ QuicklistManager* qm = QuicklistManager::Default();
305+ if (qm)
306+ {
307+ QuicklistView* view = qm->Current();
308+ if (view)
309+ {
310+ nux::Geometry geo = view->GetAbsoluteGeometry();
311+ CompRegion quicklist_region(geo.x, geo.y, geo.width, geo.height);
312+ if (damage.intersects(quicklist_region))
313+ view->QueueDraw();
314+ }
315+ }
316+}
317+
318 /* Grab changed nux regions and add damage rects for them */
319-void UnityScreen::damageNuxRegions()
320+void UnityScreen::nuxDamageCompiz()
321 {
322+ // Workaround Nux bug LP: #1014610 (unbounded DrawList growth)
323+ // Also, ensure we don't dereference null *controller_ on startup.
324+ if (damaged || !launcher_controller_ || !dash_controller_)
325+ return;
326+ damaged = true;
327+
328 CompRegion nux_damage;
329
330- if (damaged)
331- return;
332-
333- std::vector<nux::Geometry> dirty = wt->GetDrawList();
334- damaged = true;
335-
336- for (std::vector<nux::Geometry>::iterator it = dirty.begin(), end = dirty.end();
337- it != end; ++it)
338- {
339- nux::Geometry const& geo = *it;
340- nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
341- }
342-
343- nux::Geometry geo = wt->GetWindowCompositor().GetTooltipMainWindowGeometry();
344- nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
345-
346- geo = lastTooltipArea;
347- nux_damage += CompRegion(lastTooltipArea.x, lastTooltipArea.y,
348- lastTooltipArea.width, lastTooltipArea.height);
349-
350- /*
351- * Avoid Nux damaging Nux as recommended by smspillaz. Though I don't
352- * believe it would be harmful or significantly expensive right now.
353- */
354+ std::vector<nux::Geometry> const& dirty = wt->GetDrawList();
355+ for (auto geo : dirty)
356+ nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
357+
358+ if (launcher_controller_->IsOverlayOpen())
359+ {
360+ nux::BaseWindow* dash_window = dash_controller_->window();
361+ nux::Geometry const& geo = dash_window->GetAbsoluteGeometry();
362+ nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
363+ }
364+
365+ auto launchers = launcher_controller_->launchers();
366+ for (auto launcher : launchers)
367+ {
368+ if (!launcher->Hidden())
369+ {
370+ nux::ObjectPtr<nux::View> tooltip = launcher->GetActiveTooltip();
371+ if (!tooltip.IsNull())
372+ {
373+ nux::Geometry const& g = tooltip->GetAbsoluteGeometry();
374+ nux_damage += CompRegion(g.x, g.y, g.width, g.height);
375+ }
376+ }
377+ }
378+
379 cScreen->damageRegionSetEnabled(this, false);
380 cScreen->damageRegion(nux_damage);
381 cScreen->damageRegionSetEnabled(this, true);
382-
383- wt->ClearDrawList();
384-
385- lastTooltipArea = geo;
386 }
387
388 /* handle X Events */
389@@ -1511,13 +1602,7 @@
390
391 void UnityScreen::damageRegion(const CompRegion &region)
392 {
393- const CompRect::vector &rects(region.rects());
394- for (const CompRect &r : rects)
395- {
396- nux::Geometry geo(r.x(), r.y(), r.width(), r.height());
397- BackgroundEffectHelper::ProcessDamage(geo);
398- }
399-
400+ compizDamageNux(region);
401 cScreen->damageRegion(region);
402 }
403
404@@ -2173,14 +2258,6 @@
405 return false;
406 }
407
408-const CompWindowList& UnityScreen::getWindowPaintList()
409-{
410- CompWindowList& pl = _withRemovedNuxWindows = cScreen->getWindowPaintList();
411- pl.remove_if(isNuxWindow);
412-
413- return pl;
414-}
415-
416 void UnityScreen::RaiseInputWindows()
417 {
418 std::vector<Window> const& xwns = nux::XInputWindow::NativeHandleList();
419@@ -2205,6 +2282,35 @@
420 const CompRegion& region,
421 unsigned int mask)
422 {
423+ /*
424+ * The occlusion pass tests windows from TOP to BOTTOM. That's opposite to
425+ * the actual painting loop.
426+ */
427+ if (isNuxWindow(window))
428+ {
429+ if (mask & PAINT_WINDOW_OCCLUSION_DETECTION_MASK)
430+ {
431+ uScreen->nuxRegion += window->geometry();
432+ uScreen->nuxRegion -= uScreen->aboveShell;
433+ }
434+ return false; // Ensure nux windows are never painted by compiz
435+ }
436+ else if (mask & PAINT_WINDOW_OCCLUSION_DETECTION_MASK)
437+ {
438+ static const unsigned int nonOcclusionBits =
439+ PAINT_WINDOW_TRANSLUCENT_MASK |
440+ PAINT_WINDOW_TRANSFORMED_MASK |
441+ PAINT_WINDOW_NO_CORE_INSTANCE_MASK;
442+ if (!(mask & nonOcclusionBits))
443+ // And I've been advised to test other things, but they don't work:
444+ // && (attrib.opacity == OPAQUE)) <-- Doesn't work; Only set in glDraw
445+ // && !window->alpha() <-- Doesn't work; Opaque windows often have alpha
446+ {
447+ uScreen->aboveShell += window->geometry();
448+ uScreen->aboveShell -= uScreen->nuxRegion;
449+ }
450+ }
451+
452 GLWindowPaintAttrib wAttrib = attrib;
453
454 if (mMinimizeHandler)
455@@ -2263,7 +2369,17 @@
456 }
457 }
458
459- if (uScreen->doShellRepaint && !uScreen->forcePaintOnTop ())
460+ /*
461+ * Paint the shell in *roughly* the compiz stacking order. This is only
462+ * approximate because we're painting all the nux windows as soon as we find
463+ * the bottom-most nux window (from bottom to top).
464+ * But remember to avoid painting the shell if it's within the aboveShell
465+ * region.
466+ */
467+ if (uScreen->doShellRepaint &&
468+ !uScreen->forcePaintOnTop () &&
469+ !uScreen->aboveShell.contains(window->geometry())
470+ )
471 {
472 std::vector<Window> const& xwns = nux::XInputWindow::NativeHandleList();
473 unsigned int size = xwns.size();
474@@ -2578,25 +2694,7 @@
475
476 void UnityScreen::onRedrawRequested()
477 {
478- // disable blur updates so we dont waste perf. This can stall the blur during animations
479- // but ensures a smooth animation.
480- if (_in_paint)
481- {
482- if (!sources_.GetSource(local::REDRAW_IDLE))
483- {
484- auto redraw_idle(std::make_shared<glib::Idle>(glib::Source::Priority::DEFAULT));
485- sources_.Add(redraw_idle, local::REDRAW_IDLE);
486-
487- redraw_idle->Run([&]() {
488- onRedrawRequested();
489- return false;
490- });
491- }
492- }
493- else
494- {
495- damageNuxRegions();
496- }
497+ nuxDamageCompiz();
498 }
499
500 /* Handle option changes and plug that into nux windows */
501
502=== modified file 'plugins/unityshell/src/unityshell.h'
503--- plugins/unityshell/src/unityshell.h 2012-06-26 17:34:31 +0000
504+++ plugins/unityshell/src/unityshell.h 2012-06-27 07:28:39 +0000
505@@ -133,9 +133,6 @@
506 CompOutput*,
507 unsigned int);
508
509- /* Pop our InputOutput windows from the paint list */
510- const CompWindowList& getWindowPaintList();
511-
512 /* handle X11 events */
513 void handleEvent(XEvent*);
514
515@@ -212,7 +209,10 @@
516
517 bool initPluginActions();
518 void initLauncher();
519- void damageNuxRegions();
520+
521+ void compizDamageNux(CompRegion const& region);
522+ void nuxDamageCompiz();
523+
524 void onRedrawRequested();
525 void Relayout();
526
527@@ -260,7 +260,6 @@
528 bool enable_shortcut_overlay_;
529
530 GestureEngine gesture_engine_;
531- nux::Geometry lastTooltipArea;
532 bool needsRelayout;
533 bool _in_paint;
534 bool super_keypressed_;
535@@ -277,11 +276,14 @@
536
537 /* handle paint order */
538 bool doShellRepaint;
539+ bool didShellRepaint;
540 bool allowWindowPaint;
541 bool damaged;
542 bool _key_nav_mode_requested;
543 CompOutput* _last_output;
544- CompWindowList _withRemovedNuxWindows;
545+
546+ CompRegion nuxRegion;
547+ CompRegion aboveShell;
548
549 nux::Property<nux::Geometry> primary_monitor_;
550