Merge lp:~vanvugt/unity/regionalDamage into lp:unity
- regionalDamage
- Merge into trunk
Status: | Merged | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Approved by: | Tim Penhey | ||||||||||||||||||||||||||||
Approved revision: | no longer in the source branch. | ||||||||||||||||||||||||||||
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 | ||||||||||||||||||||||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel d'Andrada (community) | Needs Information | ||
Tim Penhey (community) | Approve | ||
Sam Spilsbury (community) | Approve | ||
jenkins (community) | continuous-integration | Approve | |
Daniel van Vugt | Abstain | ||
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.
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal | # |
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_
Overall looks good...
PS: pay attention to style, i.e.:
bool UnityScreen:
const std::vector<Window> &nuxwins(
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://
but then it allows both styles :)
http://
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 | # |
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://
(recently updated by Tim) http://
So here we use:
PointerType* ptr;
Object const& obj = ...;
Sam Spilsbury (smspillaz) 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?
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 BackgroundEffec
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 DetectOcclusion
{
GLWindowPaintAttrib a;
CompRegion tmpRegion (*output);
/* Top to bottom */
for (CompWindow *w : screen-
{
UnityWindow *uw = UnityWindow::get (w);
/* We only care about windows we actually clipped */
clip = infiniteRegion;
if (window->alpha ())
continue;
if (!window-
continue;
/* GLWindow::glPaint will return true if this window should be regarded
* as occluding other windows. PAINT_WINDOW_
* 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_
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 ...
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<
122 +{
123 + std::vector<
124 + views.reserve(
125 + for (auto window: windows_)
126 + views.push_
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->inShowDeskt
You probably wanted
w->state () & CompWindowState
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-
{
if (w->isMapped() &&
w->isViewable() &&
!w->inShowDes
w->geometry(
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
}
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 | # |
Looks pretty good now :)
427 + if (isNuxWindow(
428 + {
429 + if (mask & PAINT_WINDOW_
430 + {
431 + uScreen->nuxRegion += window->geometry();
432 + uScreen->nuxRegion -= uScreen->overShell;
433 + }
434 + return false;
435 + }
436 + else if (mask & PAINT_WINDOW_
437 + !(mask & PAINT_WINDOW_
438 + window->state() & CompWindowState
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_
{
if (nux_window)
{
uScreen-
uScreen-
}
else
{
if (window->alpha () && attrib.opacity != opaque)
mask |= PAINT_WINDOW_
unsigned int noOcclusionMask = PAINT_WINDOW_
if (!(mask & noOcclusionMask == noOcclusionMask))
{
uScreen-
uScreen-
}
}
}
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_
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_
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:2463
http://
Daniel van Vugt (vanvugt) : | # |
jenkins (martin-mrazik+qa) wrote : | # |
PASSED: Continuous integration, rev:2469
http://
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.
Tim Penhey (thumper) wrote : | # |
> std::vector<
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_controlle
Similarly for the other for loops.
> std::vector<
Again here you are taking a reference to a temporary.
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.
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?
Daniel d'Andrada (dandrader) wrote : | # |
Things came back to normal again with revision 2475.
Preview Diff
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 ®ion) |
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 |
227 + /* ::opacity( ) but that would be slower opMode( ) && // Why must this != isViewable? ).contains( output) )
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
231 + * and almost always pointless.
232 + */
233 + if (w->isMapped() &&
234 + w->isViewable() &&
235 + !w->inShowDeskt
236 + w->geometry(
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.