Bug 1545262 - Update BasicCompositor's mFullWindowRenderTarget before we capture the screenshot for the current frame. r=mattwoodrow

In the past, mFullWindowRenderTarget was updated in EndFrame, so the captured
screenshots (which were captured before EndFrame) were always one frame behind.
This affected both profiler screenshots and window recording screenshots.

This also fixes a bug with the destination offset in the call to
mFullWindowRenderTarget->mDrawTarget->CopySurface(): In the case where mTarget
was non-null, those calls would use mTargetBounds.TopLeft() as the destination
offset, which is very much unrelated to anything in mFullWindowRenderTarget.
Now the destination offset for mFullWindowRenderTarget is always zero -
mFullWindowRenderTarget->mDrawTarget's device space is the same as window space.

This patch also moves the creation of mFullWindowRenderTarget down to where we
have mRenderTarget->mDrawTarget, so that we can create a DrawTarget of a type
that can efficiently copy from mRenderTarget->mDrawTarget. I think this is
important on Windows where mRenderTarget->mDrawTarget will be the Cairo/pixman
re-wrapped DrawTarget and mDrawTarget is some kind of Windows/GDI DrawTarget.

Depends on D41612

Differential Revision: https://phabricator.services.mozilla.com/D41613

--HG--
extra : moz-landing-system : lando

gfx/layers/Compositor.h

@@ -430,6 +430,12 @@ class Compositor : public TextureSourceProvider {
   /**
    * Notification that we've finished issuing draw commands for normal
    * layers (as opposed to the diagnostic overlay which comes after).
+   * This is called between BeginFrame and EndFrame, and it's called before
+   * GetWindowRenderTarget() is called for the purposes of screenshot capturing.
+   * That next call to GetWindowRenderTarget() expects up-to-date contents for
+   * the current frame.
+   * Called at a time when the current render target is the one that BeginFrame
+   * put in place.
    */
   virtual void NormalDrawingDone() {}
 

gfx/layers/basic/BasicCompositor.cpp

@@ -963,25 +963,6 @@ void BasicCompositor::BeginFrame(
   RefPtr<CompositingRenderTarget> target =
       CreateRenderTargetForWindow(mInvalidRect, clearRect, bufferMode);
 
-  if (ShouldRecordFrames()) {
-    IntSize windowSize = rect.ToUnknownRect().Size();
-
-    // On some platforms (notably Linux with X11) we do not always have a
-    // full-size draw target. While capturing profiles with screenshots, we need
-    // access to a full-size target so we can record the contents.
-    if (!mFullWindowRenderTarget ||
-        mFullWindowRenderTarget->mDrawTarget->GetSize() != windowSize) {
-      // We have either (1) just started recording and not yet allocated a
-      // buffer or (2) are already recording and have resized the window. In
-      // either case, we need a new render target.
-      RefPtr<gfx::DrawTarget> drawTarget = mDrawTarget->CreateSimilarDrawTarget(
-          windowSize, mDrawTarget->GetFormat());
-
-      mFullWindowRenderTarget =
-          new BasicCompositingRenderTarget(drawTarget, rect);
-    }
-  }
-
   mDrawTarget->PopClip();
 
   if (!target) {
@@ -997,6 +978,26 @@ void BasicCompositor::BeginFrame(
   mRenderTarget->mDrawTarget->SetTransform(
       Matrix::Translation(-mRenderTarget->GetOrigin()));
 
+  if (ShouldRecordFrames()) {
+    IntSize windowSize = rect.ToUnknownRect().Size();
+
+    // On some platforms (notably Linux with X11) we do not always have a
+    // full-size draw target. While capturing profiles with screenshots, we need
+    // access to a full-size target so we can record the contents.
+    if (!mFullWindowRenderTarget ||
+        mFullWindowRenderTarget->mDrawTarget->GetSize() != windowSize) {
+      // We have either (1) just started recording and not yet allocated a
+      // buffer or (2) are already recording and have resized the window. In
+      // either case, we need a new render target.
+      RefPtr<gfx::DrawTarget> drawTarget =
+          mRenderTarget->mDrawTarget->CreateSimilarDrawTarget(
+              windowSize, mRenderTarget->mDrawTarget->GetFormat());
+
+      mFullWindowRenderTarget =
+          new BasicCompositingRenderTarget(drawTarget, rect);
+    }
+  }
+
   gfxUtils::ClipToRegion(mRenderTarget->mDrawTarget,
                          mInvalidRegion.ToUnknownRegion());
 
@@ -1060,38 +1061,21 @@ void BasicCompositor::TryToEndRemoteDrawing(bool aForceToEnd) {
     return;
   }
 
-  if (mRenderTarget->mDrawTarget != mDrawTarget || mFullWindowRenderTarget) {
-    RefPtr<SourceSurface> source;
-
-    // Note: Most platforms require us to buffer drawing to the widget
-    // surface. That's why we don't draw to mDrawTarget directly.
+  if (mRenderTarget->mDrawTarget != mDrawTarget) {
+    // This is the case where we have a back buffer for BufferMode::BUFFERED
+    // drawing.
+    RefPtr<SourceSurface> source = mWidget->EndBackBufferDrawing();
     IntPoint srcOffset = mRenderTarget->GetOrigin();
     IntPoint dstOffset = mTarget ? mTargetBounds.TopLeft() : IntPoint();
 
-    if (mRenderTarget->mDrawTarget != mDrawTarget) {
-      source = mWidget->EndBackBufferDrawing();
-
-      // The source DrawTarget is clipped to the invalidation region, so we have
-      // to copy the individual rectangles in the region or else we'll draw
-      // blank pixels.
-      for (auto iter = mInvalidRegion.RectIter(); !iter.Done(); iter.Next()) {
-        const LayoutDeviceIntRect& r = iter.Get();
-        mDrawTarget->CopySurface(source, r.ToUnknownRect() - srcOffset,
-                                 r.TopLeft().ToUnknownPoint() - dstOffset);
-      }
-    } else {
-      source = mRenderTarget->mDrawTarget->Snapshot();
-    }
-
-    if (mFullWindowRenderTarget) {
-      for (auto iter = mInvalidRegion.RectIter(); !iter.Done(); iter.Next()) {
-        const LayoutDeviceIntRect& r = iter.Get();
-        mFullWindowRenderTarget->mDrawTarget->CopySurface(
-            source, r.ToUnknownRect() - srcOffset,
-            r.TopLeft().ToUnknownPoint() - dstOffset);
-      }
-
-      mFullWindowRenderTarget->mDrawTarget->Flush();
+    // The source DrawTarget is clipped to the invalidation region, so we have
+    // to copy the individual rectangles in the region or else we'll draw
+    // blank pixels.
+    // CopySurface ignores both the transform and the clip.
+    for (auto iter = mInvalidRegion.RectIter(); !iter.Done(); iter.Next()) {
+      const LayoutDeviceIntRect& r = iter.Get();
+      mDrawTarget->CopySurface(source, r.ToUnknownRect() - srcOffset,
+                               r.TopLeft().ToUnknownPoint() - dstOffset);
     }
   }
 
@@ -1104,6 +1088,22 @@ void BasicCompositor::TryToEndRemoteDrawing(bool aForceToEnd) {
   mIsPendingEndRemoteDrawing = false;
 }
 
+void BasicCompositor::NormalDrawingDone() {
+  if (!mFullWindowRenderTarget) {
+    return;
+  }
+
+  // Now is a good time to update mFullWindowRenderTarget.
+  RefPtr<SourceSurface> source = mRenderTarget->mDrawTarget->Snapshot();
+  IntPoint srcOffset = mRenderTarget->GetOrigin();
+  for (auto iter = mInvalidRegion.RectIter(); !iter.Done(); iter.Next()) {
+    IntRect r = iter.Get().ToUnknownRect();
+    mFullWindowRenderTarget->mDrawTarget->CopySurface(source, r - srcOffset,
+                                                      r.TopLeft());
+  }
+  mFullWindowRenderTarget->mDrawTarget->Flush();
+}
+
 bool BasicCompositor::NeedsToDeferEndRemoteDrawing() {
   MOZ_ASSERT(mDrawTarget);
   MOZ_ASSERT(mRenderTarget);

gfx/layers/basic/BasicCompositor.h

@@ -118,6 +118,7 @@ class BasicCompositor : public Compositor {
                   const nsIntRegion& aOpaqueRegion,
                   gfx::IntRect* aClipRectOut = nullptr,
                   gfx::IntRect* aRenderBoundsOut = nullptr) override;
+  void NormalDrawingDone() override;
   void EndFrame() override;
 
   bool SupportsPartialTextureUpdate() override { return true; }

gfx/layers/composite/LayerManagerComposite.cpp

@@ -1049,6 +1049,8 @@ bool LayerManagerComposite::Render(const nsIntRegion& aInvalidRegion,
   mCompositor->GetWidget()->DrawWindowOverlay(
       &widgetContext, LayoutDeviceIntRect::FromUnknownRect(actualBounds));
 
+  mCompositor->NormalDrawingDone();
+
   mProfilerScreenshotGrabber.MaybeGrabScreenshot(mCompositor);
 
   if (mCompositionRecorder) {
@@ -1068,8 +1070,6 @@ bool LayerManagerComposite::Render(const nsIntRegion& aInvalidRegion,
     }
   }
 
-  mCompositor->NormalDrawingDone();
-
 #if defined(MOZ_WIDGET_ANDROID)
   // Depending on the content shift the toolbar may be rendered on top of
   // some of the content so it must be rendered after the content.