From 71fc64fb2f56e3283eedf8ea28eeffbb0df05278 Mon Sep 17 00:00:00 2001 From: Ali Juma Date: Mon, 12 Mar 2012 16:32:02 -0400 Subject: [PATCH] Bug 725095 - Address GFX review comments. r=joe --- gfx/layers/ipc/CompositorParent.cpp | 64 ++++++++++++----------------- gfx/layers/ipc/CompositorParent.h | 15 ++++--- gfx/thebes/gfx3DMatrix.h | 7 ++++ widget/android/AndroidBridge.cpp | 6 +-- widget/xpwidgets/nsBaseWidget.cpp | 2 +- 5 files changed, 47 insertions(+), 47 deletions(-) diff --git a/gfx/layers/ipc/CompositorParent.cpp b/gfx/layers/ipc/CompositorParent.cpp index cdc5e50f5d05..e1fbb0277596 100644 --- a/gfx/layers/ipc/CompositorParent.cpp +++ b/gfx/layers/ipc/CompositorParent.cpp @@ -51,11 +51,14 @@ #include #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 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() diff --git a/gfx/layers/ipc/CompositorParent.h b/gfx/layers/ipc/CompositorParent.h index b102d410be3f..6aa65fae7f8e 100644 --- a/gfx/layers/ipc/CompositorParent.h +++ b/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 mLayerManager; + base::Thread* mCompositorThread; nsIWidget* mWidget; CancelableTask *mCurrentCompositeTask; TimeStamp mLastCompose; diff --git a/gfx/thebes/gfx3DMatrix.h b/gfx/thebes/gfx3DMatrix.h index de8656283bf6..aa28c7235c2f 100644 --- a/gfx/thebes/gfx3DMatrix.h +++ b/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.. * diff --git a/widget/android/AndroidBridge.cpp b/widget/android/AndroidBridge.cpp index 52d26c0c0039..6e7fec4c5912 100644 --- a/widget/android/AndroidBridge.cpp +++ b/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(); } } diff --git a/widget/xpwidgets/nsBaseWidget.cpp b/widget/xpwidgets/nsBaseWidget.cpp index e48d05cc8113..47c4a8255fe0 100644 --- a/widget/xpwidgets/nsBaseWidget.cpp +++ b/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();