Bug 1143575. Add a bias value to ImageHost to avoid unpredictable results when image times and compositor times are closely aligned. r=nical

--HG--
extra : commitid : GtORzoze3fw
extra : rebase_source : f16d56e72c02e9ba48bc6f01568e59205e87014f

gfx/layers/composite/ImageHost.cpp

@@ -35,6 +35,7 @@ ImageHost::ImageHost(const TextureInfo& aTextureInfo)
   , mImageContainer(nullptr)
   , mLastFrameID(-1)
   , mLastProducerID(-1)
+  , mBias(BIAS_NONE)
   , mLocked(false)
 {}
 
@@ -115,6 +116,60 @@ ImageHost::RemoveTextureHost(TextureHost* aTexture)
   }
 }
 
+static TimeStamp
+GetBiasedTime(const TimeStamp& aInput, ImageHost::Bias aBias)
+{
+  switch (aBias) {
+  case ImageHost::BIAS_NEGATIVE:
+    return aInput - TimeDuration::FromMilliseconds(1.0);
+  case ImageHost::BIAS_POSITIVE:
+    return aInput + TimeDuration::FromMilliseconds(1.0);
+  default:
+    return aInput;
+  }
+}
+
+static ImageHost::Bias
+UpdateBias(const TimeStamp& aCompositionTime,
+           const TimeStamp& aCompositedImageTime,
+           const TimeStamp& aNextImageTime, // may be null
+           ImageHost::Bias aBias)
+{
+  if (aCompositedImageTime.IsNull()) {
+    return ImageHost::BIAS_NONE;
+  }
+  TimeDuration threshold = TimeDuration::FromMilliseconds(1.0);
+  if (aCompositionTime - aCompositedImageTime < threshold &&
+      aCompositionTime - aCompositedImageTime > -threshold) {
+    // The chosen frame's time is very close to the composition time (probably
+    // just before the current composition time, but due to previously set
+    // negative bias, it could be just after the current composition time too).
+    // If the inter-frame time is almost exactly equal to (a multiple of)
+    // the inter-composition time, then we're in a dangerous situation because
+    // jitter might cause frames to fall one side or the other of the
+    // composition times, causing many frames to be skipped or duplicated.
+    // Try to prevent that by adding a negative bias to the frame times during
+    // the next composite; that should ensure the next frame's time is treated
+    // as falling just before a composite time.
+    return ImageHost::BIAS_NEGATIVE;
+  }
+  if (!aNextImageTime.IsNull() &&
+      aNextImageTime - aCompositionTime < threshold &&
+      aNextImageTime - aCompositionTime > -threshold) {
+    // The next frame's time is very close to our composition time (probably
+    // just after the current composition time, but due to previously set
+    // positive bias, it could be just before the current composition time too).
+    // We're in a dangerous situation because jitter might cause frames to
+    // fall one side or the other of the composition times, causing many frames
+    // to be skipped or duplicated.
+    // Try to prevent that by adding a negative bias to the frame times during
+    // the next composite; that should ensure the next frame's time is treated
+    // as falling just before a composite time.
+    return ImageHost::BIAS_POSITIVE;
+  }
+  return ImageHost::BIAS_NONE;
+}
+
 int ImageHost::ChooseImageIndex() const
 {
   if (!GetCompositor() || mImages.IsEmpty()) {
@@ -136,7 +191,7 @@ int ImageHost::ChooseImageIndex() const
 
   uint32_t result = 0;
   while (result + 1 < mImages.Length() &&
-      mImages[result + 1].mTimeStamp <= now) {
+      GetBiasedTime(mImages[result + 1].mTimeStamp, mBias) <= now) {
     ++result;
   }
   return result;
@@ -206,104 +261,117 @@ ImageHost::Composite(LayerComposite* aLayer,
   // Make sure the front buffer has a compositor
   img->mFrontBuffer->SetCompositor(GetCompositor());
 
-  AutoLockCompositableHost autoLock(this);
-  if (autoLock.Failed()) {
-    NS_WARNING("failed to lock front buffer");
-    return;
-  }
-
-  if (!img->mFrontBuffer->BindTextureSource(img->mTextureSource)) {
-    return;
-  }
-
-  if (!img->mTextureSource) {
-    // BindTextureSource above should have returned false!
-    MOZ_ASSERT(false);
-    return;
-  }
-
-  bool isAlphaPremultiplied =
-      !(img->mFrontBuffer->GetFlags() & TextureFlags::NON_PREMULTIPLIED);
-  RefPtr<TexturedEffect> effect =
-      CreateTexturedEffect(img->mFrontBuffer->GetFormat(),
-          img->mTextureSource.get(), aFilter, isAlphaPremultiplied,
-          GetRenderState());
-  if (!effect) {
-    return;
-  }
-
-  if (mLastFrameID != img->mFrameID || mLastProducerID != img->mProducerID) {
-    if (mImageContainer) {
-      aLayer->GetLayerManager()->
-          AppendImageCompositeNotification(ImageCompositeNotification(
-              mImageContainer, nullptr,
-              img->mTimeStamp, GetCompositor()->GetCompositionTime(),
-              img->mFrameID, img->mProducerID));
+  {
+    AutoLockCompositableHost autoLock(this);
+    if (autoLock.Failed()) {
+      NS_WARNING("failed to lock front buffer");
+      return;
     }
-    mLastFrameID = img->mFrameID;
-    mLastProducerID = img->mProducerID;
-  }
-  aEffectChain.mPrimaryEffect = effect;
-  gfx::Rect pictureRect(0, 0, img->mPictureRect.width, img->mPictureRect.height);
-  BigImageIterator* it = img->mTextureSource->AsBigImageIterator();
-  if (it) {
 
-    // This iteration does not work if we have multiple texture sources here
-    // (e.g. 3 YCbCr textures). There's nothing preventing the different
-    // planes from having different resolutions or tile sizes. For example, a
-    // YCbCr frame could have Cb and Cr planes that are half the resolution of
-    // the Y plane, in such a way that the Y plane overflows the maximum
-    // texture size and the Cb and Cr planes do not. Then the Y plane would be
-    // split into multiple tiles and the Cb and Cr planes would just be one
-    // tile each.
-    // To handle the general case correctly, we'd have to create a grid of
-    // intersected tiles over all planes, and then draw each grid tile using
-    // the corresponding source tiles from all planes, with appropriate
-    // per-plane per-tile texture coords.
-    // DrawQuad currently assumes that all planes use the same texture coords.
-    MOZ_ASSERT(it->GetTileCount() == 1 || !img->mTextureSource->GetNextSibling(),
-               "Can't handle multi-plane BigImages");
+    if (!img->mFrontBuffer->BindTextureSource(img->mTextureSource)) {
+      return;
+    }
+
+    if (!img->mTextureSource) {
+      // BindTextureSource above should have returned false!
+      MOZ_ASSERT(false);
+      return;
+    }
+
+    bool isAlphaPremultiplied =
+        !(img->mFrontBuffer->GetFlags() & TextureFlags::NON_PREMULTIPLIED);
+    RefPtr<TexturedEffect> effect =
+        CreateTexturedEffect(img->mFrontBuffer->GetFormat(),
+            img->mTextureSource.get(), aFilter, isAlphaPremultiplied,
+            GetRenderState());
+    if (!effect) {
+      return;
+    }
+
+    if (mLastFrameID != img->mFrameID || mLastProducerID != img->mProducerID) {
+      if (mImageContainer) {
+        aLayer->GetLayerManager()->
+            AppendImageCompositeNotification(ImageCompositeNotification(
+                mImageContainer, nullptr,
+                img->mTimeStamp, GetCompositor()->GetCompositionTime(),
+                img->mFrameID, img->mProducerID));
+      }
+      mLastFrameID = img->mFrameID;
+      mLastProducerID = img->mProducerID;
+    }
+    aEffectChain.mPrimaryEffect = effect;
+    gfx::Rect pictureRect(0, 0, img->mPictureRect.width, img->mPictureRect.height);
+    BigImageIterator* it = img->mTextureSource->AsBigImageIterator();
+    if (it) {
+
+      // This iteration does not work if we have multiple texture sources here
+      // (e.g. 3 YCbCr textures). There's nothing preventing the different
+      // planes from having different resolutions or tile sizes. For example, a
+      // YCbCr frame could have Cb and Cr planes that are half the resolution of
+      // the Y plane, in such a way that the Y plane overflows the maximum
+      // texture size and the Cb and Cr planes do not. Then the Y plane would be
+      // split into multiple tiles and the Cb and Cr planes would just be one
+      // tile each.
+      // To handle the general case correctly, we'd have to create a grid of
+      // intersected tiles over all planes, and then draw each grid tile using
+      // the corresponding source tiles from all planes, with appropriate
+      // per-plane per-tile texture coords.
+      // DrawQuad currently assumes that all planes use the same texture coords.
+      MOZ_ASSERT(it->GetTileCount() == 1 || !img->mTextureSource->GetNextSibling(),
+                 "Can't handle multi-plane BigImages");
+
+      it->BeginBigImageIteration();
+      do {
+        IntRect tileRect = it->GetTileRect();
+        gfx::Rect rect(tileRect.x, tileRect.y, tileRect.width, tileRect.height);
+        rect = rect.Intersect(pictureRect);
+        effect->mTextureCoords = Rect(Float(rect.x - tileRect.x) / tileRect.width,
+                                      Float(rect.y - tileRect.y) / tileRect.height,
+                                      Float(rect.width) / tileRect.width,
+                                      Float(rect.height) / tileRect.height);
+        if (img->mFrontBuffer->GetFlags() & TextureFlags::ORIGIN_BOTTOM_LEFT) {
+          effect->mTextureCoords.y = effect->mTextureCoords.YMost();
+          effect->mTextureCoords.height = -effect->mTextureCoords.height;
+        }
+        GetCompositor()->DrawQuad(rect, aClipRect, aEffectChain,
+                                  aOpacity, aTransform);
+        GetCompositor()->DrawDiagnostics(DiagnosticFlags::IMAGE | DiagnosticFlags::BIGIMAGE,
+                                         rect, aClipRect, aTransform, mFlashCounter);
+      } while (it->NextTile());
+      it->EndBigImageIteration();
+      // layer border
+      GetCompositor()->DrawDiagnostics(DiagnosticFlags::IMAGE, pictureRect,
+                                       aClipRect, aTransform, mFlashCounter);
+    } else {
+      IntSize textureSize = img->mTextureSource->GetSize();
+      effect->mTextureCoords = Rect(Float(img->mPictureRect.x) / textureSize.width,
+                                    Float(img->mPictureRect.y) / textureSize.height,
+                                    Float(img->mPictureRect.width) / textureSize.width,
+                                    Float(img->mPictureRect.height) / textureSize.height);
 
-    it->BeginBigImageIteration();
-    do {
-      IntRect tileRect = it->GetTileRect();
-      gfx::Rect rect(tileRect.x, tileRect.y, tileRect.width, tileRect.height);
-      rect = rect.Intersect(pictureRect);
-      effect->mTextureCoords = Rect(Float(rect.x - tileRect.x) / tileRect.width,
-                                    Float(rect.y - tileRect.y) / tileRect.height,
-                                    Float(rect.width) / tileRect.width,
-                                    Float(rect.height) / tileRect.height);
       if (img->mFrontBuffer->GetFlags() & TextureFlags::ORIGIN_BOTTOM_LEFT) {
         effect->mTextureCoords.y = effect->mTextureCoords.YMost();
         effect->mTextureCoords.height = -effect->mTextureCoords.height;
       }
-      GetCompositor()->DrawQuad(rect, aClipRect, aEffectChain,
+
+      GetCompositor()->DrawQuad(pictureRect, aClipRect, aEffectChain,
                                 aOpacity, aTransform);
-      GetCompositor()->DrawDiagnostics(DiagnosticFlags::IMAGE | DiagnosticFlags::BIGIMAGE,
-                                       rect, aClipRect, aTransform, mFlashCounter);
-    } while (it->NextTile());
-    it->EndBigImageIteration();
-    // layer border
-    GetCompositor()->DrawDiagnostics(DiagnosticFlags::IMAGE, pictureRect,
-                                     aClipRect, aTransform, mFlashCounter);
-  } else {
-    IntSize textureSize = img->mTextureSource->GetSize();
-    effect->mTextureCoords = Rect(Float(img->mPictureRect.x) / textureSize.width,
-                                  Float(img->mPictureRect.y) / textureSize.height,
-                                  Float(img->mPictureRect.width) / textureSize.width,
-                                  Float(img->mPictureRect.height) / textureSize.height);
-
-    if (img->mFrontBuffer->GetFlags() & TextureFlags::ORIGIN_BOTTOM_LEFT) {
-      effect->mTextureCoords.y = effect->mTextureCoords.YMost();
-      effect->mTextureCoords.height = -effect->mTextureCoords.height;
+      GetCompositor()->DrawDiagnostics(DiagnosticFlags::IMAGE,
+                                       pictureRect, aClipRect,
+                                       aTransform, mFlashCounter);
     }
-
-    GetCompositor()->DrawQuad(pictureRect, aClipRect, aEffectChain,
-                              aOpacity, aTransform);
-    GetCompositor()->DrawDiagnostics(DiagnosticFlags::IMAGE,
-                                     pictureRect, aClipRect,
-                                     aTransform, mFlashCounter);
   }
+
+  // Update mBias last. This can change which frame ChooseImage(Index) would
+  // return, and we don't want to do that until we've finished compositing
+  // since callers of ChooseImage(Index) assume the same image will be chosen
+  // during a given composition. This must happen after autoLock's
+  // destructor!
+  mBias = UpdateBias(
+      GetCompositor()->GetCompositionTime(), mImages[imageIndex].mTimeStamp,
+      uint32_t(imageIndex + 1) < mImages.Length() ?
+          mImages[imageIndex + 1].mTimeStamp : TimeStamp(),
+      mBias);
 }
 
 void

gfx/layers/composite/ImageHost.h

@@ -91,6 +91,15 @@ public:
     return img ? img->mFrameID : -1;
   }
 
+  enum Bias {
+    // Don't apply bias to frame times
+    BIAS_NONE,
+    // Apply a negative bias to frame times to keep them before the vsync time
+    BIAS_NEGATIVE,
+    // Apply a positive bias to frame times to keep them after the vsync time
+    BIAS_POSITIVE,
+  };
+
 protected:
   struct TimedImage {
     CompositableTextureHostRef mFrontBuffer;
@@ -103,8 +112,9 @@ protected:
 
   /**
    * ChooseImage is guaranteed to return the same TimedImage every time it's
-   * called during the same composition --- it depends only on mImages and
-   * mCompositor->GetCompositionTime().
+   * called during the same composition, up to the end of Composite() ---
+   * it depends only on mImages, mCompositor->GetCompositionTime(), and mBias.
+   * mBias is updated at the end of Composite().
    */
   const TimedImage* ChooseImage() const;
   TimedImage* ChooseImage();
@@ -115,6 +125,10 @@ protected:
   ImageContainerParent* mImageContainer;
   int32_t mLastFrameID;
   int32_t mLastProducerID;
+  /**
+   * Bias to apply to the next frame.
+   */
+  Bias mBias;
 
   bool mLocked;
 };