Merge lp:~smspillaz/unity/unity.big_fbo into lp:unity

[Sam Spilsbury]

Fix bug 868120 and bug 872625

We should use one framebuffer object per screen rather than one per monitor. Using one per monitor leads to all kinds of interesting rendering glitches because the plugins expect that paint is clipped to the entire backbuffer rather than paint being contained in one buffer.

This should also fix crashes on changing resolutions, as we don't have the race condition where a monitor paints and an fbo hasn't been created for it yet.

[Mirco Müller (macslow)]

This branch does break the repaint-cycle. While it compiles/works/runs screen-updates only happen when a window is moved. From the diff I currently don't see what you're missing.

review: Needs Fixing

[Mirco Müller (macslow)]

I tried this on my laptop (intel graphics, mesa) and it worked ok. So something odd is happening with nvidia (binary driver). Might need to see how ATI/fglrx behaves with this.

[Michal Hruby (mhr3)]

Nvidia here and I can confirm MacSlow's findings.

[Sam Spilsbury (smspillaz)]

> Nvidia here and I can confirm MacSlow's findings.

Damn, ok. Their driver is broken :( I will have to work around it.

[Sam Spilsbury (smspillaz)]

Noted: the driver is not carrying over attributes when set using glPushAttrib (GL_CURRENT_BIT), Jay says that they should be, and this is what the mesa drivers do, so NVIDIA should know about this.

[Michal Hruby (mhr3)]

I can confirm that it properly repaints now, can't test multiple monitors though.

[Jason Smith (jassmith)]

+1

review: Approve

[Sam Spilsbury (smspillaz)]

Can someone with fglrx test this ?

[Gord Allott (gordallott)]

On 11/10/11 12:18, Sam Spilsbury wrote:
> Noted: the driver is not carrying over attributes when set using glPushAttrib (GL_CURRENT_BIT), Jay says that they should be, and this is what the mesa drivers do, so NVIDIA should know about this.

merge approved

--
Gordon Allott
Canonical Ltd.
27 Floor, Millbank Tower
London SW1P 4QP
www.canonical.com

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2011-10-11 02:24:19 +0000
+++ plugins/unityshell/src/unityshell.cpp	2011-10-12 14:53:30 +0000
@@ -112,7 +112,7 @@
   , _key_nav_mode_requested(false)
   , _last_output(nullptr)
   , switcher_desktop_icon(nullptr)
-  , mActiveFbo (0)
+  , _active_fbo (0)
   , dash_is_open_ (false)
   , grab_index_ (0)
   , painting_tray_ (false)
@@ -227,10 +227,8 @@
 
      if (GL::fbo)
      {
-       foreach (CompOutput & o, screen->outputDevs())
-	     uScreen->mFbos[&o] = UnityFBO::Ptr (new UnityFBO(&o));
-
-       uScreen->mFbos[&(screen->fullscreenOutput ())] = UnityFBO::Ptr (new UnityFBO(&(screen->fullscreenOutput ())));
+       CompRect geometry = CompRect (0, 0, screen->width (), screen->height ());
+       uScreen->_fbo = UnityFBO::Ptr (new UnityFBO (geometry));
      }
 
      optionSetLauncherHideModeNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
@@ -433,6 +431,8 @@
    * drivers (lp:703140). */
   glDisable(GL_LIGHTING);
 
+  _fbo->unbind ();
+
   /* reset matrices */
   glPushAttrib(GL_VIEWPORT_BIT | GL_ENABLE_BIT |
                GL_TEXTURE_BIT | GL_COLOR_BUFFER_BIT | GL_SCISSOR_BIT);
@@ -448,7 +448,7 @@
 
 void UnityScreen::nuxEpilogue()
 {
-  (*GL::bindFramebuffer)(GL_FRAMEBUFFER_EXT, mActiveFbo);
+  (*GL::bindFramebuffer)(GL_FRAMEBUFFER_EXT, _active_fbo);
 
   glMatrixMode(GL_PROJECTION);
   glLoadIdentity();
@@ -471,6 +471,8 @@
   glPopAttrib();
 
   glDisable(GL_SCISSOR_TEST);
+
+  _fbo->bind (_last_output);
 }
 
 void UnityScreen::OnLauncherHiddenChanged()
@@ -489,8 +491,6 @@
   if (PluginAdapter::Default()->IsExpoActive())
     return;
 
-  nuxPrologue();
-
   CompOutput* output = _last_output;
   float vc[4];
   float h = 20.0f;
@@ -501,6 +501,9 @@
   float y1 = output->y() + panel_h;
   float x2 = x1 + output->width();
   float y2 = y1 + h;
+  GLMatrix sTransform = GLMatrix ();
+
+  sTransform.toScreenSpace(output, -DEFAULT_Z_CAMERA);
 
   vc[0] = x1;
   vc[1] = x2;
@@ -540,7 +543,6 @@
       glDisable(GL_BLEND);
     }
   }
-  nuxEpilogue();
 }
 
 void
@@ -559,33 +561,29 @@
 {
   CompOutput *output = _last_output;
   Window     tray_xid = panelController->GetTrayXid ();
-  bool       bound = mFbos[output]->bound ();
-
-
-  if (bound)
-  {
-    mFbos[output]->unbind ();
-
-    /* Draw the bit of the relevant framebuffer for each output */
-    mFbos[output]->paint ();
-  }
 
   nuxPrologue();
 
+  /* Draw the bit of the relevant framebuffer for each output */
+  _fbo->paint (output);
+
   nux::ObjectPtr<nux::IOpenGLTexture2D> device_texture =
-  nux::GetGraphicsDisplay()->GetGpuDevice()->CreateTexture2DFromID(mFbos[output]->texture(),
-  output->width(), output->height(), 1, nux::BITFMT_R8G8B8A8);
+      nux::GetGraphicsDisplay()->GetGpuDevice()->CreateTexture2DFromID(_fbo->texture(),
+                                                                       screen->width (), screen->height(), 1, nux::BITFMT_R8G8B8A8);
 
   nux::GetGraphicsDisplay()->GetGpuDevice()->backup_texture0_ = device_texture;
 
-  nux::Geometry geo = nux::Geometry (output->x (), output->y (), output->width (), output->height ());
+  nux::Geometry geo = nux::Geometry (0, 0, screen->width (), screen->height ());
+  nux::Geometry oGeo = nux::Geometry (output->x (), output->y (), output->width (), output->height ());
   BackgroundEffectHelper::monitor_rect_ = geo;
 
   _in_paint = true;
-  wt->RenderInterfaceFromForeignCmd (&geo);
+  wt->RenderInterfaceFromForeignCmd (&oGeo);
   _in_paint = false;
   nuxEpilogue();
 
+  _fbo->unbind ();
+
   if (tray_xid && !allowWindowPaint)
   {
     CompWindow *tray = screen->findWindow (tray_xid);
@@ -936,6 +934,10 @@
 {
   bool ret;
 
+  doShellRepaint = true;
+  allowWindowPaint = true;
+  _last_output = output;
+
   /* bind the framebuffer here
    * - it will be unbound and flushed
    *   to the backbuffer when some
@@ -946,11 +948,7 @@
    *   attempts to bind it will only increment
    *   its bind reference so make sure that
    *   you always unbind as much as you bind */
-  mFbos[output]->bind ();
-
-  doShellRepaint = true;
-  allowWindowPaint = true;
-  _last_output = output;
+  _fbo->bind (output);
 
   /* glPaintOutput is part of the opengl plugin, so we need the GLScreen base class. */
   ret = gScreen->glPaintOutput(attrib, transform, region, output, mask);
@@ -2084,16 +2082,14 @@
   GdkRectangle rect;
   nux::Geometry lCurGeom;
   int panel_height = 24;
+  CompRect geometry = CompRect (0, 0, screen->width (), screen->height ());
 
   if (!needsRelayout)
     return;
 
   if (GL::fbo)
   {
-    foreach (CompOutput &o, screen->outputDevs ())
-      uScreen->mFbos[&o] = UnityFBO::Ptr (new UnityFBO (&o));
-
-    uScreen->mFbos[&(screen->fullscreenOutput ())] = UnityFBO::Ptr (new UnityFBO (&(screen->fullscreenOutput ())));
+    uScreen->_fbo = UnityFBO::Ptr (new UnityFBO (geometry));
   }
 
   UScreen *uscreen = UScreen::GetDefault();
@@ -2132,8 +2128,6 @@
 {
   UnityScreen* uScr = reinterpret_cast<UnityScreen*>(data);
 
-  uScreen->mFbos.clear ();
-
   uScr->NeedsRelayout ();
   uScr->Relayout();
   uScr->relayoutSourceId = 0;
@@ -2166,23 +2160,17 @@
   ScheduleRelayout(500);
 }
 
-void UnityFBO::paint ()
+void UnityFBO::paint (CompOutput *output)
 {
-  //CompositeScreen *cScreen = CompositeScreen::get (screen);
-  //unsigned int    mask = cScreen->damageMask ();
   float texx, texy, texwidth, texheight;
 
-  /* Must be completely unbound before painting */
-  if (mBoundCnt)
-    return;
-
   /* Draw the bit of the relevant framebuffer for each output */
   GLMatrix transform;
 
-  glViewport (output->x (), screen->height () - output->y2 (), output->width (), output->height ());
-  GLScreen::get (screen)->clearOutput (output, GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+  glPushAttrib (GL_VIEWPORT_BIT);
+  glViewport (0, 0, screen->width (), screen->height ());
 
-  transform.toScreenSpace (output, -DEFAULT_Z_CAMERA);
+  transform.toScreenSpace (&screen->fullscreenOutput (), -DEFAULT_Z_CAMERA);
   glPushMatrix ();
   glLoadMatrixf (transform.getMatrix ());
 
@@ -2204,18 +2192,18 @@
     glEnable (GL_SCISSOR_TEST);
 
     glScissor (output->x1 (), screen->height () - output->y2 (),
-               output->width (), output->height ());
+	       output->width (), output->height ());
 
     /* FIXME: This needs to be GL_TRIANGLE_STRIP */
     glBegin (GL_QUADS);
     glTexCoord2f (texx, texy + texheight);
-    glVertex2i   (output->x1 (), output->y1 ());
+    glVertex2i   (mGeometry.x1 (), mGeometry.y1 ());
     glTexCoord2f (texx, texy);
-    glVertex2i   (output->x1 (), output->y2 ());
+    glVertex2i   (mGeometry.x1 (), mGeometry.y2 ());
     glTexCoord2f (texx + texwidth, texy);
-    glVertex2i   (output->x2 (), output->y2 ());
+    glVertex2i   (mGeometry.x2 (), mGeometry.y2 ());
     glTexCoord2f (texx + texwidth, texy + texheight);
-    glVertex2i   (output->x2 (),  output->y1 ());
+    glVertex2i   (mGeometry.x2 (), mGeometry.y1 ());
     glEnd ();
 
     GL::activeTexture (GL_TEXTURE0_ARB);
@@ -2227,16 +2215,13 @@
     glDisable (GL_SCISSOR_TEST);
     glPopAttrib ();
   }
+  glPopAttrib ();
 
   glPopMatrix();
 }
 
 void UnityFBO::unbind ()
 {
-  mBoundCnt--;
-
-  if (mBoundCnt)
-    return;
 
   uScreen->setActiveFbo (0);
   (*GL::bindFramebuffer) (GL_FRAMEBUFFER_EXT, 0);
@@ -2244,11 +2229,9 @@
   glDrawBuffer (GL_BACK);
   glReadBuffer (GL_BACK);
 
-}
+  /* Matches the viewport set we did in ::bind () */
+  glPopAttrib ();
 
-bool UnityFBO::bound ()
-{
-  return mBoundCnt > 0;
 }
 
 bool UnityFBO::status ()
@@ -2256,14 +2239,8 @@
   return mFboStatus;
 }
 
-void UnityFBO::bind ()
+void UnityFBO::bind (CompOutput *output)
 {
-  if (mBoundCnt)
-  {
-    mBoundCnt++;
-    return;
-  }
-
   if (!mFBTexture)
   {
     glGenTextures (1, &mFBTexture);
@@ -2273,7 +2250,7 @@
     glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
     glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
     glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
-    glTexImage2D (GL_TEXTURE_2D, 0, GL_RGBA, output->width(), output->height(), 0, GL_BGRA,
+    glTexImage2D (GL_TEXTURE_2D, 0, GL_RGBA, mGeometry.width (), mGeometry.height (), 0, GL_BGRA,
 #if IMAGE_BYTE_ORDER == MSBFirst
                  GL_UNSIGNED_INT_8_8_8_8_REV,
 #else
@@ -2351,23 +2328,19 @@
   {
     uScreen->setActiveFbo (mFboHandle);
 
-    glDrawBuffer (GL_COLOR_ATTACHMENT0_EXT);
-    glReadBuffer (GL_COLOR_ATTACHMENT0_EXT);
-
-    glViewport (0,
-               0,
-               output->width(),
-               output->height());
-
-    mBoundCnt++;
+    glPushAttrib (GL_VIEWPORT_BIT);
+
+    glViewport (output->x (),
+	       screen->height () - output->y2 (),
+	       output->width (),
+	       output->height());
   }
 }
 
-UnityFBO::UnityFBO (CompOutput *o)
+UnityFBO::UnityFBO (CompRect geometry)
  : mFboStatus (false)
  , mFBTexture (0)
- , output (o)
- , mBoundCnt (0)
+ , mGeometry (geometry)
 {
   (*GL::genFramebuffers) (1, &mFboHandle);
 }

=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2011-10-03 12:54:22 +0000
+++ plugins/unityshell/src/unityshell.h	2011-10-12 14:53:30 +0000
@@ -54,17 +54,16 @@
 
   typedef boost::shared_ptr <UnityFBO> Ptr;
 
-  UnityFBO (CompOutput *o);
+  UnityFBO (CompRect r);
   ~UnityFBO ();
 
 public:
 
-  void bind ();
+  void bind (CompOutput *o);
   void unbind ();
 
   bool status ();
-  bool bound ();
-  void paint ();
+  void paint (CompOutput *o);
 
   GLuint texture () { return mFBTexture; }
 
@@ -74,7 +73,7 @@
   GLuint   mFboHandle; // actual handle to the framebuffer_ext
   bool    mFboStatus; // did the framebuffer texture bind succeed
   GLuint   mFBTexture;
-  CompOutput *output;
+  CompRect mGeometry;
   unsigned int mBoundCnt;
 };
 
@@ -234,7 +233,7 @@
   void NeedsRelayout();
   void ScheduleRelayout(guint timeout);
 
-  void setActiveFbo (GLuint fbo) { mActiveFbo = fbo; }
+  void setActiveFbo (GLuint fbo) { _active_fbo = fbo; }
 
   bool forcePaintOnTop ();
 
@@ -321,8 +320,8 @@
 
   unity::BGHash _bghash;
 
-  std::map <CompOutput *, UnityFBO::Ptr> mFbos;
-  GLuint                                 mActiveFbo;
+  UnityFBO::Ptr _fbo;
+  GLuint        _active_fbo;
 
   bool   queryForShader ();