0

Revert 173406 - Crashes on Win & Mac

[cc] Fold more update calls into updateDrawProperties

Now that we have a clean update system, we can remove the manual
calling of updateRootScrollLayerImplTransform and other similar kinds of "I'm going to do something manually in a half dozen places because I can." This removes some surface area
on LTHI which helps with the LTHI->LTI push.

BUG=155209
R=enne,skyostil

Review URL: https://chromiumcodereview.appspot.com/11529006

TBR=nduca@chromium.org
Review URL: https://codereview.chromium.org/11596005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@173407 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
rsleevi@chromium.org
2012-12-17 01:57:20 +00:00
parent dd228142a7
commit 1e0f6c30ca
7 changed files with 78 additions and 73 deletions

@ -7,7 +7,6 @@
#include "cc/single_thread_proxy.h"
#include "cc/test/fake_impl_proxy.h"
#include "cc/test/fake_layer_tree_host_impl.h"
#include "cc/test/fake_output_surface.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/effects/SkBlurImageFilter.h"
@ -54,13 +53,13 @@ namespace {
#define VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(codeToTest) \
root->resetAllChangeTrackingForSubtree(); \
hostImpl.forcePrepareToDraw(); \
hostImpl.resetNeedsUpdateDrawPropertiesForTesting(); \
codeToTest; \
EXPECT_TRUE(hostImpl.needsUpdateDrawProperties());
#define VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(codeToTest) \
root->resetAllChangeTrackingForSubtree(); \
hostImpl.forcePrepareToDraw(); \
hostImpl.resetNeedsUpdateDrawPropertiesForTesting(); \
codeToTest; \
EXPECT_FALSE(hostImpl.needsUpdateDrawProperties());
@ -74,7 +73,6 @@ TEST(LayerImplTest, verifyLayerChangesAreTrackedProperly)
// Create a simple LayerImpl tree:
FakeImplProxy proxy;
FakeLayerTreeHostImpl hostImpl(&proxy);
EXPECT_TRUE(hostImpl.initializeRenderer(createFakeOutputSurface()));
scoped_ptr<LayerImpl> root = LayerImpl::create(hostImpl.activeTree(), 1);
root->addChild(LayerImpl::create(hostImpl.activeTree(), 2));
LayerImpl* child = root->children()[0];
@ -171,7 +169,6 @@ TEST(LayerImplTest, VerifyNeedsUpdateDrawProperties)
{
FakeImplProxy proxy;
FakeLayerTreeHostImpl hostImpl(&proxy);
EXPECT_TRUE(hostImpl.initializeRenderer(createFakeOutputSurface()));
scoped_ptr<LayerImpl> root = LayerImpl::create(hostImpl.activeTree(), 1);
gfx::PointF arbitraryPointF = gfx::PointF(0.125f, 0.25f);
@ -202,7 +199,7 @@ TEST(LayerImplTest, VerifyNeedsUpdateDrawProperties)
VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(root->scrollBy(arbitraryVector2d));
VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->scrollBy(gfx::Vector2d()));
root->setScrollDelta(gfx::Vector2d(0, 0));
hostImpl.forcePrepareToDraw();
hostImpl.resetNeedsUpdateDrawPropertiesForTesting();
VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(root->setScrollDelta(arbitraryVector2d));
VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setScrollDelta(arbitraryVector2d));
VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(root->setScrollOffset(arbitraryVector2d));

@ -383,6 +383,13 @@ void LayerTreeHostImpl::trackDamageForAllSurfaces(LayerImpl* rootDrawLayer, cons
}
}
void LayerTreeHostImpl::updateRootScrollLayerImplTransform()
{
if (rootScrollLayer()) {
rootScrollLayer()->setImplTransform(implTransform());
}
}
void LayerTreeHostImpl::updateDrawProperties()
{
if (!needsUpdateDrawProperties())
@ -394,16 +401,20 @@ void LayerTreeHostImpl::updateDrawProperties()
if (!rootLayer())
return;
if (!m_renderer) // For maxTextureSize.
return;
if (rootScrollLayer())
rootScrollLayer()->setImplTransform(implTransform());
calculateRenderSurfaceLayerList(m_renderSurfaceLayerList);
}
void LayerTreeHostImpl::calculateRenderSurfaceLayerList(LayerList& renderSurfaceLayerList)
{
DCHECK(renderSurfaceLayerList.empty());
DCHECK(rootLayer());
DCHECK(m_renderer); // For maxTextureSize.
{
updateRootScrollLayerImplTransform();
TRACE_EVENT0("cc", "LayerTreeHostImpl::calcDrawEtc");
float pageScaleFactor = m_pinchZoomViewport.pageScaleFactor();
LayerTreeHostCommon::calculateDrawProperties(rootLayer(), deviceViewportSize(), m_deviceScaleFactor, pageScaleFactor, rendererCapabilities().maxTextureSize, m_settings.canUseLCDText, m_renderSurfaceLayerList);
LayerTreeHostCommon::calculateDrawProperties(rootLayer(), deviceViewportSize(), m_deviceScaleFactor, pageScaleFactor, rendererCapabilities().maxTextureSize, m_settings.canUseLCDText, renderSurfaceLayerList);
}
}
@ -504,9 +515,6 @@ bool LayerTreeHostImpl::calculateRenderPasses(FrameData& frame)
DCHECK(frame.renderPasses.empty());
updateDrawProperties();
if (!canDraw())
return false;
trackDamageForAllSurfaces(rootLayer(), *frame.renderSurfaceLayerList);
TRACE_EVENT1("cc", "LayerTreeHostImpl::calculateRenderPasses", "renderSurfaceLayerList.size()", static_cast<long long unsigned>(frame.renderSurfaceLayerList->size()));
@ -769,6 +777,7 @@ void LayerTreeHostImpl::removeRenderPasses(RenderPassCuller culler, FrameData& f
bool LayerTreeHostImpl::prepareToDraw(FrameData& frame)
{
TRACE_EVENT0("cc", "LayerTreeHostImpl::prepareToDraw");
DCHECK(canDraw());
if (m_tileManager)
m_tileManager->CheckForCompletedSetPixels();
@ -1197,7 +1206,25 @@ void LayerTreeHostImpl::setNeedsRedraw()
bool LayerTreeHostImpl::ensureRenderSurfaceLayerList()
{
// TODO(enne): See http://crbug.com/164949. This function should really
// just call updateDrawProperties(), but that breaks a number of
// impl transform tests that don't expect the tree to be updated.
if (!rootLayer())
return false;
if (!m_renderer)
return false;
// We need both a non-empty render surface layer list and a root render
// surface to be able to iterate over the visible layers.
if (m_renderSurfaceLayerList.size() && rootLayer()->renderSurface())
return true;
// If we are called after setRootLayer() but before prepareToDraw(), we need
// to recalculate the visible layers. This prevents being unable to scroll
// during part of a commit.
setNeedsUpdateDrawProperties();
updateDrawProperties();
return m_renderSurfaceLayerList.size();
}

@ -52,7 +52,7 @@ public:
// PinchZoomViewport models the bounds and offset of the viewport that is used during a pinch-zoom operation.
// It tracks the layout-space dimensions of the viewport before any applied scale, and then tracks the layout-space
// coordinates of the viewport respecting the pinch settings.
class CC_EXPORT PinchZoomViewport {
class PinchZoomViewport {
public:
PinchZoomViewport();
@ -257,6 +257,8 @@ public:
void renderingStats(RenderingStats*) const;
void updateRootScrollLayerImplTransform();
void sendManagedMemoryStats(
size_t memoryVisibleBytes,
size_t memoryVisibleAndNearbyBytes,
@ -301,11 +303,18 @@ public:
template<typename RenderPassCuller>
static void removeRenderPasses(RenderPassCuller, FrameData&);
float totalPageScaleFactorForTesting() const { return m_pinchZoomViewport.totalPageScaleFactor(); }
protected:
LayerTreeHostImpl(const LayerTreeSettings&, LayerTreeHostImplClient*, Proxy*);
void animatePageScale(base::TimeTicks monotonicTime);
void animateScrollbars(base::TimeTicks monotonicTime);
void updateDrawProperties();
// Exposed for testing.
void calculateRenderSurfaceLayerList(LayerList&);
void resetNeedsUpdateDrawPropertiesForTesting() { m_needsUpdateDrawProperties = false; }
// Virtual for testing.
virtual void animateLayers(base::TimeTicks monotonicTime, base::Time wallClockTime);
@ -316,11 +325,6 @@ protected:
Proxy* m_proxy;
private:
void animatePageScale(base::TimeTicks monotonicTime);
void animateScrollbars(base::TimeTicks monotonicTime);
void updateDrawProperties();
void computeDoubleTapZoomDeltas(ScrollAndScaleSet* scrollInfo);
void computePinchZoomDeltas(ScrollAndScaleSet* scrollInfo);
void makeScrollAndScaleSet(ScrollAndScaleSet* scrollInfo, gfx::Vector2d scrollOffset, float pageScale);

@ -206,12 +206,6 @@ public:
protected:
virtual scoped_ptr<OutputSurface> createOutputSurface() { return createFakeOutputSurface(); }
void drawOneFrame() {
LayerTreeHostImpl::FrameData frameData;
m_hostImpl->prepareToDraw(frameData);
m_hostImpl->didDrawAllLayers(frameData);
}
FakeProxy m_proxy;
DebugScopedSetImplThread m_alwaysImplThread;
DebugScopedSetMainThreadBlocked m_alwaysMainThreadBlocked;
@ -1296,7 +1290,7 @@ TEST_P(LayerTreeHostImplTest, scrollRootAndChangePageScaleOnImplThread)
m_hostImpl->pinchGestureBegin();
m_hostImpl->pinchGestureUpdate(pageScale, gfx::Point());
m_hostImpl->pinchGestureEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
// The scroll delta is not scaled because the main thread did not scale.
scoped_ptr<ScrollAndScaleSet> scrollInfo = m_hostImpl->processScrollDeltas();
@ -1334,7 +1328,7 @@ TEST_P(LayerTreeHostImplTest, pageScaleDeltaAppliedToRootScrollLayerOnly)
m_hostImpl->pinchGestureBegin();
m_hostImpl->pinchGestureUpdate(newPageScale, gfx::Point());
m_hostImpl->pinchGestureEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
// The page scale delta should only be applied to the scrollable root layer.
EXPECT_EQ(root->implTransform(), newPageScaleMatrix);
@ -1382,7 +1376,7 @@ TEST_P(LayerTreeHostImplTest, scrollChildAndChangePageScaleOnMainThread)
float pageScale = 2;
m_hostImpl->setPageScaleFactorAndLimits(pageScale, 1, pageScale);
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
if (!m_hostImpl->settings().pageScalePinchZoomEnabled) {
// The scale should apply to the scroll delta.
@ -4555,7 +4549,7 @@ void LayerTreeHostImplTest::pinchZoomPanViewportForcesCommitRedraw(const float d
m_hostImpl->pinchGestureBegin();
m_hostImpl->pinchGestureUpdate(pageScale, gfx::Point());
m_hostImpl->pinchGestureEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
gfx::Transform expectedImplTransform;
expectedImplTransform.Scale(pageScale, pageScale);
@ -4627,7 +4621,7 @@ void LayerTreeHostImplTest::pinchZoomPanViewportTest(const float deviceScaleFact
m_hostImpl->pinchGestureBegin();
m_hostImpl->pinchGestureUpdate(pageScale, gfx::Point());
m_hostImpl->pinchGestureEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
gfx::Transform expectedImplTransform;
expectedImplTransform.Scale(pageScale, pageScale);
@ -4640,14 +4634,11 @@ void LayerTreeHostImplTest::pinchZoomPanViewportTest(const float deviceScaleFact
return;
gfx::Vector2d scrollDelta(5, 0);
// TODO(wjmaclean): Fix the math here so that the expectedTranslation is
// scaled instead of the scroll input.
gfx::Vector2d scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
gfx::Vector2d expectedMaxScroll(m_hostImpl->rootLayer()->maxScrollOffset());
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
gfx::Vector2dF expectedTranslation = gfx::ScaleVector2d(scrollDelta, m_hostImpl->deviceScaleFactor());
expectedImplTransform.Translate(-expectedTranslation.x(), -expectedTranslation.y());
@ -4661,11 +4652,10 @@ void LayerTreeHostImplTest::pinchZoomPanViewportTest(const float deviceScaleFact
// Test scroll in y-direction also.
scrollDelta = gfx::Vector2d(0, 5);
scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
expectedTranslation = gfx::ScaleVector2d(scrollDelta, m_hostImpl->deviceScaleFactor());
expectedImplTransform.Translate(-expectedTranslation.x(), -expectedTranslation.y());
@ -4711,7 +4701,7 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollTest(const float device
m_hostImpl->pinchGestureBegin();
m_hostImpl->pinchGestureUpdate(pageScale, gfx::Point());
m_hostImpl->pinchGestureEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
gfx::Transform expectedImplTransform;
expectedImplTransform.Scale(pageScale, pageScale);
@ -4726,13 +4716,12 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollTest(const float device
// Scroll document only: scrollDelta chosen to move document horizontally
// to its max scroll offset.
gfx::Vector2d scrollDelta(3, 0);
gfx::Vector2d scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
gfx::Vector2d expectedScrollDelta(scrollDelta);
gfx::Vector2d expectedMaxScroll(m_hostImpl->rootLayer()->maxScrollOffset());
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
// The scroll delta is not scaled because the main thread did not scale.
scoped_ptr<ScrollAndScaleSet> scrollInfo = m_hostImpl->processScrollDeltas();
@ -4744,11 +4733,10 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollTest(const float device
// Further scrolling should move the pinchZoomViewport only.
scrollDelta = gfx::Vector2d(2, 0);
scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
gfx::Vector2d expectedPanDelta(scrollDelta);
gfx::Vector2dF expectedTranslation = gfx::ScaleVector2d(expectedPanDelta, m_hostImpl->deviceScaleFactor());
@ -4764,12 +4752,11 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollTest(const float device
// Perform same test sequence in y-direction also.
// Document only scroll.
scrollDelta = gfx::Vector2d(0, 4);
scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
expectedScrollDelta += scrollDelta;
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
// The scroll delta is not scaled because the main thread did not scale.
scrollInfo = m_hostImpl->processScrollDeltas();
@ -4781,11 +4768,10 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollTest(const float device
// pinchZoomViewport scroll only.
scrollDelta = gfx::Vector2d(0, 1);
scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
expectedPanDelta = scrollDelta;
expectedTranslation = gfx::ScaleVector2d(expectedPanDelta, m_hostImpl->deviceScaleFactor());
@ -4834,7 +4820,7 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollBoundaryTest(const floa
m_hostImpl->pinchGestureBegin();
m_hostImpl->pinchGestureUpdate(pageScale, gfx::Point());
m_hostImpl->pinchGestureEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
gfx::Transform expectedImplTransform;
expectedImplTransform.Scale(pageScale, pageScale);
@ -4848,14 +4834,13 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollBoundaryTest(const floa
// Scroll document and pann zoomViewport in one scroll-delta.
gfx::Vector2d scrollDelta(5, 0);
gfx::Vector2d scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
gfx::Vector2d expectedScrollDelta(gfx::Vector2d(3, 0)); // This component gets handled by document scroll.
gfx::Vector2d expectedMaxScroll(m_hostImpl->rootLayer()->maxScrollOffset());
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
// The scroll delta is not scaled because the main thread did not scale.
scoped_ptr<ScrollAndScaleSet> scrollInfo = m_hostImpl->processScrollDeltas();
@ -4870,12 +4855,11 @@ void LayerTreeHostImplTest::pinchZoomPanViewportAndScrollBoundaryTest(const floa
// Perform same test sequence in y-direction also.
scrollDelta = gfx::Vector2d(0, 5);
scrollDeltaInZoomedViewport = ToFlooredVector2d(gfx::ScaleVector2d(scrollDelta, m_hostImpl->totalPageScaleFactorForTesting()));
expectedScrollDelta += gfx::Vector2d(0, 4); // This component gets handled by document scroll.
EXPECT_EQ(InputHandlerClient::ScrollStarted, m_hostImpl->scrollBegin(gfx::Point(0, 0), InputHandlerClient::Gesture));
m_hostImpl->scrollBy(gfx::Point(), scrollDeltaInZoomedViewport);
m_hostImpl->scrollBy(gfx::Point(), scrollDelta);
m_hostImpl->scrollEnd();
drawOneFrame();
m_hostImpl->updateRootScrollLayerImplTransform();
// The scroll delta is not scaled because the main thread did not scale.
scrollInfo = m_hostImpl->processScrollDeltas(); // This component gets handled by zoomViewport pan.

@ -1339,12 +1339,8 @@ public:
EXPECT_EQ(gfx::Point(2, 2), child->position());
// Compute all the layer transforms for the frame.
LayerTreeHostImpl::FrameData frameData;
mockImpl->prepareToDraw(frameData);
mockImpl->didDrawAllLayers(frameData);
const MockLayerTreeHostImpl::LayerList& renderSurfaceLayerList =
*frameData.renderSurfaceLayerList;
MockLayerTreeHostImpl::LayerList renderSurfaceLayerList;
mockImpl->calculateRenderSurfaceLayerList(renderSurfaceLayerList);
// Both layers should be drawing into the root render surface.
ASSERT_EQ(1u, renderSurfaceLayerList.size());

@ -16,11 +16,7 @@ class FakeLayerTreeHostImpl : public LayerTreeHostImpl {
FakeLayerTreeHostImpl(Proxy* proxy);
virtual ~FakeLayerTreeHostImpl();
void forcePrepareToDraw() {
LayerTreeHostImpl::FrameData frameData;
prepareToDraw(frameData);
didDrawAllLayers(frameData);
}
using LayerTreeHostImpl::resetNeedsUpdateDrawPropertiesForTesting;
private:
FakeLayerTreeHostImplClient client_;

@ -153,6 +153,7 @@ public:
// Make these public.
typedef std::vector<LayerImpl*> LayerList;
using LayerTreeHostImpl::calculateRenderSurfaceLayerList;
protected:
virtual void animateLayers(base::TimeTicks monotonicTime, base::Time wallClockTime) OVERRIDE;