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

[Daniel van Vugt]

Restore the panel shadow, which has been missing since Unity 6.4.
(LP: #1045705)

Restore the panel shadow, which has been missing since Unity 6.4.
(LP: #1045705)

REQUIRES a compiz fix first:
https://code.launchpad.net/~vanvugt/compiz/decorate-after-draw/+merge/124157
Without that fix, the active window will always have the panel shadow on top of it.

[Sam Spilsbury (smspillaz)]

Wasn't the design supposed to be that it draws on top of all windows unless a maximized window is the active one? I've subscribed John to check.

review: Needs Information

[John Lea (johnlea)]

@smspillaz; the shadow is supposed to be drawn on top of all *non-focused* windows, including non-focused maximised windows. The behaviour with LIM is slightly different, but this is not relevant atm.

[Sam Spilsbury (smspillaz)]

Okay, so is that to say that it shouldn't draw on top of unmaximized windows which are in focus ?

[Daniel van Vugt (vanvugt)]

I know the design. I'm saying it may not be feasible to re-introduce that feature. Hence the comment.

[MC Return (mc-return)]

> @smspillaz; the shadow is supposed to be drawn on top of all *non-focused*
> windows, including non-focused maximised windows.

This would result in a popping shadow, which would not look good.

Imagine you have a maximized, focused window and select another non-fullscreen window. The shadow would suddenly pop-up and be drawn on top of the now non-focused maximized window - Once you select this maximized window again, the shadow would suddenly disappear - this behavior would not be good and would feel like a bug.

[Omer Akram (om26er)]

> Imagine you have a maximized, focused window and select another non-fullscreen
> window. The shadow would suddenly pop-up and be drawn on top of the now non-
> focused maximized window - Once you select this maximized window again, the
> shadow would suddenly disappear - this behavior would not be good and would
> feel like a bug.

This behavior exists in 12.04 and does not feel like a bug to me :-)

[Daniel van Vugt (vanvugt)]

All,

The correct place to discuss the validity of design decision is in bug 967294. Not here.

The discussion here is about whether or not we want to accept this proposal which shadows just the desktop, or wait to see if someone can figure out how to restore shadows over unfocussed windows too.

[Omer Akram (om26er)]

Yes, I am aware of that. I just wanted to make it clear that this feature already exists in 12.04 and have regressed in 12.10

[John Lea (johnlea)]

review: Approve (design)

[John Lea (johnlea)]

yes, this is a regression that needs to be fixed before release

[Daniel van Vugt (vanvugt)]

Just to clarify: You are choosing no shadow at all, over shadowing just the desktop :(

[Daniel van Vugt (vanvugt)]

But shadowing just the desktop is a good first step I think. Hence this proposal.

[Andrea Cimitan (cimi)]

Yes, but it's not the fix to the regression. If there isn't a way to fix this, we revert the change that introduced the regression.

[Daniel van Vugt (vanvugt)]

The change that disabled shadowing, I don't know. It seems to have come from Linaro in their original GLES port a while ago. And we only just noticed now when switching to COMPIZ_MODERN_GL.

I never noticed it because the shadow has *never* worked for me when I built Unity locally. So it looked the same as usual to me in testing.

[Andrea Cimitan (cimi)]

My vote goes for a partial fix, so all screenshots (documentation too) will benefit the fix.

[Daniel van Vugt (vanvugt)]

Fully fixed now, pending a small compiz fix.

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

Nice, works ok and looks fine. Just one small thing:

49 + CompRect bounds(redraw.boundingRect());

Why not CompRect const& bounds = redraw.boundingRect() instead?
We don't need a copy here...

review: Approve

[Andrea Azzarone (azzar1)]

Code looks good but I got a crash. Seems like _shadow_texture[0] == 0x0 but I'm sure that the shadow_texture png file is correctly installed.

[Daniel van Vugt (vanvugt)]

Updated per Marco and Andy's comments.

[Andrea Azzarone (azzar1)]

No crash here but I still don't get the shadow.

review: Abstain

[Omer Akram (om26er)]

Shadow was working for me with this branch on Saturday. didn't test with latest changes to this branch though.

[Tim Penhey (thumper)]

Does this also fix bug 1043260 ?

[Daniel van Vugt (vanvugt)]

No, this is not related to bug 1043260. In fact, bug 1043260 has nothing to do with the panel shadow at all. That one is caused by the active blur.

[Omer Akram (om26er)]

Just tested this branch with latest revisions and the Shadow is there.

review: Approve (functional)

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-09-15 00:07:22 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-09-17 03:47:21 +0000
@@ -526,34 +526,48 @@
   panel_shadow_matrix_ = matrix;
 }
 
-/* Currently unimplemented */
-void UnityScreen::paintPanelShadow(const GLMatrix& matrix)
+void UnityScreen::paintPanelShadow(const CompRegion& clip)
 {
-  return;
-
   if (sources_.GetSource(local::RELAYOUT_TIMEOUT))
     return;
 
   if (PluginAdapter::Default()->IsExpoActive())
     return;
 
-  nuxPrologue();
-
   CompOutput* output = _last_output;
-  float vc[4];
-  float h = 20.0f;
-  float w = 1.0f;
   float panel_h = static_cast<float>(panel_style_.panel_height);
 
-  float x1 = output->x();
-  float y1 = output->y() + panel_h;
-  float x2 = x1 + output->width();
-  float y2 = y1 + h;
-
-  vc[0] = x1;
-  vc[1] = x2;
-  vc[2] = y1;
-  vc[3] = y2;
+  // You have no shadow texture. But how?
+  if (_shadow_texture.empty() || !_shadow_texture[0])
+    return;
+
+  float shadowX = output->x();
+  float shadowY = output->y() + panel_h;
+  float shadowWidth = output->width();
+  float shadowHeight = _shadow_texture[0]->height();
+  CompRect shadowRect(shadowX, shadowY, shadowWidth, shadowHeight);
+
+  CompRegion redraw(clip);
+  redraw &= shadowRect;
+
+  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
@@ -594,17 +608,17 @@
                   };
 
       vertexData = {
-        vc[0], vc[2], 0,
-        vc[0], vc[3], 0,
-        vc[1], vc[2], 0,
-        vc[1], vc[3], 0,
+        x1, y1, 0,
+        x1, y2, 0,
+        x2, y1, 0,
+        x2, y2, 0,
       };
 
       textureData = {
-        COMP_TEX_COORD_X(tex->matrix(), 0), COMP_TEX_COORD_Y(tex->matrix(), 0),
-        COMP_TEX_COORD_X(tex->matrix(), 0), COMP_TEX_COORD_Y(tex->matrix(), h),
-        COMP_TEX_COORD_X(tex->matrix(), w), COMP_TEX_COORD_Y(tex->matrix(), 0),
-        COMP_TEX_COORD_X(tex->matrix(), w), COMP_TEX_COORD_Y(tex->matrix(), h),
+        tx1, ty1,
+        tx1, ty2,
+        tx2, ty1,
+        tx2, ty2,
       };
 
       streamingBuffer->begin(GL_TRIANGLE_STRIP);
@@ -614,7 +628,7 @@
       streamingBuffer->addTexCoords(0, 4, &textureData[0]);
 
       streamingBuffer->end();
-      streamingBuffer->render(matrix);
+      streamingBuffer->render(panel_shadow_matrix_);
 
       tex->disable();
       if (!wasBlend)
@@ -2373,7 +2387,7 @@
   Window active_window = screen->activeWindow();
   if (window->id() == active_window && window->type() != CompWindowTypeDesktopMask)
   {
-    uScreen->paintPanelShadow(matrix);
+    uScreen->paintPanelShadow(region);
   }
 
   bool ret = gWindow->glDraw(matrix, attrib, region, mask);
@@ -2381,7 +2395,7 @@
   if ((active_window == 0 || active_window == window->id()) &&
       (window->type() == CompWindowTypeDesktopMask))
   {
-    uScreen->paintPanelShadow(matrix);
+    uScreen->paintPanelShadow(region);
   }
 
 

=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2012-09-12 18:00:22 +0000
+++ plugins/unityshell/src/unityshell.h	2012-09-17 03:47:21 +0000
@@ -95,7 +95,7 @@
 
   /* nux draw wrapper */
   void paintDisplay();
-  void paintPanelShadow(const GLMatrix& matrix);
+  void paintPanelShadow(const CompRegion& clip);
   void setPanelShadowMatrix(const GLMatrix& matrix);
 
   void preparePaint (int ms);