Merge lp:~3v1n0/unity/shadows-on-existing-pixmaps-cleanup into lp:unity

[Marco Trevisan (Treviño)]

DecoratedWindow: Cleanup shadows for shaped windows, reduce recomputation

[Andrea Azzarone (azzar1)]

The changes to the prerequiste make sense to me.

review: Approve

[lp:~3v1n0/unity/shadows-on-existing-pixmaps-cleanup]

4086. By Marco Trevisan (Treviño) on 2016-07-26

Merging with parent branch

4087. By Marco Trevisan (Treviño) on 2016-07-26

DecoratedWindow: don't try to draw an invalid texture

Fixes crash on shaped client unmap

4088. By Marco Trevisan (Treviño) on 2016-07-27

DecoratedWindow: don't mark an unmapped window as dirty geo

This was causing a new paint on a window that was about to be destroyed
with invalid id, and thus with no shape informations

4089. By Marco Trevisan (Treviño) on 2016-07-27

UnityWindow: undecorate it when about to be destroyed

=== modified file 'decorations/DecoratedWindow.cpp'
--- decorations/DecoratedWindow.cpp	2016-07-27 10:19:12 +0000
+++ decorations/DecoratedWindow.cpp	2016-07-27 10:19:13 +0000
@@ -31,8 +31,6 @@
 #include "WindowManager.h"
 #include "UnitySettings.h"
 
-#include <X11/extensions/shape.h>
-
 namespace unity
 {
 namespace decoration
@@ -40,6 +38,7 @@
 namespace
 {
 const std::string MENUS_PANEL_NAME = "WindowLIM";
+const int SHADOW_BLUR_MARGIN_FACTOR = 2;
 }
 
 Window::Impl::Impl(Window* parent, CompWindow* win)
@@ -504,13 +503,21 @@
   return (win_->frame() || win_->hasUnmapReference()) && FullyDecorated();
 }
 
+bool Window::Impl::IsRectangular() const
+{
+  return win_->region().numRects() == 1;
+}
+
 GLTexture* Window::Impl::ShadowTexture() const
 {
+  if (!IsRectangular())
+    return shaped_shadow_pixmap_->texture();
+
   auto const& mi = manager_->impl_;
   if (active() || parent_->scaled())
-    return win_->region().numRects() > 1 ? shaped_shadow_pixmap_->texture() : mi->active_shadow_pixmap_->texture();
+    return mi->active_shadow_pixmap_->texture();
 
-  return win_->region().numRects() > 1 ? shaped_shadow_pixmap_->texture() : mi->inactive_shadow_pixmap_->texture();
+  return mi->inactive_shadow_pixmap_->texture();
 }
 
 unsigned Window::Impl::ShadowRadius() const
@@ -678,6 +685,26 @@
   }
 }
 
+cu::PixmapTexture::Ptr Window::Impl::BuildShapedShadowTexture(nux::Size const& size, unsigned radius, nux::Color const& color, Shape const& shape) {
+  nux::CairoGraphics img(CAIRO_FORMAT_ARGB32, size.width, size.height);
+  auto* img_ctx = img.GetInternalContext();
+
+  for (auto const& rect : shape.GetRectangles())
+  {
+    cairo_rectangle(img_ctx, rect.x + radius * SHADOW_BLUR_MARGIN_FACTOR - shape.XOffset(), rect.y + radius * SHADOW_BLUR_MARGIN_FACTOR - shape.YOffset(), rect.width, rect.height);
+    cairo_set_source_rgba(img_ctx, color.red, color.green, color.blue, color.alpha);
+    cairo_fill(img_ctx);
+  }
+
+  img.BlurSurface(radius);
+
+  cu::CairoContext shadow_ctx(size.width, size.height);
+  cairo_set_source_surface(shadow_ctx, img.GetSurface(), 0, 0);
+  cairo_paint(shadow_ctx);
+
+  return shadow_ctx;
+}
+
 void Window::Impl::ComputeShapedShadowQuad()
 {
   if (!(deco_elements_ & cu::DecorationElement::SHADOW))
@@ -690,23 +717,26 @@
 
   nux::Color color = active() ? manager_->active_shadow_color() : manager_->inactive_shadow_color();
   unsigned int radius = active() ? manager_->active_shadow_radius() : manager_->inactive_shadow_radius();
-  DecorationsShape shape;
-  shape.initShape(win_->id());
-  shaped_shadow_pixmap_ = manager_->impl_->BuildShapedShadowTexture(radius, color, shape);
-
-  const auto* texture = ShadowTexture();
+
+  Shape shape(win_->id());
+  auto const& border = win_->borderRect();
+  auto const& shadow_offset = manager_->shadow_offset();
+
+  // Ideally it would be shape.getWidth + radius * 2 but Cairographics::BlurSurface
+  // isn't bounded by the radius and we need to compensate by using a larger texture.
+  int width = shape.Width() + radius * 2 * SHADOW_BLUR_MARGIN_FACTOR;
+  int height = shape.Height() + radius * 2 * SHADOW_BLUR_MARGIN_FACTOR;
+
+  if (width != last_shadow_rect_.width() || height != last_shadow_rect_.height())
+    shaped_shadow_pixmap_ = BuildShapedShadowTexture({width, height}, radius, color, shape);
+
+  const auto* texture = shaped_shadow_pixmap_->texture();
 
   if (!texture || !texture->width() || !texture->height())
     return;
 
-  CompRect border = win_->borderRect();
-  nux::Point2D<int> shadow_offset = manager_->shadow_offset();
-// ideally it would be -radius for the *2 part see comment in Manager::Impl::BuildShapedShadowTexture
-// in DecorationsManager.cpp Make sure to keep these factors in sync.
-  int x = border.x() + shadow_offset.x - radius * 2 + shape.getXoffs();
-  int y = border.y() + shadow_offset.y - radius * 2 + shape.getYoffs();
-  int width = texture->width();
-  int height = texture->height();
+  int x = border.x() + shadow_offset.x - radius * 2 + shape.XOffset();
+  int y = border.y() + shadow_offset.y - radius * 2 + shape.YOffset();
 
   auto* quad = &shadow_quads_[Quads::Pos(0)];
   quad->box.setGeometry(x, y, width, height);
@@ -715,14 +745,14 @@
   quad->matrix.y0 = -COMP_TEX_COORD_Y(quad->matrix, quad->box.y1());
 
   CompRect shaped_shadow_rect(x, y, width, height);
-  if (shaped_shadow_rect != last_shadow_rect_) {
+  if (shaped_shadow_rect != last_shadow_rect_)
+  {
     auto const& win_region = win_->region();
     quad->region = CompRegion(quad->box) - win_region;
 
     last_shadow_rect_ = shaped_shadow_rect;
     win_->updateWindowOutputExtents();
   }
-  cwin_->addDamage(true);
 }
 
 void Window::Impl::Paint(GLMatrix const& transformation,
@@ -763,11 +793,7 @@
 
   glwin_->vertexBuffer()->begin();
 
-  // numRects is != 1 when the window is shaped
-  unsigned int num_quads = shadow_quads_.size();
-  if (win_->region().numRects() != 1)
-    num_quads = 1;
-
+  unsigned int num_quads = IsRectangular() ? shadow_quads_.size() : 1;
   for (unsigned int i = 0; i < num_quads; ++i)
   {
     auto& quad = shadow_quads_[Quads::Pos(i)];
@@ -775,7 +801,10 @@
   }
 
   if (glwin_->vertexBuffer()->end())
-    glwin_->glDrawTexture(ShadowTexture(), transformation, attrib, mask);
+  {
+    if (GLTexture* texture = ShadowTexture())
+      glwin_->glDrawTexture(texture, transformation, attrib, mask);
+  }
 
   for (auto const& dtex : bg_textures_)
   {
@@ -800,6 +829,9 @@
 
 void Window::Impl::RedrawDecorations()
 {
+  if (!win_->isMapped())
+    return;
+
   dirty_geo_ = true;
   cwin_->damageOutputExtents();
 }
@@ -1003,10 +1035,7 @@
 void Window::UpdateDecorationPosition()
 {
   impl_->UpdateMonitor();
-  if (impl_->win_->region().numRects() > 1)
-    impl_->ComputeShapedShadowQuad();
-  else
-      impl_->ComputeShadowQuads();
+  impl_->IsRectangular() ? impl_->ComputeShadowQuads() : impl_->ComputeShapedShadowQuad();
   impl_->UpdateWindowEdgesGeo();
   impl_->UpdateDecorationTextures();
   impl_->UpdateForceQuitDialogPosition();

=== modified file 'decorations/DecoratedWindow.h'
--- decorations/DecoratedWindow.h	2015-08-07 18:54:07 +0000
+++ decorations/DecoratedWindow.h	2016-07-27 10:19:13 +0000
@@ -21,6 +21,7 @@
 #define UNITY_DECORATED_WINDOW
 
 #include "Introspectable.h"
+#include <NuxCore/Property.h>
 #include <memory>
 
 class CompRegion;

=== modified file 'decorations/DecorationsManager.cpp'
--- decorations/DecorationsManager.cpp	2016-07-27 10:19:12 +0000
+++ decorations/DecorationsManager.cpp	2016-07-27 10:19:13 +0000
@@ -20,10 +20,8 @@
 #include "DecorationsPriv.h"
 
 #include <core/atoms.h>
-#include <NuxGraphics/CairoGraphics.h>
 #include <UnityCore/DBusIndicators.h>
 #include <X11/Xatom.h>
-#include <X11/extensions/shape.h>
 
 #include "WindowManager.h"
 
@@ -82,31 +80,6 @@
   return shadow_ctx;
 }
 
-cu::PixmapTexture::Ptr Manager::Impl::BuildShapedShadowTexture(unsigned int radius, nux::Color const& color, DecorationsShape const& shape) {
-  //Ideally it would be shape.getWidth + radius * 2 but Cairographics::BlurSurface isn't bounded by the radius
-  //and we need to compensate by using a larger texture. Make sure to modify Window::Impl::ComputeShapedShadowQuad in
-  //DecoratedWindow.cpp if you change the factor.
-  int blur_margin_factor = 2;
-  int img_width = shape.getWidth() + radius * 2 * blur_margin_factor;
-  int img_height = shape.getHeight() + radius * 2 * blur_margin_factor;
-  nux::CairoGraphics img(CAIRO_FORMAT_ARGB32, img_width, img_height);
-  auto* img_ctx = img.GetInternalContext();
-
-  for (int i=0; i<shape.getRectangleCount(); i++) {
-    XRectangle rect = shape.getRectangle(i);
-    cairo_rectangle(img_ctx, rect.x + radius * blur_margin_factor - shape.getXoffs(), rect.y + radius * blur_margin_factor - shape.getYoffs(), rect.width, rect.height);
-    cairo_set_source_rgba(img_ctx, color.red, color.green, color.blue, color.alpha);
-    cairo_fill(img_ctx);
-  }
-  img.BlurSurface(radius);
-
-  cu::CairoContext shadow_ctx(img_width, img_height);
-  cairo_set_source_surface(shadow_ctx, img.GetSurface(), 0, 0);
-  cairo_paint(shadow_ctx);
-
-  return shadow_ctx;
-}
-
 void Manager::Impl::BuildActiveShadowTexture()
 {
   active_shadow_pixmap_ = BuildShadowTexture(manager_->active_shadow_radius(), manager_->active_shadow_color());

=== modified file 'decorations/DecorationsPriv.h'
--- decorations/DecorationsPriv.h	2016-07-27 10:19:12 +0000
+++ decorations/DecorationsPriv.h	2016-07-27 10:19:13 +0000
@@ -23,11 +23,13 @@
 #include <unordered_map>
 #include <NuxCore/NuxCore.h>
 #include <NuxCore/Rect.h>
+#include <NuxGraphics/CairoGraphics.h>
 #include <UnityCore/ConnectionManager.h>
 #include <UnityCore/Indicators.h>
 #include <core/core.h>
 #include <opengl/opengl.h>
 #include <composite/composite.h>
+#include <X11/extensions/shape.h>
 
 #include "DecorationsShape.h"
 #include "DecorationsDataPool.h"
@@ -116,6 +118,7 @@
   void SyncXShapeWithFrameRegion();
   void SyncMenusGeometries() const;
   bool ShouldBeDecorated() const;
+  bool IsRectangular() const;
   GLTexture* ShadowTexture() const;
   unsigned ShadowRadius() const;
   std::string const& GetMenusPanelID() const;
@@ -126,6 +129,7 @@
   void UpdateWindowEdgesGeo();
   void UpdateForceQuitDialogPosition();
   void RenderDecorationTexture(Side, nux::Geometry const&);
+  cu::PixmapTexture::Ptr BuildShapedShadowTexture(nux::Size const&, unsigned radius, nux::Color const&, Shape const&);
   void Paint(GLMatrix const&, GLWindowPaintAttrib const&, CompRegion const&, unsigned mask);
   void Draw(GLMatrix const&, GLWindowPaintAttrib const&, CompRegion const&, unsigned mask);
 
@@ -156,6 +160,7 @@
   std::string last_title_;
   std::string panel_id_;
   std::vector<cu::SimpleTextureQuad> bg_textures_;
+  cu::PixmapTexture::Ptr shaped_shadow_pixmap_;
   std::shared_ptr<ForceQuitDialog> force_quit_;
   InputMixer::Ptr input_mixer_;
   Layout::Ptr top_layout_;
@@ -166,8 +171,6 @@
   Item::Ptr edge_borders_;
 
   EMConverter::Ptr cv_;
-
-  cu::PixmapTexture::Ptr shaped_shadow_pixmap_;
 };
 
 struct Manager::Impl : sigc::trackable
@@ -192,7 +195,6 @@
   void BuildActiveShadowTexture();
   void BuildInactiveShadowTexture();
   cu::PixmapTexture::Ptr BuildShadowTexture(unsigned radius, nux::Color const&);
-  cu::PixmapTexture::Ptr BuildShapedShadowTexture(unsigned int radius, nux::Color const& color, DecorationsShape const& shape);
   void OnShadowOptionsChanged(bool active);
   void OnWindowFrameChanged(bool, ::Window, std::weak_ptr<decoration::Window> const&);
   bool OnMenuKeyActivated(std::string const&);

=== modified file 'decorations/DecorationsShape.cpp'
--- decorations/DecorationsShape.cpp	2016-07-27 10:19:12 +0000
+++ decorations/DecorationsShape.cpp	2016-07-27 10:19:13 +0000
@@ -17,86 +17,91 @@
  * Authored by: Eleni Maria Stea <[e-mail]>
  */
 
-#include <string.h>
+#include "DecorationsShape.h"
+
+#include <core/core.h>
+#include <NuxCore/Logger.h>
 #include <X11/extensions/shape.h>
-#include "DecoratedWindow.h"
-#include "DecorationsShape.h"
-
-bool DecorationsShape::initShape(XID win)
+
+namespace unity
+{
+namespace decoration
+{
+namespace
+{
+DECLARE_LOGGER(logger, "unity.decoration.shape");
+}
+
+Shape::Shape(Window xid)
 {
   Bool buse, cuse;
   int bx, by, cx, cy;
   unsigned int bw, bh, cw, ch;
   Display *dpy = screen->dpy();
 
-  XShapeQueryExtents(dpy, win, &buse, &bx, &by, &bw, &bh, &cuse, &cx, &cy, &cw, &ch);
+  XShapeQueryExtents(dpy, xid, &buse, &bx, &by, &bw, &bh, &cuse, &cx, &cy, &cw, &ch);
 
   int kind;
-  if (buse) {
-    width = bw;
-    height = bh;
-    xoffs = bx;
-    yoffs = by;
+
+  if (buse)
+  {
+    width_ = bw;
+    height_ = bh;
+    xoffs_ = bx;
+    yoffs_ = by;
     kind = ShapeBounding;
   }
-  else if (cuse) {
-    width = cw;
-    height = ch;
-    xoffs = cx;
-    yoffs = cy;
+  else if (cuse)
+  {
+    width_ = cw;
+    height_ = ch;
+    xoffs_ = cx;
+    yoffs_ = cy;
     kind = ShapeClip;
   }
-  else {
-    fprintf(stderr, "XShapeQueryExtend returned no extends.\n");
-    return false;
+  else
+  {
+    LOG_ERROR(logger) << "XShapeQueryExtend returned no extents";
+    return;
   }
 
   int rect_count, rect_order;
-  XRectangle *rectangles;
-  if (!(rectangles = XShapeGetRectangles(dpy, win, kind, &rect_count, &rect_order))) {
-    fprintf(stderr, "Failed to get shape rectangles\n");
-    return false;
-  }
-
-  for (int i=0; i< rect_count; i++) {
-    rects.push_back(rectangles[i]);
-  }
-
-  XFree(rectangles);
-  return true;
-}
-
-const XRectangle& DecorationsShape::getRectangle(int idx) const
-{
-  return rects[idx];
-}
-
-int DecorationsShape::getRectangleCount() const
-{
-  return (int)rects.size();
-}
-
-int DecorationsShape::getWidth() const
-{
-  return width;
-}
-
-int DecorationsShape::getHeight() const
-{
-  return height;
-}
-
-int DecorationsShape::getXoffs() const
-{
-  return xoffs;
-}
-
-int DecorationsShape::getYoffs() const
-{
-  return yoffs;
-}
-void DecorationsShape::clear()
-{
-  width = height = 0;
-  rects.clear();
-}
+  std::unique_ptr<XRectangle[], int(*)(void*)> rectangles(XShapeGetRectangles(dpy, xid, kind, &rect_count, &rect_order), XFree);
+
+  if (!rectangles)
+  {
+    LOG_ERROR(logger) << "Failed to get shape rectangles";
+    return;
+  }
+
+  for (int i = 0; i < rect_count; ++i)
+    rectangles_.push_back(rectangles[i]);
+}
+
+std::vector<XRectangle> const& Shape::GetRectangles() const
+{
+  return rectangles_;
+}
+
+int Shape::Width() const
+{
+  return width_;
+}
+
+int Shape::Height() const
+{
+  return height_;
+}
+
+int Shape::XOffset() const
+{
+  return xoffs_;
+}
+
+int Shape::YOffset() const
+{
+  return yoffs_;
+}
+
+} // decoration namespace
+} // unity namespace

=== modified file 'decorations/DecorationsShape.h'
--- decorations/DecorationsShape.h	2016-07-27 10:19:12 +0000
+++ decorations/DecorationsShape.h	2016-07-27 10:19:13 +0000
@@ -20,24 +20,35 @@
 #ifndef DECORATIONS_SHAPE_H_
 #define DECORATIONS_SHAPE_H_
 
-#include "WindowManager.h"
-#include "DecoratedWindow.h"
-
-class DecorationsShape
-{
+#include <X11/Xlib.h>
+#include <vector>
+
+namespace unity
+{
+namespace decoration
+{
+class Shape
+{
+public:
+  Shape(Window);
+
+  int Width() const;
+  int Height() const;
+  int XOffset() const;
+  int YOffset() const;
+
+  std::vector<XRectangle> const& GetRectangles() const;
+
 private:
-  std::vector<XRectangle> rects;
-  int width, height;
-  int xoffs, yoffs;
+  int width_;
+  int height_;
+  int xoffs_;
+  int yoffs_;
 
-public:
-  bool initShape(XID win);
-  const XRectangle& getRectangle(int idx) const;
-  int getRectangleCount() const;
-  int getWidth() const;
-  int getHeight() const;
-  int getXoffs() const;
-  int getYoffs() const;
-  void clear();
+  std::vector<XRectangle> rectangles_;
 };
+
+} // decoration namespace
+} // unity namespace
+
 #endif //DECORATIONS_SHAPE_H_

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2016-07-27 10:19:12 +0000
+++ plugins/unityshell/src/unityshell.cpp	2016-07-27 10:19:13 +0000
@@ -3357,6 +3357,7 @@
       PluginAdapter::Default().UpdateShowDesktopState();
       break;
     case CompWindowNotifyBeforeDestroy:
+      deco_win_->Undecorate();
       being_destroyed.emit();
       break;
     case CompWindowNotifyMinimize:

=== modified file 'unity-shared/CompizUtils.cpp'
--- unity-shared/CompizUtils.cpp	2016-07-27 10:19:12 +0000
+++ unity-shared/CompizUtils.cpp	2016-07-27 10:19:13 +0000
@@ -190,11 +190,7 @@
   if (win->wmType() & (CompWindowTypeDockMask | CompWindowTypeDesktopMask))
     return elements;
 
-  auto const& region = win->region();
-  bool rectangular = (region.numRects() == 1);
-  bool alpha = win->alpha();
-
-  if (alpha)
+  if (win->alpha())
   {
     if (wf == WindowFilter::CLIENTSIDE_DECORATED)
     {
@@ -205,7 +201,7 @@
 
       return elements;
     }
-    else if (!rectangular) // Non-rectangular windows with alpha channel
+    else if (win->region().numRects() != 1) // Non-rectangular windows with alpha channel
     {
       return elements;
     }
@@ -220,11 +216,14 @@
     if (win->actions() & CompWindowActionResizeMask)
       elements |= DecorationElement::EDGE;
 
+    auto const& region = win->region();
+    bool rectangular = (region.numRects() == 1);
+
     if (rectangular && (win->mwmDecor() & (MwmDecorAll | MwmDecorTitle)))
       elements |= DecorationElement::BORDER;
   }
 
-  if (alpha && !(elements & DecorationElement::BORDER) && !(win->mwmDecor() & MwmDecorBorder))
+  if (win->alpha() && !(elements & DecorationElement::BORDER) && !(win->mwmDecor() & MwmDecorBorder))
     elements &= ~DecorationElement::SHADOW;
 
   return elements;