From 2feb6e206ee0fdb524fbac977a9543ab6a23d5b1 Mon Sep 17 00:00:00 2001 From: "shawnsingh@chromium.org" Date: Wed, 8 Feb 2012 21:41:47 +0000 Subject: [PATCH] [chromium] Remove incorrect early exit in CCDamageTracker https://bugs.webkit.org/show_bug.cgi?id=76924 Reviewed by James Robinson. Source/WebCore: New unit test added to CCDamageTrackerTest.cpp This patch does three things: (1) adds unit test that demonstrates that early exiting in CCDamageTracker is wrong, (2) removes the early exit and cleans up the surrounding code, and (3) re-names several functions in CCDamageTracker so that state updating is implied by the name, and not just a bad side-effect of the functions. * platform/graphics/chromium/cc/CCDamageTracker.cpp: (WebCore::CCDamageTracker::updateDamageTrackingState): (WebCore::CCDamageTracker::trackDamageFromActiveLayers): (WebCore::CCDamageTracker::trackDamageFromSurfaceMask): (WebCore::CCDamageTracker::trackDamageFromLeftoverRects): * platform/graphics/chromium/cc/CCDamageTracker.h: (CCDamageTracker): * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp: (WebCore::CCLayerTreeHostImpl::trackDamageForAllSurfaces): Source/WebKit/chromium: * tests/CCDamageTrackerTest.cpp: (WebKitTests::emulateDrawingOneFrame): (WebKitTests::TEST_F): (WebKitTests): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107130 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 25 ++++++++++++++ .../graphics/chromium/cc/CCDamageTracker.cpp | 38 +++++++++++----------- .../graphics/chromium/cc/CCDamageTracker.h | 10 +++--- .../graphics/chromium/cc/CCLayerTreeHostImpl.cpp | 2 +- Source/WebKit/chromium/ChangeLog | 12 +++++++ .../WebKit/chromium/tests/CCDamageTrackerTest.cpp | 2 +- 6 files changed, 63 insertions(+), 26 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index c81ae19..1232dae 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,28 @@ +2012-02-08 Shawn Singh + + [chromium] Remove incorrect early exit in CCDamageTracker + https://bugs.webkit.org/show_bug.cgi?id=76924 + + Reviewed by James Robinson. + + New unit test added to CCDamageTrackerTest.cpp + + This patch does three things: (1) adds unit test that demonstrates + that early exiting in CCDamageTracker is wrong, (2) removes the + early exit and cleans up the surrounding code, and (3) re-names + several functions in CCDamageTracker so that state updating is + implied by the name, and not just a bad side-effect of the functions. + + * platform/graphics/chromium/cc/CCDamageTracker.cpp: + (WebCore::CCDamageTracker::updateDamageTrackingState): + (WebCore::CCDamageTracker::trackDamageFromActiveLayers): + (WebCore::CCDamageTracker::trackDamageFromSurfaceMask): + (WebCore::CCDamageTracker::trackDamageFromLeftoverRects): + * platform/graphics/chromium/cc/CCDamageTracker.h: + (CCDamageTracker): + * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp: + (WebCore::CCLayerTreeHostImpl::trackDamageForAllSurfaces): + 2012-02-08 James Robinson [chromium] Check that we can make the SharedGraphicsContext3D current before returning diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp index 6e562e1..2d6f43f 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp @@ -56,13 +56,13 @@ CCDamageTracker::~CCDamageTracker() { } -void CCDamageTracker::updateDamageRectForNextFrame(const Vector >& layerList, int targetSurfaceLayerID, CCLayerImpl* targetSurfaceMaskLayer) +void CCDamageTracker::updateDamageTrackingState(const Vector >& layerList, int targetSurfaceLayerID, CCLayerImpl* targetSurfaceMaskLayer) { // - // This function computes the "damage rect" of a target surface. The damage - // rect is the region of the surface that may have changed and needs to be redrawn. - // This can be used to scissor what is actually drawn, to save GPU computation and - // bandwidth. + // This function computes the "damage rect" of a target surface, and updates the state + // that is used to correctly track damage across frames. The damage rect is the region + // of the surface that may have changed and needs to be redrawn. This can be used to + // scissor what is actually drawn, to save GPU computation and bandwidth. // // The surface's damage rect is computed as the union of all possible changes that // have happened to the surface since the last frame was drawn. This includes: @@ -121,23 +121,23 @@ void CCDamageTracker::updateDamageRectForNextFrame(const VectortargetRenderSurface()->surfacePropertyChangedOnlyFromDescendant()) { - m_currentDamageRect = FloatRect(layer->targetRenderSurface()->contentRect()); - // FIXME: this early exit is incorrect: https://bugs.webkit.org/show_bug.cgi?id=76924 - return; - } - - FloatRect damageFromActiveLayers = computeDamageFromActiveLayers(layerList, targetSurfaceLayerID); - FloatRect damageFromSurfaceMask = computeDamageFromSurfaceMask(targetSurfaceMaskLayer); - FloatRect damageFromLeftoverRects = computeDamageFromLeftoverRects(); + CCRenderSurface* targetSurface = layer->targetRenderSurface(); - if (m_forceFullDamageNextUpdate) { - m_currentDamageRect = FloatRect(layer->targetRenderSurface()->contentRect()); + if (m_forceFullDamageNextUpdate || targetSurface->surfacePropertyChangedOnlyFromDescendant()) { + m_currentDamageRect = FloatRect(targetSurface->contentRect()); m_forceFullDamageNextUpdate = false; } else { + // FIXME: can we need to clamp this damage to the surface's content rect? (affects performance, but not correctness) m_currentDamageRect = damageFromActiveLayers; m_currentDamageRect.uniteIfNonZero(damageFromSurfaceMask); m_currentDamageRect.uniteIfNonZero(damageFromLeftoverRects); @@ -161,7 +161,7 @@ void CCDamageTracker::saveRectForNextFrame(int layerID, const FloatRect& targetS m_nextRectHistory->set(layerID, targetSpaceRect); } -FloatRect CCDamageTracker::computeDamageFromActiveLayers(const Vector >& layerList, int targetSurfaceLayerID) +FloatRect CCDamageTracker::trackDamageFromActiveLayers(const Vector >& layerList, int targetSurfaceLayerID) { FloatRect damageRect = FloatRect(); @@ -177,7 +177,7 @@ FloatRect CCDamageTracker::computeDamageFromActiveLayers(const Vector >& layerList, int targetSurfaceLayerID, CCLayerImpl* targetSurfaceMaskLayer); + void updateDamageTrackingState(const Vector >& layerList, int targetSurfaceLayerID, CCLayerImpl* targetSurfaceMaskLayer); const FloatRect& currentDamageRect() { return m_currentDamageRect; } private: CCDamageTracker(); - FloatRect computeDamageFromActiveLayers(const Vector >& layerList, int targetSurfaceLayerID); - FloatRect computeDamageFromSurfaceMask(CCLayerImpl* targetSurfaceMaskLayer); - FloatRect computeDamageFromLeftoverRects(); + FloatRect trackDamageFromActiveLayers(const Vector >& layerList, int targetSurfaceLayerID); + FloatRect trackDamageFromSurfaceMask(CCLayerImpl* targetSurfaceMaskLayer); + FloatRect trackDamageFromLeftoverRects(); FloatRect removeRectFromCurrentFrame(int layerID); void saveRectForNextFrame(int layerID, const FloatRect& targetSpaceRect); - // These helper functions are used only in computeDamageFromActiveLayers(). + // These helper functions are used only in trackDamageFromActiveLayers(). void extendDamageForLayer(CCLayerImpl*, FloatRect& targetDamageRect); void extendDamageForRenderSurface(CCLayerImpl*, FloatRect& targetDamageRect); diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp index e930bc2..088377c 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp @@ -148,7 +148,7 @@ void CCLayerTreeHostImpl::trackDamageForAllSurfaces(CCLayerImpl* rootDrawLayer, CCLayerImpl* renderSurfaceLayer = renderSurfaceLayerList[surfaceIndex].get(); CCRenderSurface* renderSurface = renderSurfaceLayer->renderSurface(); ASSERT(renderSurface); - renderSurface->damageTracker()->updateDamageRectForNextFrame(renderSurface->layerList(), renderSurfaceLayer->id(), renderSurfaceLayer->maskLayer()); + renderSurface->damageTracker()->updateDamageTrackingState(renderSurface->layerList(), renderSurfaceLayer->id(), renderSurfaceLayer->maskLayer()); } } diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog index bf2acbc..acedbad 100644 --- a/Source/WebKit/chromium/ChangeLog +++ b/Source/WebKit/chromium/ChangeLog @@ -1,3 +1,15 @@ +2012-02-08 Shawn Singh + + [chromium] Remove incorrect early exit in CCDamageTracker + https://bugs.webkit.org/show_bug.cgi?id=76924 + + Reviewed by James Robinson. + + * tests/CCDamageTrackerTest.cpp: + (WebKitTests::emulateDrawingOneFrame): + (WebKitTests::TEST_F): + (WebKitTests): + 2012-02-08 Sadrul Habib Chowdhury Add support for pinch gesture processing in the MT compositor. diff --git a/Source/WebKit/chromium/tests/CCDamageTrackerTest.cpp b/Source/WebKit/chromium/tests/CCDamageTrackerTest.cpp index 24fcfba..a9f7b1c 100644 --- a/Source/WebKit/chromium/tests/CCDamageTrackerTest.cpp +++ b/Source/WebKit/chromium/tests/CCDamageTrackerTest.cpp @@ -70,7 +70,7 @@ void emulateDrawingOneFrame(CCLayerImpl* root) // Iterate back-to-front, so that damage correctly propagates from descendant surfaces to ancestors. for (int i = renderSurfaceLayerList.size() - 1; i >= 0; --i) { CCRenderSurface* targetSurface = renderSurfaceLayerList[i]->renderSurface(); - targetSurface->damageTracker()->updateDamageRectForNextFrame(targetSurface->layerList(), targetSurface->owningLayerId(), renderSurfaceLayerList[i]->maskLayer()); + targetSurface->damageTracker()->updateDamageTrackingState(targetSurface->layerList(), targetSurface->owningLayerId(), renderSurfaceLayerList[i]->maskLayer()); } root->resetAllChangeTrackingForSubtree(); -- 2.7.4