- Revision
- 107130
- Author
- [email protected]
- Date
- 2012-02-08 13:41:47 -0800 (Wed, 08 Feb 2012)
Log Message
[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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (107129 => 107130)
--- trunk/Source/WebCore/ChangeLog 2012-02-08 21:37:31 UTC (rev 107129)
+++ trunk/Source/WebCore/ChangeLog 2012-02-08 21:41:47 UTC (rev 107130)
@@ -1,3 +1,28 @@
+2012-02-08 Shawn Singh <[email protected]>
+
+ [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 <[email protected]>
[chromium] Check that we can make the SharedGraphicsContext3D current before returning
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp (107129 => 107130)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp 2012-02-08 21:37:31 UTC (rev 107129)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp 2012-02-08 21:41:47 UTC (rev 107130)
@@ -56,13 +56,13 @@
{
}
-void CCDamageTracker::updateDamageRectForNextFrame(const Vector<RefPtr<CCLayerImpl> >& layerList, int targetSurfaceLayerID, CCLayerImpl* targetSurfaceMaskLayer)
+void CCDamageTracker::updateDamageTrackingState(const Vector<RefPtr<CCLayerImpl> >& 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 @@
// damage tracker is ready for the next frame.
//
+ // These functions cannot be bypassed with early-exits, even if we know what the
+ // damage will be for this frame, because we need to update the damage tracker state
+ // to correctly track the next frame.
+ FloatRect damageFromActiveLayers = trackDamageFromActiveLayers(layerList, targetSurfaceLayerID);
+ FloatRect damageFromSurfaceMask = trackDamageFromSurfaceMask(targetSurfaceMaskLayer);
+ FloatRect damageFromLeftoverRects = trackDamageFromLeftoverRects();
+
// If the target surface already knows its entire region is damaged, we can return early.
// FIXME: this should go away, or will be cleaner, after refactoring into RenderPass/RenderSchedule.
CCLayerImpl* layer = layerList[0].get();
- if (layer->targetRenderSurface()->surfacePropertyChangedOnlyFromDescendant()) {
- m_currentDamageRect = FloatRect(layer->targetRenderSurface()->contentRect());
- // FIXME: this early exit is incorrect: https://bugs.webkit.org/show_bug.cgi?id=76924
- return;
- }
+ CCRenderSurface* targetSurface = layer->targetRenderSurface();
- FloatRect damageFromActiveLayers = computeDamageFromActiveLayers(layerList, targetSurfaceLayerID);
- FloatRect damageFromSurfaceMask = computeDamageFromSurfaceMask(targetSurfaceMaskLayer);
- FloatRect damageFromLeftoverRects = computeDamageFromLeftoverRects();
-
- 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 @@
m_nextRectHistory->set(layerID, targetSpaceRect);
}
-FloatRect CCDamageTracker::computeDamageFromActiveLayers(const Vector<RefPtr<CCLayerImpl> >& layerList, int targetSurfaceLayerID)
+FloatRect CCDamageTracker::trackDamageFromActiveLayers(const Vector<RefPtr<CCLayerImpl> >& layerList, int targetSurfaceLayerID)
{
FloatRect damageRect = FloatRect();
@@ -177,7 +177,7 @@
return damageRect;
}
-FloatRect CCDamageTracker::computeDamageFromSurfaceMask(CCLayerImpl* targetSurfaceMaskLayer)
+FloatRect CCDamageTracker::trackDamageFromSurfaceMask(CCLayerImpl* targetSurfaceMaskLayer)
{
FloatRect damageRect = FloatRect();
@@ -193,7 +193,7 @@
return damageRect;
}
-FloatRect CCDamageTracker::computeDamageFromLeftoverRects()
+FloatRect CCDamageTracker::trackDamageFromLeftoverRects()
{
// After computing damage for all active layers, any leftover items in the current
// rect history correspond to layers/surfaces that no longer exist. So, these regions
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.h (107129 => 107130)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.h 2012-02-08 21:37:31 UTC (rev 107129)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.h 2012-02-08 21:41:47 UTC (rev 107130)
@@ -44,20 +44,20 @@
~CCDamageTracker();
void forceFullDamageNextUpdate() { m_forceFullDamageNextUpdate = true; }
- void updateDamageRectForNextFrame(const Vector<RefPtr<CCLayerImpl> >& layerList, int targetSurfaceLayerID, CCLayerImpl* targetSurfaceMaskLayer);
+ void updateDamageTrackingState(const Vector<RefPtr<CCLayerImpl> >& layerList, int targetSurfaceLayerID, CCLayerImpl* targetSurfaceMaskLayer);
const FloatRect& currentDamageRect() { return m_currentDamageRect; }
private:
CCDamageTracker();
- FloatRect computeDamageFromActiveLayers(const Vector<RefPtr<CCLayerImpl> >& layerList, int targetSurfaceLayerID);
- FloatRect computeDamageFromSurfaceMask(CCLayerImpl* targetSurfaceMaskLayer);
- FloatRect computeDamageFromLeftoverRects();
+ FloatRect trackDamageFromActiveLayers(const Vector<RefPtr<CCLayerImpl> >& 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);
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp (107129 => 107130)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-02-08 21:37:31 UTC (rev 107129)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-02-08 21:41:47 UTC (rev 107130)
@@ -148,7 +148,7 @@
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());
}
}
Modified: trunk/Source/WebKit/chromium/ChangeLog (107129 => 107130)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-02-08 21:37:31 UTC (rev 107129)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-02-08 21:41:47 UTC (rev 107130)
@@ -1,3 +1,15 @@
+2012-02-08 Shawn Singh <[email protected]>
+
+ [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 <[email protected]>
Add support for pinch gesture processing in the MT compositor.
Modified: trunk/Source/WebKit/chromium/tests/CCDamageTrackerTest.cpp (107129 => 107130)
--- trunk/Source/WebKit/chromium/tests/CCDamageTrackerTest.cpp 2012-02-08 21:37:31 UTC (rev 107129)
+++ trunk/Source/WebKit/chromium/tests/CCDamageTrackerTest.cpp 2012-02-08 21:41:47 UTC (rev 107130)
@@ -70,7 +70,7 @@
// 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();
