Merge lp:~vanvugt/unity/fix-1057528 into lp:unity

[Daniel van Vugt]

Draw the panel shadow only within the redraw region. Using a bounding box
allowed parts to get redrawn when they should not, appearing as small chunks
of extra-dark panel shadow.
(LP: #1057528)

See commit message.

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

75 + for (auto r : rects)

auto const& r could save some copies there..

67 + if (launcher_controller_->IsOverlayOpen() &&
68 + current_monitor == overlay_monitor_)

On stylistic basis I'd prefer that to be in one line...

Btw, code looks good and it works as expected.

review: Approve

[Sam Spilsbury (smspillaz)]

82 + // Sub-rectangle of the shadow needing redrawing:
83 + float x1 = r.x1();
84 + float y1 = r.y1();
85 + float x2 = r.x2();
86 + float y2 = r.y2();
87 +
88 + // Texture coordinates of the above rectangle:
89 + float tx1 = (x1 - shadowX) / shadowWidth;
90 + float ty1 = (y1 - shadowY) / shadowHeight;
91 + float tx2 = (x2 - shadowX) / shadowWidth;
92 + float ty2 = (y2 - shadowY) / shadowHeight;

This definitely seems like something that's autotestable. Has that been looked into?

[Daniel van Vugt (vanvugt)]

Certainly a simple formula can be auto-tested. However in this case it would make the code significantly less readable and more complex. And more importantly, any mistakes or regressions here are immediately visible on screen.

[Łukasz Zemczak (sil2100)]

Works for me. I also think that a manual test in this case can be sufficient. We don't want to complicate code unnecessarily.

review: Approve

=== modified file 'manual-tests/Panel.txt'
--- manual-tests/Panel.txt	2012-08-23 16:55:54 +0000
+++ manual-tests/Panel.txt	2012-10-02 09:41:22 +0000
@@ -95,3 +95,19 @@
   At no point during the actions should the panel (or launcher) blink or
   flicker out of existence.
 
+
+Panel shadow overdraw
+---------------------
+Setup:
+#. Install Google Chrome or Chromium.
+
+Actions:
+#. Maximize a Chrome window.
+#. Open several tabs so that the titles of some tabs are not fully visible
+   and fade out to the right.
+#. Hover the mouse over the very top part of some tabs so tooltips appear.
+
+Expected Result:
+  No part of the panel shadow should ever appear on top of the maximized
+  Chrome window.
+

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-10-01 23:32:49 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-10-02 09:41:22 +0000
@@ -562,6 +562,9 @@
 
 void UnityScreen::paintPanelShadow(const CompRegion& clip)
 {
+  if (panel_controller_->opacity() == 0.0f)
+    return;
+
   if (sources_.GetSource(local::RELAYOUT_TIMEOUT))
     return;
 
@@ -587,22 +590,6 @@
   if (redraw.isEmpty())
     return;
 
-  const CompRect& bounds(redraw.boundingRect());
-
-  // Sub-rectangle of the shadow needing redrawing:
-  float x1 = bounds.x1();
-  float y1 = bounds.y1();
-  float x2 = bounds.x2();
-  float y2 = bounds.y2();
-
-  // Texture coordinates of the above rectangle:
-  float tx1 = (x1 - shadowX) / shadowWidth;
-  float ty1 = (y1 - shadowY) / shadowHeight;
-  float tx2 = (x2 - shadowX) / shadowWidth;
-  float ty2 = (y2 - shadowY) / shadowHeight;
-
-  nuxPrologue();
-
   // compiz doesn't use the same method of tracking monitors as our toolkit
   // we need to make sure we properly associate with the right monitor
   int current_monitor = -1;
@@ -618,8 +605,14 @@
     i++;
   }
 
-  if (!(launcher_controller_->IsOverlayOpen() && current_monitor == overlay_monitor_)
-      && panel_controller_->opacity() > 0.0f)
+  if (launcher_controller_->IsOverlayOpen() && current_monitor == overlay_monitor_)
+    return;
+
+  nuxPrologue();
+
+  const CompRect::vector& rects = redraw.rects();
+
+  for (auto const& r : rects)
   {
     foreach(GLTexture * tex, _shadow_texture)
     {
@@ -641,6 +634,18 @@
                     (GLushort)(panel_controller_->opacity() * 0xFFFF)
                   };
 
+      // Sub-rectangle of the shadow needing redrawing:
+      float x1 = r.x1();
+      float y1 = r.y1();
+      float x2 = r.x2();
+      float y2 = r.y2();
+
+      // Texture coordinates of the above rectangle:
+      float tx1 = (x1 - shadowX) / shadowWidth;
+      float ty1 = (y1 - shadowY) / shadowHeight;
+      float tx2 = (x2 - shadowX) / shadowWidth;
+      float ty2 = (y2 - shadowY) / shadowHeight;
+
       vertexData = {
         x1, y1, 0,
         x1, y2, 0,