Bug 725095 - Address GFX review comments. r=joe

gfx/layers/ipc/CompositorParent.cpp

@@ -51,11 +51,14 @@
 #include <android/log.h>
 #endif
 
+using base::Thread;
+
 namespace mozilla {
 namespace layers {
 
-CompositorParent::CompositorParent(nsIWidget* aWidget)
-  : mWidget(aWidget)
+CompositorParent::CompositorParent(nsIWidget* aWidget, base::Thread* aCompositorThread)
+  : mCompositorThread(aCompositorThread)
+  , mWidget(aWidget)
   , mCurrentCompositeTask(NULL)
   , mPaused(false)
   , mIsFirstPaint(false)
@@ -87,15 +90,17 @@ CompositorParent::RecvStop()
 }
 
 void
-CompositorParent::ScheduleRenderOnCompositorThread(::base::Thread &aCompositorThread)
+CompositorParent::ScheduleRenderOnCompositorThread()
 {
-  CancelableTask *renderTask = NewRunnableMethod(this, &CompositorParent::AsyncRender);
-  aCompositorThread.message_loop()->PostTask(FROM_HERE, renderTask);
+  CancelableTask *renderTask = NewRunnableMethod(this, &CompositorParent::ScheduleComposition);
+  mCompositorThread->message_loop()->PostTask(FROM_HERE, renderTask);
 }
 
 void
 CompositorParent::PauseComposition()
 {
+  NS_ABORT_IF_FALSE(mCompositorThread->thread_id() == PlatformThread::CurrentId(),
+                    "PauseComposition() can only be called on the compositor thread");
   if (!mPaused) {
     mPaused = true;
 
@@ -108,6 +113,8 @@ CompositorParent::PauseComposition()
 void
 CompositorParent::ResumeComposition()
 {
+  NS_ABORT_IF_FALSE(mCompositorThread->thread_id() == PlatformThread::CurrentId(),
+                    "ResumeComposition() can only be called on the compositor thread");
   mPaused = false;
 
 #ifdef MOZ_WIDGET_ANDROID
@@ -116,19 +123,19 @@ CompositorParent::ResumeComposition()
 }
 
 void
-CompositorParent::SchedulePauseOnCompositorThread(::base::Thread &aCompositorThread)
+CompositorParent::SchedulePauseOnCompositorThread()
 {
   CancelableTask *pauseTask = NewRunnableMethod(this,
                                                 &CompositorParent::PauseComposition);
-  aCompositorThread.message_loop()->PostTask(FROM_HERE, pauseTask);
+  mCompositorThread->message_loop()->PostTask(FROM_HERE, pauseTask);
 }
 
 void
-CompositorParent::ScheduleResumeOnCompositorThread(::base::Thread &aCompositorThread)
+CompositorParent::ScheduleResumeOnCompositorThread()
 {
   CancelableTask *resumeTask = NewRunnableMethod(this,
                                                  &CompositorParent::ResumeComposition);
-  aCompositorThread.message_loop()->PostTask(FROM_HERE, resumeTask);
+  mCompositorThread->message_loop()->PostTask(FROM_HERE, resumeTask);
 }
 
 void
@@ -148,6 +155,9 @@ CompositorParent::ScheduleComposition()
 #endif
 
   mCurrentCompositeTask = NewRunnableMethod(this, &CompositorParent::Composite);
+
+  // Since 60 fps is the maximum frame rate we can acheive, scheduling composition
+  // events less than 15 ms apart wastes computation..
   if (!initialComposition && delta.ToMilliseconds() < 15) {
 #ifdef COMPOSITOR_PERFORMANCE_WARNING
     mExpectedComposeTime = mozilla::TimeStamp::Now() + TimeDuration::FromMilliseconds(15 - delta.ToMilliseconds());
@@ -169,6 +179,8 @@ CompositorParent::SetTransformation(float aScale, nsIntPoint aScrollOffset)
 void
 CompositorParent::Composite()
 {
+  NS_ABORT_IF_FALSE(mCompositorThread->thread_id() == PlatformThread::CurrentId(),
+                    "Composite can only be called on the compositor thread");
   mCurrentCompositeTask = NULL;
 
   mLastCompose = mozilla::TimeStamp::Now();
@@ -204,8 +216,9 @@ CompositorParent::GetPrimaryScrollableLayer()
 
   nsTArray<Layer*> queue;
   queue.AppendElement(root);
-  for (unsigned i = 0; i < queue.Length(); i++) {
-    ContainerLayer* containerLayer = queue[i]->AsContainerLayer();
+  while (queue.Length()) {
+    ContainerLayer* containerLayer = queue[0]->AsContainerLayer();
+    queue.RemoveElementAt(0);
     if (!containerLayer) {
       continue;
     }
@@ -243,16 +256,6 @@ SetShadowProperties(Layer* aLayer)
   }
 }
 
-static double GetXScale(const gfx3DMatrix& aTransform)
-{
-  return aTransform._11;
-}
-
-static double GetYScale(const gfx3DMatrix& aTransform)
-{
-  return aTransform._22;
-}
-
 void
 CompositorParent::TransformShadowTree()
 {
@@ -265,8 +268,8 @@ CompositorParent::TransformShadowTree()
   const gfx3DMatrix& rootTransform = mLayerManager->GetRoot()->GetTransform();
   const gfx3DMatrix& currentTransform = layer->GetTransform();
 
-  float rootScaleX = GetXScale(rootTransform);
-  float rootScaleY = GetYScale(rootTransform);
+  float rootScaleX = rootTransform.GetXScale();
+  float rootScaleY = rootTransform.GetYScale();
 
   if (mIsFirstPaint && metrics) {
     nsIntPoint scrollOffset = metrics->mViewportScrollOffset;
@@ -309,21 +312,6 @@ CompositorParent::TransformShadowTree()
 #endif
 }
 
-void
-CompositorParent::AsyncRender()
-{
-  if (mPaused || !mLayerManager) {
-    return;
-  }
-
-  Layer* root = mLayerManager->GetRoot();
-  if (!root) {
-    return;
-  }
-
-  ScheduleComposition();
-}
-
 #ifdef MOZ_WIDGET_ANDROID
 void
 CompositorParent::RequestViewTransform()

gfx/layers/ipc/CompositorParent.h

@@ -43,7 +43,7 @@
 
 // Enable this pref to turn on compositor performance warning.
 // This will print warnings if the compositor isn't meeting
-// it's responsiveness objectives:
+// its responsiveness objectives:
 //    1) Compose a frame within 15ms of receiving a ScheduleCompositeCall
 //    2) Unless a frame was composited within the throttle threshold in
 //       which the deadline will be 15ms + throttle threshold
@@ -56,6 +56,10 @@
 
 class nsIWidget;
 
+namespace base {
+class Thread;
+}
+
 namespace mozilla {
 namespace layers {
 
@@ -86,7 +90,7 @@ class CompositorParent : public PCompositorParent,
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorParent)
 public:
-  CompositorParent(nsIWidget* aWidget);
+  CompositorParent(nsIWidget* aWidget, base::Thread* aCompositorThread);
   virtual ~CompositorParent();
 
   virtual bool RecvStop() MOZ_OVERRIDE;
@@ -100,9 +104,9 @@ public:
   void AsyncRender();
 
   // Can be called from any thread
-  void ScheduleRenderOnCompositorThread(::base::Thread &aCompositorThread);
-  void SchedulePauseOnCompositorThread(::base::Thread &aCompositorThread);
-  void ScheduleResumeOnCompositorThread(::base::Thread &aCompositorThread);
+  void ScheduleRenderOnCompositorThread();
+  void SchedulePauseOnCompositorThread();
+  void ScheduleResumeOnCompositorThread();
 
 protected:
   virtual PLayersParent* AllocPLayers(const LayersBackend &backendType);
@@ -132,6 +136,7 @@ private:
 #endif
 
   nsRefPtr<LayerManager> mLayerManager;
+  base::Thread* mCompositorThread;
   nsIWidget* mWidget;
   CancelableTask *mCurrentCompositeTask;
   TimeStamp mLastCompose;

gfx/thebes/gfx3DMatrix.h

@@ -181,6 +181,13 @@ public:
    */
   void Scale(float aX, float aY, float aZ);
 
+  /**
+   * Return the currently set scaling factors.
+   */
+  float GetXScale() const { return _11; }
+  float GetYScale() const { return _22; }
+  float GetZScale() const { return _33; }
+
   /**
    * Rotate around the X axis..
    *

widget/android/AndroidBridge.cpp

@@ -1861,7 +1861,7 @@ void
 AndroidBridge::ScheduleComposite()
 {
     if (mCompositorParent) {
-        mCompositorParent->ScheduleRenderOnCompositorThread(*mCompositorThread);
+        mCompositorParent->ScheduleRenderOnCompositorThread();
     }
 }
 
@@ -1869,7 +1869,7 @@ void
 AndroidBridge::SchedulePauseComposition()
 {
     if (mCompositorParent) {
-        mCompositorParent->SchedulePauseOnCompositorThread(*mCompositorThread);
+        mCompositorParent->SchedulePauseOnCompositorThread();
     }
 }
 
@@ -1877,7 +1877,7 @@ void
 AndroidBridge::ScheduleResumeComposition()
 {
     if (mCompositorParent) {
-        mCompositorParent->ScheduleResumeOnCompositorThread(*mCompositorThread);
+        mCompositorParent->ScheduleResumeOnCompositorThread();
     }
 }
 

widget/xpwidgets/nsBaseWidget.cpp

@@ -846,8 +846,8 @@ nsBaseWidget::GetShouldAccelerate()
 
 void nsBaseWidget::CreateCompositor()
 {
-  mCompositorParent = new CompositorParent(this);
   mCompositorThread = new Thread("CompositorThread");
+  mCompositorParent = new CompositorParent(this, mCompositorThread);
   if (mCompositorThread->Start()) {
     LayerManager* lm = CreateBasicLayerManager();
     MessageLoop *childMessageLoop = mCompositorThread->message_loop();