Bug 1545262 - Update BasicCompositor's mFullWindowRenderTarget before we capture the screenshot for the current frame. r=mattwoodrow
authorMarkus Stange <mstange@themasta.com>
Wed, 14 Aug 2019 06:34:24 +0000
changeset 488369 77c8de59e66a2f416cfb3f5254f6c3f6c8e0d246
parent 488368 759f7dabeddf8e4d78fbc6c8fb29eaab9b88f4b9
child 488370 698f0817d19dc636e46a17c66a3183199b07ef63
push id36440
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 03:57:48 +0000
treeherdermozilla-central@a58b7dc85887 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1545262
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
gfx/layers/Compositor.h
gfx/layers/basic/BasicCompositor.cpp
gfx/layers/basic/BasicCompositor.h
gfx/layers/composite/LayerManagerComposite.cpp
--- a/gfx/layers/Compositor.h
+++ b/gfx/layers/Compositor.h
@@ -425,16 +425,22 @@ class Compositor : public TextureSourceP
                           const gfx::IntRect& aRenderBounds,
                           const nsIntRegion& aOpaqueRegion,
                           gfx::IntRect* aClipRectOut = nullptr,
                           gfx::IntRect* aRenderBoundsOut = nullptr) = 0;
 
   /**
    * 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() {}
 
   /**
    * Flush the current frame to the screen and tidy up.
    *
    * Derived class overriding this should call Compositor::EndFrame.
    */
--- a/gfx/layers/basic/BasicCompositor.cpp
+++ b/gfx/layers/basic/BasicCompositor.cpp
@@ -958,50 +958,51 @@ void BasicCompositor::BeginFrame(
   // Prevent CreateRenderTargetForWindow from clearing unwanted area.
   gfxUtils::ClipToRegion(mDrawTarget, mInvalidRegion.ToUnknownRegion());
 
   // Setup an intermediate render target to buffer all compositing. We will
   // copy this into mDrawTarget (the widget), and/or mTarget in EndFrame()
   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) {
     if (!mTarget) {
       mWidget->EndRemoteDrawingInRegion(mDrawTarget, mInvalidRegion);
     }
     return;
   }
   SetRenderTarget(target);
 
   // We only allocate a surface sized to the invalidated region, so we need to
   // translate future coordinates.
   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());
 
   if (aRenderBoundsOut) {
     *aRenderBoundsOut = rect;
   }
 
   if (aClipRectIn) {
@@ -1055,60 +1056,59 @@ void BasicCompositor::TryToEndRemoteDraw
     RefPtr<BasicCompositor> self = this;
     RefPtr<Runnable> runnable =
         NS_NewRunnableFunction("layers::BasicCompositor::TryToEndRemoteDrawing",
                                [self]() { self->TryToEndRemoteDrawing(); });
     MessageLoop::current()->PostDelayedTask(runnable.forget(), retryMs);
     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);
     }
   }
 
   if (aForceToEnd || !mTarget) {
     mWidget->EndRemoteDrawingInRegion(mDrawTarget, mInvalidRegion);
   }
 
   mDrawTarget = nullptr;
   mRenderTarget = nullptr;
   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);
 
   if (mTarget || mRenderTarget->mDrawTarget == mDrawTarget) {
     return false;
   }
 
--- a/gfx/layers/basic/BasicCompositor.h
+++ b/gfx/layers/basic/BasicCompositor.h
@@ -113,16 +113,17 @@ class BasicCompositor : public Composito
   void ClearRect(const gfx::Rect& aRect) override;
 
   void BeginFrame(const nsIntRegion& aInvalidRegion,
                   const gfx::IntRect* aClipRectIn,
                   const gfx::IntRect& aRenderBounds,
                   const nsIntRegion& aOpaqueRegion,
                   gfx::IntRect* aClipRectOut = nullptr,
                   gfx::IntRect* aRenderBoundsOut = nullptr) override;
+  void NormalDrawingDone() override;
   void EndFrame() override;
 
   bool SupportsPartialTextureUpdate() override { return true; }
   bool CanUseCanvasLayerForSize(const gfx::IntSize& aSize) override {
     return true;
   }
   int32_t GetMaxTextureSize() const override;
   void SetDestinationSurfaceSize(const gfx::IntSize& aSize) override {}
--- a/gfx/layers/composite/LayerManagerComposite.cpp
+++ b/gfx/layers/composite/LayerManagerComposite.cpp
@@ -1044,16 +1044,18 @@ bool LayerManagerComposite::Render(const
     PopGroupForLayerEffects(previousTarget, clipRect.ToUnknownRect(),
                             grayscaleVal, invertVal, contrastVal);
   }
 
   // Allow widget to render a custom foreground.
   mCompositor->GetWidget()->DrawWindowOverlay(
       &widgetContext, LayoutDeviceIntRect::FromUnknownRect(actualBounds));
 
+  mCompositor->NormalDrawingDone();
+
   mProfilerScreenshotGrabber.MaybeGrabScreenshot(mCompositor);
 
   if (mCompositionRecorder) {
     bool hasContentPaint = false;
     for (CompositionPayload& payload : mPayload) {
       if (payload.mType == CompositionPayloadType::eContentPaint) {
         hasContentPaint = true;
         break;
@@ -1063,18 +1065,16 @@ bool LayerManagerComposite::Render(const
     if (hasContentPaint) {
       if (RefPtr<RecordedFrame> frame =
               mCompositor->RecordFrame(TimeStamp::Now())) {
         mCompositionRecorder->RecordFrame(frame);
       }
     }
   }
 
-  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.
   if (jni::IsFennec()) {
     RenderToolbar();
   }
   HandlePixelsTarget();
 #endif  // defined(MOZ_WIDGET_ANDROID)