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

[Daniel van Vugt]

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
frames.

Coincidentally, this also fixes LP: #967112 and LP: #992516. Maybe more...

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.

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)]

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)]

Does it fix bug 977984 too?

[Marco Trevisan (Treviño) (3v1n0)]

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)]

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)]

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

[Daniel van Vugt (vanvugt)]

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)]

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)]

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)]

review: Needs Fixing

[Marco Trevisan (Treviño) (3v1n0)]

> 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)]

> 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 ...

=== modified file 'launcher/AbstractLauncherIcon.h'
--- launcher/AbstractLauncherIcon.h	2012-06-06 15:40:19 +0000
+++ launcher/AbstractLauncherIcon.h	2012-06-22 06:31:22 +0000
@@ -218,6 +218,7 @@
 
   sigc::signal<void, AbstractLauncherIcon::Ptr> needs_redraw;
   sigc::signal<void, AbstractLauncherIcon::Ptr> remove;
+  sigc::signal<void, nux::ObjectPtr<nux::View>> tooltip_visible;
   sigc::signal<void> visibility_changed;
 
   sigc::connection needs_redraw_connection;

=== modified file 'launcher/Launcher.cpp'
--- launcher/Launcher.cpp	2012-06-20 18:51:16 +0000
+++ launcher/Launcher.cpp	2012-06-22 06:31:22 +0000
@@ -1520,6 +1520,11 @@
   }
 }
 
+nux::ObjectPtr<nux::View> Launcher::GetActiveTooltip() const
+{
+  return _active_tooltip;
+}
+
 void Launcher::SetActionState(LauncherActionState actionstate)
 {
   if (_launcher_action_state == actionstate)
@@ -1681,6 +1686,7 @@
   EnsureAnimation();
 
   icon->needs_redraw.connect(sigc::mem_fun(this, &Launcher::OnIconNeedsRedraw));
+  icon->tooltip_visible.connect(sigc::mem_fun(this, &Launcher::OnTooltipVisible));
 }
 
 void Launcher::OnIconRemoved(AbstractLauncherIcon::Ptr icon)
@@ -1757,6 +1763,11 @@
   EnsureAnimation();
 }
 
+void Launcher::OnTooltipVisible(nux::ObjectPtr<nux::View> view)
+{
+  _active_tooltip = view;
+}
+
 void Launcher::Draw(nux::GraphicsEngine& GfxContext, bool force_draw)
 {
 

=== modified file 'launcher/Launcher.h'
--- launcher/Launcher.h	2012-06-15 02:03:31 +0000
+++ launcher/Launcher.h	2012-06-22 06:31:22 +0000
@@ -96,6 +96,8 @@
     return _parent;
   };
 
+  nux::ObjectPtr<nux::View> GetActiveTooltip() const;  // nullptr = no tooltip
+
   virtual void RecvMouseUp(int x, int y, unsigned long button_flags, unsigned long key_flags);
   virtual void RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags);
   virtual void RecvMouseDrag(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags);
@@ -269,6 +271,7 @@
   void OnOrderChanged();
 
   void OnIconNeedsRedraw(AbstractLauncherIcon::Ptr icon);
+  void OnTooltipVisible(nux::ObjectPtr<nux::View> view);
 
   void OnOverlayHidden(GVariant* data);
   void OnOverlayShown(GVariant* data);
@@ -311,6 +314,7 @@
 
   LauncherModel::Ptr _model;
   nux::BaseWindow* _parent;
+  nux::ObjectPtr<nux::View> _active_tooltip;
   QuicklistView* _active_quicklist;
 
   nux::HLayout* m_Layout;

=== modified file 'launcher/LauncherIcon.cpp'
--- launcher/LauncherIcon.cpp	2012-06-19 01:00:36 +0000
+++ launcher/LauncherIcon.cpp	2012-06-22 06:31:22 +0000
@@ -513,6 +513,7 @@
     LoadTooltip();
   _tooltip->ShowTooltipWithTipAt(tip_x, tip_y);
   _tooltip->ShowWindow(!tooltip_text().empty());
+  tooltip_visible.emit(_tooltip);
 }
 
 void
@@ -534,6 +535,7 @@
 
   if (_tooltip)
     _tooltip->ShowWindow(false);
+  tooltip_visible.emit(nux::ObjectPtr<nux::View>(nullptr));
 }
 
 bool LauncherIcon::OpenQuicklist(bool select_first_item, int monitor)
@@ -653,6 +655,7 @@
 {
   if (_tooltip)
     _tooltip->ShowWindow(false);
+  tooltip_visible.emit(nux::ObjectPtr<nux::View>(nullptr));
 }
 
 bool

=== modified file 'panel/PanelController.cpp'
--- panel/PanelController.cpp	2012-06-15 02:03:31 +0000
+++ panel/PanelController.cpp	2012-06-22 06:31:22 +0000
@@ -49,6 +49,7 @@
   void QueueRedraw();
 
   std::vector<Window> GetTrayXids() const;
+  std::vector<nux::View*> GetPanelViews() const;
   std::vector<nux::Geometry> GetGeometries() const;
 
   // NOTE: nux::Property maybe?
@@ -106,6 +107,15 @@
   return xids;
 }
 
+std::vector<nux::View*> Controller::Impl::GetPanelViews() const
+{
+  std::vector<nux::View*> views;
+  views.reserve(windows_.size());
+  for (auto window: windows_)
+    views.push_back(ViewForWindow(window));
+  return views;
+}
+
 std::vector<nux::Geometry> Controller::Impl::GetGeometries() const
 {
   std::vector<nux::Geometry> geometries;
@@ -325,6 +335,11 @@
   return pimpl->GetTrayXids();
 }
 
+std::vector<nux::View*> Controller::GetPanelViews() const
+{
+  return pimpl->GetPanelViews();
+}
+
 std::vector<nux::Geometry> Controller::GetGeometries() const
 {
   return pimpl->GetGeometries();

=== modified file 'panel/PanelController.h'
--- panel/PanelController.h	2012-05-06 23:48:38 +0000
+++ panel/PanelController.h	2012-06-22 06:31:22 +0000
@@ -41,6 +41,7 @@
   void QueueRedraw();
 
   std::vector<Window> GetTrayXids() const;
+  std::vector<nux::View*> GetPanelViews() const;
   std::vector<nux::Geometry> GetGeometries() const;
 
   // NOTE: nux::Property maybe?

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-06-15 02:03:31 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-06-22 06:31:22 +0000
@@ -100,7 +100,6 @@
 const unsigned int SCROLL_UP_BUTTON = 7;
 
 const std::string RELAYOUT_TIMEOUT = "relayout-timeout";
-const std::string REDRAW_IDLE = "redraw-idle";
 } // namespace local
 } // anon namespace
 
@@ -118,6 +117,7 @@
   , super_keypressed_(false)
   , newFocusedWindow(nullptr)
   , doShellRepaint(false)
+  , didShellRepaint(false)
   , allowWindowPaint(false)
   , damaged(false)
   , _key_nav_mode_requested(false)
@@ -902,6 +902,7 @@
   }
 
   doShellRepaint = false;
+  didShellRepaint = true;
   damaged = false;
 }
 
@@ -1204,7 +1205,20 @@
 {
   bool ret;
 
-  doShellRepaint = true;
+  /*
+   * Very important!
+   * Don't waste GPU and CPU rendering the shell on every frame if you don't
+   * need to. Doing so on every frame causes Nux to hog the GPU and slow down
+   * all other OpenGL apps (LP: #988079)
+   */
+  if (forcePaintOnTop() || PluginAdapter::Default()->IsExpoActive())
+    doShellRepaint = true;
+  else if (region.isEmpty() || shellIsHidden(*output))
+    doShellRepaint = false;
+  else
+    doShellRepaint = wt->GetDrawList().size() > 0 ||
+                     BackgroundEffectHelper::HasDirtyHelpers();
+
   allowWindowPaint = true;
   _last_output = output;
   paint_panel_ = false;
@@ -1220,7 +1234,8 @@
    *   attempts to bind it will only increment
    *   its bind reference so make sure that
    *   you always unbind as much as you bind */
-  _fbo->bind (nux::Geometry (output->x (), output->y (), output->width (), output->height ()));
+  if (doShellRepaint)
+    _fbo->bind (nux::Geometry (output->x (), output->y (), output->width (), output->height ()));
 #endif
 
   /* glPaintOutput is part of the opengl plugin, so we need the GLScreen base class. */
@@ -1279,16 +1294,32 @@
   for (ShowdesktopHandlerWindowInterface *wi : ShowdesktopHandler::animating_windows)
     wi->HandleAnimations (ms);
 
+  // Workaround Nux bug LP: #1014610:
   if (damaged)
   {
     damaged = false;
-    damageNuxRegions();
+    nuxDamageCompiz();
   }
 
+  compizDamageNux(cScreen->currentDamage());
+
+  didShellRepaint = false;
 }
 
 void UnityScreen::donePaint()
 {
+  /*
+   * It's only safe to clear the draw list if drawing actually occurred
+   * (i.e. the shell was not obscured behind a fullscreen window).
+   * If you clear the draw list and drawing has not occured then you'd be
+   * left with all your views thinking they're queued for drawing still and
+   * would refuse to redraw when you return from fullscreen.
+   * I think this is a Nux bug. ClearDrawList should ideally also mark all
+   * the queued views as draw_cmd_queued_=false.
+   */
+  if (didShellRepaint)
+    wt->ClearDrawList();
+
   std::list <ShowdesktopHandlerWindowInterface *> remove_windows;
 
   for (ShowdesktopHandlerWindowInterface *wi : ShowdesktopHandler::animating_windows)
@@ -1309,42 +1340,137 @@
   cScreen->donePaint ();
 }
 
+bool UnityScreen::shellIsHidden(CompOutput const& output)
+{
+  bool hidden = false;
+  std::vector<Window> const& nuxwins(nux::XInputWindow::NativeHandleList());
+
+  // Loop through windows from back to front
+  for (CompWindow* w : screen->windows ())
+  {
+    /*
+     * The shell is hidden if there exists any window that fully covers
+     * the output and is in front of all Nux windows on that output.
+     * We could also check CompositeWindow::opacity() but that would be slower
+     * and almost always pointless.
+     */
+    if (w->isMapped() &&
+        w->isViewable() &&
+        !w->inShowDesktopMode() &&  // Why must this != isViewable?
+        w->geometry().contains(output))
+    {
+      hidden = true;
+    }
+    else if (hidden)
+    {
+      for (Window n : nuxwins)
+      {
+        if (w->id() == n && output.intersects(w->geometry()))
+        {
+          hidden = false;
+          break;
+        }
+      }
+    }
+  }
+
+  return hidden;
+}
+
+void UnityScreen::compizDamageNux(CompRegion const& damage)
+{
+  if (!launcher_controller_)
+    return;
+
+  CompRect::vector const& rects(damage.rects());
+  for (const CompRect &r : rects)
+  {
+    nux::Geometry geo(r.x(), r.y(), r.width(), r.height());
+    BackgroundEffectHelper::ProcessDamage(geo);
+  }
+
+  auto launchers = launcher_controller_->launchers();
+  for (auto launcher : launchers)
+  {
+    if (!launcher->Hidden())
+    {
+      nux::Geometry geo = launcher->GetAbsoluteGeometry();
+      CompRegion launcher_region(geo.x, geo.y, geo.width, geo.height);
+      if (damage.intersects(launcher_region))
+        launcher->QueueDraw();
+      nux::ObjectPtr<nux::View> tooltip = launcher->GetActiveTooltip();
+      if (!tooltip.IsNull())
+      {
+        nux::Geometry tip = tooltip->GetAbsoluteGeometry();
+        CompRegion tip_region(tip.x, tip.y, tip.width, tip.height);
+        if (damage.intersects(tip_region))
+          tooltip->QueueDraw();
+      }
+    }
+  }
+
+  std::vector<nux::View*> const& panels(panel_controller_->GetPanelViews());
+  for (nux::View* view : panels)
+  {
+    nux::Geometry geo = view->GetAbsoluteGeometry();
+    CompRegion panel_region(geo.x, geo.y, geo.width, geo.height);
+    if (damage.intersects(panel_region))
+      view->QueueDraw();
+  }
+
+  QuicklistManager* qm = QuicklistManager::Default();
+  if (qm)
+  {
+    QuicklistView* view = qm->Current();
+    if (view)
+    {
+      nux::Geometry geo = view->GetAbsoluteGeometry();
+      CompRegion quicklist_region(geo.x, geo.y, geo.width, geo.height);
+      if (damage.intersects(quicklist_region))
+        view->QueueDraw();
+    }
+  }
+}
+
 /* Grab changed nux regions and add damage rects for them */
-void UnityScreen::damageNuxRegions()
+void UnityScreen::nuxDamageCompiz()
 {
+  // Workaround Nux bug LP: #1014610 (unbounded DrawList growth)
+  // Also, ensure we don't dereference null *controller_ on startup.
+  if (damaged || !launcher_controller_ || !dash_controller_)
+    return;
+  damaged = true;
+
   CompRegion nux_damage;
 
-  if (damaged)
-    return;
-
-  std::vector<nux::Geometry> dirty = wt->GetDrawList();
-  damaged = true;
-
-  for (std::vector<nux::Geometry>::iterator it = dirty.begin(), end = dirty.end();
-       it != end; ++it)
-  {
-    nux::Geometry const& geo = *it;
-    nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
-  }
-
-  nux::Geometry geo = wt->GetWindowCompositor().GetTooltipMainWindowGeometry();
-  nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
-
-  geo = lastTooltipArea;
-  nux_damage += CompRegion(lastTooltipArea.x, lastTooltipArea.y,
-                         lastTooltipArea.width, lastTooltipArea.height);
-
-  /*
-   * Avoid Nux damaging Nux as recommended by smspillaz. Though I don't
-   * believe it would be harmful or significantly expensive right now.
-   */
+  std::vector<nux::Geometry> const& dirty = wt->GetDrawList();
+  for (auto geo : dirty)
+    nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
+
+  if (launcher_controller_->IsOverlayOpen())
+  {
+    nux::BaseWindow* dash_window = dash_controller_->window();
+    nux::Geometry const& geo = dash_window->GetAbsoluteGeometry();
+    nux_damage += CompRegion(geo.x, geo.y, geo.width, geo.height);
+  }
+
+  auto launchers = launcher_controller_->launchers();
+  for (auto launcher : launchers)
+  {
+    if (!launcher->Hidden())
+    {
+      nux::ObjectPtr<nux::View> tooltip = launcher->GetActiveTooltip();
+      if (!tooltip.IsNull())
+      {
+        nux::Geometry const& g = tooltip->GetAbsoluteGeometry();
+        nux_damage += CompRegion(g.x, g.y, g.width, g.height);
+      }
+    }
+  }
+
   cScreen->damageRegionSetEnabled(this, false);
   cScreen->damageRegion(nux_damage);
   cScreen->damageRegionSetEnabled(this, true);
-
-  wt->ClearDrawList();
-
-  lastTooltipArea = geo;
 }
 
 /* handle X Events */
@@ -1504,13 +1630,7 @@
 
 void UnityScreen::damageRegion(const CompRegion &region)
 {
-  const CompRect::vector &rects(region.rects());
-  for (const CompRect &r : rects)
-  {
-    nux::Geometry geo(r.x(), r.y(), r.width(), r.height());
-    BackgroundEffectHelper::ProcessDamage(geo);
-  }
-
+  compizDamageNux(region);
   cScreen->damageRegion(region);
 }
 
@@ -2527,25 +2647,7 @@
 
 void UnityScreen::onRedrawRequested()
 {
-  // disable blur updates so we dont waste perf. This can stall the blur during animations
-  // but ensures a smooth animation.
-  if (_in_paint)
-  {
-    if (!sources_.GetSource(local::REDRAW_IDLE))
-    {
-      auto redraw_idle(std::make_shared<glib::Idle>(glib::Source::Priority::DEFAULT));
-      sources_.Add(redraw_idle, local::REDRAW_IDLE);
-
-      redraw_idle->Run([&]() {
-        onRedrawRequested();
-        return false;
-      });
-    }
-  }
-  else
-  {
-    damageNuxRegions();
-  }
+  nuxDamageCompiz();
 }
 
 /* Handle option changes and plug that into nux windows */

=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2012-06-15 02:03:31 +0000
+++ plugins/unityshell/src/unityshell.h	2012-06-22 06:31:22 +0000
@@ -212,7 +212,12 @@
 
   bool initPluginActions();
   void initLauncher();
-  void damageNuxRegions();
+
+  void compizDamageNux(CompRegion const& region);
+  void nuxDamageCompiz();
+
+  bool shellIsHidden(CompOutput const& output);
+
   void onRedrawRequested();
   void Relayout();
 
@@ -255,7 +260,6 @@
   bool enable_shortcut_overlay_;
 
   GestureEngine                         gesture_engine_;
-  nux::Geometry                         lastTooltipArea;
   bool                                  needsRelayout;
   bool                                  _in_paint;
   bool                                  super_keypressed_;
@@ -272,6 +276,7 @@
 
   /* handle paint order */
   bool    doShellRepaint;
+  bool    didShellRepaint;
   bool    allowWindowPaint;
   bool    damaged;
   bool    _key_nav_mode_requested;