Bug 1249813 - part 1 - revise nsShmImage to allow draw targets anywhere inside its bounds. r=jrmuizel

gfx/layers/basic/BasicCompositor.cpp

@@ -186,11 +186,14 @@ BasicCompositor::CreateRenderTargetForWindow(const IntRect& aRect, SurfaceInitMo
   MOZ_ASSERT(mDrawTarget);
 
   // Adjust bounds rect to account for new origin at (0, 0).
-  IntRect rect(0, 0, aRect.XMost(), aRect.YMost());
+  IntRect windowRect = aRect;
-  RefPtr<BasicCompositingRenderTarget> rt = new BasicCompositingRenderTarget(mDrawTarget, rect);
+  if (aRect.Size() != mDrawTarget->GetSize()) {
+    windowRect.ExpandToEnclose(IntPoint(0, 0));
+  }
+  RefPtr<BasicCompositingRenderTarget> rt = new BasicCompositingRenderTarget(mDrawTarget, windowRect);
 
   if (aInit == INIT_MODE_CLEAR) {
-    mDrawTarget->ClearRect(gfx::Rect(aRect));
+    mDrawTarget->ClearRect(Rect(aRect - rt->GetOrigin()));
   }
 
   return rt.forget();

widget/gtk/nsWindow.cpp

@@ -2236,22 +2236,21 @@ nsWindow::OnExposeEvent(cairo_t *cr)
         return FALSE;
     }
     RefPtr<gfxContext> ctx;
+    IntRect boundsRect = region.GetBounds().ToUnknownRect();
+    IntPoint offset(0, 0);
+    if (dt->GetSize() == boundsRect.Size()) {
+      offset = boundsRect.TopLeft();
+      dt->SetTransform(Matrix::Translation(-offset));
+    }
 
 #ifdef MOZ_X11
-    nsIntRect boundsRect; // for shaped only
-
     if (shaped) {
         // Collapse update area to the bounding box. This is so we only have to
         // call UpdateTranslucentWindowAlpha once. After we have dropped
         // support for non-Thebes graphics, UpdateTranslucentWindowAlpha will be
         // our private interface so we can rework things to avoid this.
-        boundsRect = region.GetBounds().ToUnknownRect();
         dt->PushClipRect(Rect(boundsRect));
-    } else {
-        gfxUtils::ClipToRegion(dt, region.ToUnknownRegion());
-    }
 
-    if (shaped) {
         // The double buffering is done here to extract the shape mask.
         // (The shape mask won't be necessary when a visual with an alpha
         // channel is used on compositing window managers.)
@@ -2259,7 +2258,9 @@ nsWindow::OnExposeEvent(cairo_t *cr)
         RefPtr<DrawTarget> destDT = dt->CreateSimilarDrawTarget(boundsRect.Size(), SurfaceFormat::B8G8R8A8);
         ctx = new gfxContext(destDT, boundsRect.TopLeft());
     } else {
-        ctx = new gfxContext(dt);
+        gfxUtils::ClipToRegion(dt, region.ToUnknownRegion());
+
+        ctx = new gfxContext(dt, offset);
     }
 
 #if 0
@@ -2304,7 +2305,7 @@ nsWindow::OnExposeEvent(cairo_t *cr)
 
 #  ifdef MOZ_HAVE_SHMIMAGE
     if (mShmImage && MOZ_LIKELY(!mIsDestroyed)) {
-      mShmImage->Put(mXDisplay, mXWindow, region);
+      mShmImage->Put(region);
     }
 #  endif  // MOZ_HAVE_SHMIMAGE
 #endif // MOZ_X11
@@ -6487,13 +6488,7 @@ nsWindow::GetSurfaceForGdkDrawable(GdkDrawable* aDrawable,
 already_AddRefed<DrawTarget>
 nsWindow::GetDrawTarget(const LayoutDeviceIntRegion& aRegion, BufferMode* aBufferMode)
 {
-  if (!mGdkWindow) {
+  if (!mGdkWindow || aRegion.IsEmpty()) {
-    return nullptr;
-  }
-
-  LayoutDeviceIntRect bounds = aRegion.GetBounds();
-  LayoutDeviceIntSize size(bounds.XMost(), bounds.YMost());
-  if (size.width <= 0 || size.height <= 0) {
     return nullptr;
   }
 
@@ -6502,12 +6497,19 @@ nsWindow::GetDrawTarget(const LayoutDeviceIntRegion& aRegion, BufferMode* aBuffe
 #ifdef MOZ_X11
 #  ifdef MOZ_HAVE_SHMIMAGE
   if (nsShmImage::UseShm()) {
-    dt = nsShmImage::EnsureShmImage(size,
+    if (!mShmImage) {
-                                    mXDisplay, mXVisual, mXDepth, mShmImage);
+      mShmImage = new nsShmImage(mXDisplay, mXWindow, mXVisual, mXDepth);
+    }
+    dt = mShmImage->CreateDrawTarget(aRegion);
     *aBufferMode = BufferMode::BUFFER_NONE;
+    if (!dt) {
+      mShmImage = nullptr;
+    }
   }
 #  endif  // MOZ_HAVE_SHMIMAGE
   if (!dt) {
+    LayoutDeviceIntRect bounds = aRegion.GetBounds();
+    LayoutDeviceIntSize size(bounds.XMost(), bounds.YMost());
     RefPtr<gfxXlibSurface> surf = new gfxXlibSurface(mXDisplay, mXWindow, mXVisual, size.ToUnknownSize());
     if (!surf->CairoStatus()) {
       dt = gfxPlatform::GetPlatform()->CreateDrawTargetForSurface(surf.get(), surf->GetSize());
@@ -6535,7 +6537,7 @@ nsWindow::EndRemoteDrawingInRegion(DrawTarget* aDrawTarget,
     return;
   }
 
-  mShmImage->Put(mXDisplay, mXWindow, aInvalidRegion);
+  mShmImage->Put(aInvalidRegion);
 #  endif // MOZ_HAVE_SHMIMAGE
 #endif // MOZ_X11
 }

widget/nsShmImage.cpp

@@ -4,13 +4,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#if defined(MOZ_WIDGET_GTK)
-#include <gtk/gtk.h>
-#include <gdk/gdkx.h>
-#elif defined(MOZ_WIDGET_QT)
-#include <QWindow>
-#endif
-
 #include "nsShmImage.h"
 #ifdef MOZ_WIDGET_GTK
 #include "gfxPlatformGtk.h"
@@ -35,9 +28,9 @@ static bool gShmAvailable = true;
 bool nsShmImage::UseShm()
 {
 #ifdef MOZ_WIDGET_GTK
-    return (gShmAvailable && !gfxPlatformGtk::GetPlatform()->UseXRender());
+  return (gShmAvailable && !gfxPlatformGtk::GetPlatform()->UseXRender());
 #else
-    return gShmAvailable;
+  return gShmAvailable;
 #endif
 }
 
@@ -47,9 +40,9 @@ static int gShmError = 0;
 static int
 TrapShmError(Display* aDisplay, XErrorEvent* aEvent)
 {
-    // store the error code and ignore the error
+  // store the error code and ignore the error
-    gShmError = aEvent->error_code;
+  gShmError = aEvent->error_code;
-    return 0;
+  return 0;
 }
 #endif
 
@@ -68,16 +61,19 @@ nsShmImage::CreateShmSegment()
   }
 
   mInfo.shmaddr = (char *)shmat(mInfo.shmid, nullptr, 0);
+
+  // Mark the handle removed so that it will destroy the segment when unmapped.
+  shmctl(mInfo.shmid, IPC_RMID, nullptr);
+
   if (mInfo.shmaddr == (void *)-1) {
+    // Since mapping failed, the segment is already destroyed.
+    mInfo.shmid = -1;
+
     nsPrintfCString warning("shmat(): %s (%d)\n", strerror(errno), errno);
     NS_WARNING(warning.get());
     return false;
   }
 
-  // Mark the handle as deleted so that, should this process go away, the
-  // segment is cleaned up.
-  shmctl(mInfo.shmid, IPC_RMID, 0);
-
 #ifdef DEBUG
   struct shmid_ds info;
   if (shmctl(mInfo.shmid, IPC_STAT, &info) < 0) {
@@ -105,55 +101,25 @@ nsShmImage::DestroyShmSegment()
 }
 
 bool
-nsShmImage::CreateImage(const LayoutDeviceIntSize& aSize,
+nsShmImage::CreateImage(const IntSize& aSize)
-                        Display* aDisplay, Visual* aVisual, unsigned int aDepth)
 {
-  mDisplay = aDisplay;
+  MOZ_ASSERT(mDisplay && mVisual);
-  mImage = XShmCreateImage(aDisplay, aVisual, aDepth,
-                           ZPixmap, nullptr,
-                           &mInfo,
-                           aSize.width, aSize.height);
-  if (!mImage || !CreateShmSegment()) {
-    return false;
-  }
 
-#if defined(MOZ_WIDGET_GTK)
-  gShmError = 0;
-  XErrorHandler previousHandler = XSetErrorHandler(TrapShmError);
-  Status attachOk = XShmAttach(aDisplay, &mInfo);
-  XSync(aDisplay, False);
-  XSetErrorHandler(previousHandler);
-  if (gShmError) {
-    attachOk = 0;
-  }
-#elif defined(MOZ_WIDGET_QT)
-  Status attachOk = XShmAttach(aDisplay, &mInfo);
-#endif
-
-  if (!attachOk) {
-    // Assume XShm isn't available, and don't attempt to use it
-    // again.
-    gShmAvailable = false;
-    return false;
-  }
-
-  mXAttached = true;
-  mSize = aSize;
   mFormat = SurfaceFormat::UNKNOWN;
-  switch (mImage->depth) {
+  switch (mDepth) {
   case 32:
-    if ((mImage->red_mask == 0xff0000) &&
+    if (mVisual->red_mask == 0xff0000 &&
-        (mImage->green_mask == 0xff00) &&
+        mVisual->green_mask == 0xff00 &&
-        (mImage->blue_mask == 0xff)) {
+        mVisual->blue_mask == 0xff) {
       mFormat = SurfaceFormat::B8G8R8A8;
     }
     break;
   case 24:
     // Only support the BGRX layout, and report it as BGRA to the compositor.
     // The alpha channel will be discarded when we put the image.
-    if ((mImage->red_mask == 0xff0000) &&
+    if (mVisual->red_mask == 0xff0000 &&
-        (mImage->green_mask == 0xff00) &&
+        mVisual->green_mask == 0xff00 &&
-        (mImage->blue_mask == 0xff)) {
+        mVisual->blue_mask == 0xff) {
       mFormat = SurfaceFormat::B8G8R8A8;
     }
     break;
@@ -168,96 +134,108 @@ nsShmImage::CreateImage(const LayoutDeviceIntSize& aSize,
     return false;
   }
 
+  mImage = XShmCreateImage(mDisplay, mVisual, mDepth,
+                           ZPixmap, nullptr,
+                           &mInfo,
+                           aSize.width, aSize.height);
+  if (!mImage || !CreateShmSegment()) {
+    DestroyImage();
+    return false;
+  }
+
+#ifdef MOZ_WIDGET_GTK
+  gShmError = 0;
+  XErrorHandler previousHandler = XSetErrorHandler(TrapShmError);
+  Status attachOk = XShmAttach(mDisplay, &mInfo);
+  XSync(mDisplay, False);
+  XSetErrorHandler(previousHandler);
+  if (gShmError) {
+    attachOk = 0;
+  }
+#else
+  Status attachOk = XShmAttach(mDisplay, &mInfo);
+#endif
+
+  if (!attachOk) {
+    DestroyShmSegment();
+    DestroyImage();
+
+    // Assume XShm isn't available, and don't attempt to use it
+    // again.
+    gShmAvailable = false;
+    return false;
+  }
+
   return true;
 }
 
-nsShmImage::~nsShmImage()
+void
+nsShmImage::DestroyImage()
 {
   if (mImage) {
     mozilla::FinishX(mDisplay);
-    if (mXAttached) {
+    if (mInfo.shmid != -1) {
       XShmDetach(mDisplay, &mInfo);
     }
     XDestroyImage(mImage);
+    mImage = nullptr;
   }
   DestroyShmSegment();
 }
 
 already_AddRefed<DrawTarget>
-nsShmImage::CreateDrawTarget()
+nsShmImage::CreateDrawTarget(const LayoutDeviceIntRegion& aRegion)
 {
+  // Due to bug 1205045, we must avoid making GTK calls off the main thread to query window size.
+  // Instead we just track the largest offset within the image we are drawing to and grow the image
+  // to accomodate it. Since usually the entire window is invalidated on the first paint to it,
+  // this should grow the image to the necessary size quickly without many intermediate reallocations.
+  IntRect bounds = aRegion.GetBounds().ToUnknownRect();
+  IntSize size(bounds.XMost(), bounds.YMost());
+  if (!mImage || size.width > mImage->width || size.height > mImage->height) {
+    DestroyImage();
+    if (!CreateImage(size)) {
+      return nullptr;
+    }
+  }
+
   return gfxPlatform::GetPlatform()->CreateDrawTargetForData(
-    reinterpret_cast<unsigned char*>(mImage->data),
+    reinterpret_cast<unsigned char*>(mImage->data)
-    mSize.ToUnknownSize(),
+      + bounds.y * mImage->bytes_per_line + bounds.x * BytesPerPixel(mFormat),
+    bounds.Size(),
     mImage->bytes_per_line,
     mFormat);
 }
 
-#ifdef MOZ_WIDGET_GTK
 void
-nsShmImage::Put(Display* aDisplay, Drawable aWindow,
+nsShmImage::Put(const LayoutDeviceIntRegion& aRegion)
-                const LayoutDeviceIntRegion& aRegion)
 {
-    GC gc = XCreateGC(aDisplay, aWindow, 0, nullptr);
+  if (!mImage) {
-    LayoutDeviceIntRegion bounded;
+    return;
-    bounded.And(aRegion,
-                LayoutDeviceIntRect(0, 0, mImage->width, mImage->height));
-    for (auto iter = bounded.RectIter(); !iter.Done(); iter.Next()) {
-        const LayoutDeviceIntRect& r = iter.Get();
-        XShmPutImage(aDisplay, aWindow, gc, mImage,
-                     r.x, r.y,
-                     r.x, r.y,
-                     r.width, r.height,
-                     False);
-    }
-
-    XFreeGC(aDisplay, gc);
-
-    // FIXME/bug 597336: we need to ensure that the shm image isn't
-    // scribbled over before all its pending XShmPutImage()s complete.
-    // However, XSync() is an unnecessarily heavyweight
-    // synchronization mechanism; other options are possible.  If this
-    // XSync is shown to hurt responsiveness, we need to explore the
-    // other options.
-    XSync(aDisplay, False);
-}
-
-#elif defined(MOZ_WIDGET_QT)
-void
-nsShmImage::Put(QWindow* aWindow, QRect& aRect)
-{
-    Display* dpy = gfxQtPlatform::GetXDisplay(aWindow);
-    Drawable d = aWindow->winId();
-
-    GC gc = XCreateGC(dpy, d, 0, nullptr);
-    // Avoid out of bounds painting
-    QRect inter = aRect.intersected(aWindow->geometry());
-    XShmPutImage(dpy, d, gc, mImage,
-                 inter.x(), inter.y(),
-                 inter.x(), inter.y(),
-                 inter.width(), inter.height(),
-                 False);
-    XFreeGC(dpy, gc);
-}
-#endif
-
-already_AddRefed<DrawTarget>
-nsShmImage::EnsureShmImage(const LayoutDeviceIntSize& aSize,
-                           Display* aDisplay, Visual* aVisual, unsigned int aDepth,
-                           RefPtr<nsShmImage>& aImage)
-{
-  if (!aImage || aImage->Size() != aSize) {
-    // Because we XSync() after XShmAttach() to trap errors, we
-    // know that the X server has the old image's memory mapped
-    // into its address space, so it's OK to destroy the old image
-    // here even if there are outstanding Puts.  The Detach is
-    // ordered after the Puts.
-    aImage = new nsShmImage;
-    if (!aImage->CreateImage(aSize, aDisplay, aVisual, aDepth)) {
-      aImage = nullptr;
-    }
   }
-  return !aImage ? nullptr : aImage->CreateDrawTarget();
+
+  GC gc = XCreateGC(mDisplay, mWindow, 0, nullptr);
+  LayoutDeviceIntRegion bounded;
+  bounded.And(aRegion,
+              LayoutDeviceIntRect(0, 0, mImage->width, mImage->height));
+  for (auto iter = bounded.RectIter(); !iter.Done(); iter.Next()) {
+    const LayoutDeviceIntRect& r = iter.Get();
+    XShmPutImage(mDisplay, mWindow, gc, mImage,
+                 r.x, r.y,
+                 r.x, r.y,
+                 r.width, r.height,
+                 False);
+  }
+
+  XFreeGC(mDisplay, gc);
+
+  // FIXME/bug 597336: we need to ensure that the shm image isn't
+  // scribbled over before all its pending XShmPutImage()s complete.
+  // However, XSync() is an unnecessarily heavyweight
+  // synchronization mechanism; other options are possible.  If this
+  // XSync is shown to hurt responsiveness, we need to explore the
+  // other options.
+  XSync(mDisplay, False);
 }
 
 #endif  // MOZ_HAVE_SHMIMAGE

widget/nsShmImage.h

@@ -20,55 +20,51 @@
 #include <X11/Xlib.h>
 #include <X11/extensions/XShm.h>
 
-#ifdef MOZ_WIDGET_QT
-class QRect;
-class QWindow;
-#endif
-
 class nsShmImage {
-    // bug 1168843, compositor thread may create shared memory instances that are destroyed by main thread on shutdown, so this must use thread-safe RC to avoid hitting assertion
+  // bug 1168843, compositor thread may create shared memory instances that are destroyed by main thread on shutdown, so this must use thread-safe RC to avoid hitting assertion
-    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsShmImage)
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsShmImage)
 
 public:
-    static bool UseShm();
+  static bool UseShm();
-    static already_AddRefed<mozilla::gfx::DrawTarget>
-        EnsureShmImage(const mozilla::LayoutDeviceIntSize& aSize,
-                       Display* aDisplay, Visual* aVisual, unsigned int aDepth,
-                       RefPtr<nsShmImage>& aImage);
 
-    already_AddRefed<mozilla::gfx::DrawTarget> CreateDrawTarget();
+  already_AddRefed<mozilla::gfx::DrawTarget>
+    CreateDrawTarget(const mozilla::LayoutDeviceIntRegion& aRegion);
 
-#ifdef MOZ_WIDGET_GTK
+  void Put(const mozilla::LayoutDeviceIntRegion& aRegion);
-    void Put(Display* aDisplay, Drawable aWindow,
-             const mozilla::LayoutDeviceIntRegion& aRegion);
-#elif defined(MOZ_WIDGET_QT)
-    void Put(QWindow* aWindow, QRect& aRect);
-#endif
 
-    mozilla::LayoutDeviceIntSize Size() const { return mSize; }
+  nsShmImage(Display* aDisplay,
+             Drawable aWindow,
+             Visual* aVisual,
+             unsigned int aDepth)
+    : mImage(nullptr)
+    , mDisplay(aDisplay)
+    , mWindow(aWindow)
+    , mVisual(aVisual)
+    , mDepth(aDepth)
+    , mFormat(mozilla::gfx::SurfaceFormat::UNKNOWN)
+  {
+    mInfo.shmid = -1;
+  }
 
 private:
-    nsShmImage()
+  ~nsShmImage()
-        : mImage(nullptr)
+  {
-        , mDisplay(nullptr)
+    DestroyImage();
-        , mFormat(mozilla::gfx::SurfaceFormat::UNKNOWN)
+  }
-        , mXAttached(false)
-    { mInfo.shmid = -1; }
 
-    ~nsShmImage();
+  bool CreateShmSegment();
+  void DestroyShmSegment();
 
-    bool CreateShmSegment();
+  bool CreateImage(const mozilla::gfx::IntSize& aSize);
-    void DestroyShmSegment();
+  void DestroyImage();
 
-    bool CreateImage(const mozilla::LayoutDeviceIntSize& aSize,
+  XImage*                      mImage;
-                     Display* aDisplay, Visual* aVisual, unsigned int aDepth);
+  Display*                     mDisplay;
-
+  Drawable                     mWindow;
-    XImage*                      mImage;
+  Visual*                      mVisual;
-    Display*                     mDisplay;
+  unsigned int                 mDepth;
-    XShmSegmentInfo              mInfo;
+  XShmSegmentInfo              mInfo;
-    mozilla::LayoutDeviceIntSize mSize;
+  mozilla::gfx::SurfaceFormat  mFormat;
-    mozilla::gfx::SurfaceFormat  mFormat;
-    bool                         mXAttached;
 };
 
 #endif // MOZ_HAVE_SHMIMAGE