Diff
Modified: branches/chromium/1132/Source/WebCore/ChangeLog (117086 => 117087)
--- branches/chromium/1132/Source/WebCore/ChangeLog 2012-05-15 16:59:12 UTC (rev 117086)
+++ branches/chromium/1132/Source/WebCore/ChangeLog 2012-05-15 17:11:25 UTC (rev 117087)
@@ -1,3 +1,35 @@
+2012-05-09 Dana Jansens <[email protected]>
+
+ [chromium] Don't draw when canDraw() is false
+ https://bugs.webkit.org/show_bug.cgi?id=85829
+
+ Reviewed by Adrienne Walker.
+
+ This is based on the work of Daniel Sievers in bug
+ https://bugs.webkit.org/show_bug.cgi?id=82680. When canDraw() is false,
+ we should not call drawLayers() or prepareToDraw() in both Single- and
+ Multi-Threaded mode.
+
+ drawLayers() is crashing in single threaded mode, and this attempts to
+ prevent it from being called with invalid state. While making it behave
+ properly in single-threaded mode, it seems appropriate to unrevert the
+ parts of 82680 that made threaded mode behave similarly appropriately.
+
+ A single-threaded test is not included since LTHTests is unable to run
+ in single-threaded mode at this time (pending work from Ian Vollick). So
+ we test in threaded mode only with a note to include a single thread
+ version.
+
+ Tests: CCLayerTreeHostTestCanDrawBlocksDrawing.runMultiThread
+
+ * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:
+ (WebCore::CCLayerTreeHostImpl::prepareToDraw):
+ (WebCore::CCLayerTreeHostImpl::drawLayers):
+ * platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:
+ (WebCore::CCSingleThreadProxy::doComposite):
+ * platform/graphics/chromium/cc/CCThreadProxy.cpp:
+ (WebCore::CCThreadProxy::scheduledActionDrawAndSwapInternal):
+
2012-05-08 Dana Jansens <[email protected]>
[chromium] Reflections with masks should not occlude
Modified: branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp (117086 => 117087)
--- branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-05-15 16:59:12 UTC (rev 117086)
+++ branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-05-15 17:11:25 UTC (rev 117087)
@@ -380,13 +380,11 @@
bool CCLayerTreeHostImpl::prepareToDraw(FrameData& frame)
{
TRACE_EVENT("CCLayerTreeHostImpl::prepareToDraw", this, 0);
+ ASSERT(canDraw());
frame.renderPasses.clear();
frame.renderSurfaceLayerList.clear();
- if (!m_rootLayerImpl)
- return false;
-
if (!calculateRenderPasses(frame.renderPasses, frame.renderSurfaceLayerList)) {
// We're missing textures on an animating layer. Request a commit.
m_client->setNeedsCommitOnImplThread();
@@ -405,11 +403,8 @@
void CCLayerTreeHostImpl::drawLayers(const FrameData& frame)
{
TRACE_EVENT("CCLayerTreeHostImpl::drawLayers", this, 0);
- ASSERT(m_layerRenderer);
+ ASSERT(canDraw());
- if (!m_rootLayerImpl)
- return;
-
// FIXME: use the frame begin time from the overall compositor scheduler.
// This value is currently inaccessible because it is up in Chromium's
// RenderWidget.
Modified: branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp (117086 => 117087)
--- branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp 2012-05-15 16:59:12 UTC (rev 117086)
+++ branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp 2012-05-15 17:11:25 UTC (rev 117087)
@@ -325,8 +325,14 @@
DebugScopedSetImplThread impl;
double monotonicTime = monotonicallyIncreasingTime();
double wallClockTime = currentTime();
+ m_layerTreeHostImpl->animate(monotonicTime, wallClockTime);
- m_layerTreeHostImpl->animate(monotonicTime, wallClockTime);
+ // We guard prepareToDraw() with canDraw() because it always returns a valid frame, so can only
+ // be used when such a frame is possible. Since drawLayers() depends on the result of
+ // prepareToDraw(), it is guarded on canDraw() as well.
+ if (!m_layerTreeHostImpl->canDraw())
+ return false;
+
CCLayerTreeHostImpl::FrameData frame;
m_layerTreeHostImpl->prepareToDraw(frame);
m_layerTreeHostImpl->drawLayers(frame);
Modified: branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp (117086 => 117087)
--- branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp 2012-05-15 16:59:12 UTC (rev 117086)
+++ branches/chromium/1132/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp 2012-05-15 17:11:25 UTC (rev 117087)
@@ -620,8 +620,18 @@
m_inputHandlerOnImplThread->animate(monotonicTime);
m_layerTreeHostImpl->animate(monotonicTime, wallClockTime);
+
+ // This method is called on a forced draw, regardless of whether we are able to produce a frame,
+ // as the calling site on main thread is blocked until its request completes, and we signal
+ // completion here. If canDraw() is false, we will indicate success=false to the caller, but we
+ // must still signal completion to avoid deadlock.
+
+ // We guard prepareToDraw() with canDraw() because it always returns a valid frame, so can only
+ // be used when such a frame is possible. Since drawLayers() depends on the result of
+ // prepareToDraw(), it is guarded on canDraw() as well.
+
CCLayerTreeHostImpl::FrameData frame;
- bool drawFrame = m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw;
+ bool drawFrame = m_layerTreeHostImpl->canDraw() && (m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw);
if (drawFrame) {
m_layerTreeHostImpl->drawLayers(frame);
result.didDraw = true;
@@ -630,9 +640,11 @@
// Check for a pending compositeAndReadback.
if (m_readbackRequestOnImplThread) {
- ASSERT(drawFrame); // This should be a forcedDraw
- m_layerTreeHostImpl->readback(m_readbackRequestOnImplThread->pixels, m_readbackRequestOnImplThread->rect);
- m_readbackRequestOnImplThread->success = !m_layerTreeHostImpl->isContextLost();
+ m_readbackRequestOnImplThread->success = false;
+ if (drawFrame) {
+ m_layerTreeHostImpl->readback(m_readbackRequestOnImplThread->pixels, m_readbackRequestOnImplThread->rect);
+ m_readbackRequestOnImplThread->success = !m_layerTreeHostImpl->isContextLost();
+ }
m_readbackRequestOnImplThread->completion.signal();
m_readbackRequestOnImplThread = 0;
}
@@ -642,7 +654,6 @@
// Process any finish request
if (m_finishAllRenderingCompletionEventOnImplThread) {
- ASSERT(drawFrame); // This should be a forcedDraw
m_layerTreeHostImpl->finishAllRendering();
m_finishAllRenderingCompletionEventOnImplThread->signal();
m_finishAllRenderingCompletionEventOnImplThread = 0;
@@ -654,7 +665,6 @@
m_mainThreadProxy->postTask(createCCThreadTask(this, &CCThreadProxy::didCommitAndDrawFrame));
}
- ASSERT(drawFrame || (!drawFrame && !forcedDraw));
return result;
}
Modified: branches/chromium/1132/Source/WebKit/chromium/ChangeLog (117086 => 117087)
--- branches/chromium/1132/Source/WebKit/chromium/ChangeLog 2012-05-15 16:59:12 UTC (rev 117086)
+++ branches/chromium/1132/Source/WebKit/chromium/ChangeLog 2012-05-15 17:11:25 UTC (rev 117087)
@@ -1,3 +1,24 @@
+2012-05-09 Dana Jansens <[email protected]>
+
+ [chromium] Don't draw when canDraw() is false
+ https://bugs.webkit.org/show_bug.cgi?id=85829
+
+ Reviewed by Adrienne Walker.
+
+ * tests/CCLayerTreeHostImplTest.cpp:
+ (WebKitTests::CCLayerTreeHostImplTest::CCLayerTreeHostImplTest):
+ (WebKitTests::TEST_F):
+ * tests/CCLayerTreeHostTest.cpp:
+ (WTF):
+ (CCLayerTreeHostTestCanDrawBlocksDrawing):
+ (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::CCLayerTreeHostTestCanDrawBlocksDrawing):
+ (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::beginTest):
+ (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::commitCompleteOnCCThread):
+ (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::drawLayersOnCCThread):
+ (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::didCommitAndDrawFrame):
+ (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::afterTest):
+ (WTF::TEST_F):
+
2012-05-08 Dana Jansens <[email protected]>
[chromium] Reflections with masks should not occlude
Modified: branches/chromium/1132/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp (117086 => 117087)
--- branches/chromium/1132/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp 2012-05-15 16:59:12 UTC (rev 117086)
+++ branches/chromium/1132/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp 2012-05-15 17:11:25 UTC (rev 117087)
@@ -59,6 +59,8 @@
{
CCSettings settings;
m_hostImpl = CCLayerTreeHostImpl::create(settings, this);
+ m_hostImpl->initializeLayerRenderer(createContext());
+ m_hostImpl->setViewportSize(IntSize(10, 10));
}
virtual void didLoseContextOnImplThread() OVERRIDE { }
@@ -254,8 +256,6 @@
TEST_F(CCLayerTreeHostImplTest, nonFastScrollableRegionWithOffset)
{
- m_hostImpl->initializeLayerRenderer(createContext());
-
OwnPtr<CCLayerImpl> root = CCLayerImpl::create(0);
root->setScrollable(true);
root->setScrollPosition(IntPoint(0, 0));
@@ -440,49 +440,51 @@
TEST_F(CCLayerTreeHostImplTest, didDrawNotCalledOnHiddenLayer)
{
- m_hostImpl->initializeLayerRenderer(createContext());
-
- // Ensure visibleLayerRect for root layer is empty
- m_hostImpl->setViewportSize(IntSize(0, 0));
-
+ // The root layer is always drawn, so run this test on a child layer that
+ // will be masked out by the root layer's bounds.
m_hostImpl->setRootLayer(DidDrawCheckLayer::create(0));
DidDrawCheckLayer* root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
+ root->setMasksToBounds(true);
+ root->addChild(DidDrawCheckLayer::create(1));
+ DidDrawCheckLayer* layer = static_cast<DidDrawCheckLayer*>(root->children()[0].get());
+ // Ensure visibleLayerRect for layer is empty
+ layer->setPosition(FloatPoint(100, 100));
+ layer->setBounds(IntSize(10, 10));
+ layer->setContentBounds(IntSize(10, 10));
+
CCLayerTreeHostImpl::FrameData frame;
- EXPECT_FALSE(root->willDrawCalled());
- EXPECT_FALSE(root->didDrawCalled());
+ EXPECT_FALSE(layer->willDrawCalled());
+ EXPECT_FALSE(layer->didDrawCalled());
EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
m_hostImpl->drawLayers(frame);
m_hostImpl->didDrawAllLayers(frame);
- EXPECT_FALSE(root->willDrawCalled());
- EXPECT_FALSE(root->didDrawCalled());
+ EXPECT_FALSE(layer->willDrawCalled());
+ EXPECT_FALSE(layer->didDrawCalled());
- EXPECT_TRUE(root->visibleLayerRect().isEmpty());
+ EXPECT_TRUE(layer->visibleLayerRect().isEmpty());
- // Ensure visibleLayerRect for root layer is not empty
- m_hostImpl->setViewportSize(IntSize(10, 10));
+ // Ensure visibleLayerRect for layer layer is not empty
+ layer->setPosition(FloatPoint(0, 0));
- EXPECT_FALSE(root->willDrawCalled());
- EXPECT_FALSE(root->didDrawCalled());
+ EXPECT_FALSE(layer->willDrawCalled());
+ EXPECT_FALSE(layer->didDrawCalled());
EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
m_hostImpl->drawLayers(frame);
m_hostImpl->didDrawAllLayers(frame);
- EXPECT_TRUE(root->willDrawCalled());
- EXPECT_TRUE(root->didDrawCalled());
+ EXPECT_TRUE(layer->willDrawCalled());
+ EXPECT_TRUE(layer->didDrawCalled());
- EXPECT_FALSE(root->visibleLayerRect().isEmpty());
+ EXPECT_FALSE(layer->visibleLayerRect().isEmpty());
}
TEST_F(CCLayerTreeHostImplTest, didDrawCalledOnAllLayers)
{
- m_hostImpl->initializeLayerRenderer(createContext());
- m_hostImpl->setViewportSize(IntSize(10, 10));
-
m_hostImpl->setRootLayer(DidDrawCheckLayer::create(0));
DidDrawCheckLayer* root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
@@ -533,9 +535,6 @@
TEST_F(CCLayerTreeHostImplTest, prepareToDrawFailsWhenAnimationUsesCheckerboard)
{
- m_hostImpl->initializeLayerRenderer(createContext());
- m_hostImpl->setViewportSize(IntSize(10, 10));
-
// When the texture is not missing, we draw as usual.
m_hostImpl->setRootLayer(DidDrawCheckLayer::create(0));
DidDrawCheckLayer* root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
@@ -660,9 +659,6 @@
// https://bugs.webkit.org/show_bug.cgi?id=75783
TEST_F(CCLayerTreeHostImplTest, blendingOffWhenDrawingOpaqueLayers)
{
- m_hostImpl->initializeLayerRenderer(createContext());
- m_hostImpl->setViewportSize(IntSize(10, 10));
-
{
OwnPtr<CCLayerImpl> root = CCLayerImpl::create(0);
root->setAnchorPoint(FloatPoint(0, 0));
@@ -878,7 +874,6 @@
TEST_F(CCLayerTreeHostImplTest, viewportCovered)
{
- m_hostImpl->initializeLayerRenderer(createContext());
m_hostImpl->setBackgroundColor(Color::gray);
IntSize viewportSize(1000, 1000);
@@ -990,7 +985,6 @@
ReshapeTrackerContext* reshapeTracker = new ReshapeTrackerContext();
RefPtr<GraphicsContext3D> context = GraphicsContext3DPrivate::createGraphicsContextFromWebContext(adoptPtr(reshapeTracker), GraphicsContext3D::RenderDirectlyToHostWindow);
m_hostImpl->initializeLayerRenderer(context);
- m_hostImpl->setViewportSize(IntSize(10, 10));
CCLayerImpl* root = new FakeDrawableCCLayerImpl(1);
root->setAnchorPoint(FloatPoint(0, 0));
@@ -1125,9 +1119,6 @@
TEST_F(CCLayerTreeHostImplTest, contextLostAndRestoredNotificationSentToAllLayers)
{
- m_hostImpl->initializeLayerRenderer(createContext());
- m_hostImpl->setViewportSize(IntSize(10, 10));
-
m_hostImpl->setRootLayer(ContextLostNotificationCheckLayer::create(0));
ContextLostNotificationCheckLayer* root = static_cast<ContextLostNotificationCheckLayer*>(m_hostImpl->rootLayer());
@@ -1172,9 +1163,6 @@
TEST_F(CCLayerTreeHostImplTest, scrollbarLayerLostContext)
{
- m_hostImpl->initializeLayerRenderer(createContext());
- m_hostImpl->setViewportSize(IntSize(10, 10));
-
m_hostImpl->setRootLayer(ScrollbarLayerFakePaint::create(0));
ScrollbarLayerFakePaint* scrollbar = static_cast<ScrollbarLayerFakePaint*>(m_hostImpl->rootLayer());
scrollbar->setBounds(IntSize(1, 1));
@@ -1322,9 +1310,6 @@
TEST_F(CCLayerTreeHostImplTest, dontUseOldResourcesAfterLostContext)
{
- m_hostImpl->initializeLayerRenderer(createContext());
- m_hostImpl->setViewportSize(IntSize(10, 10));
-
OwnPtr<CCLayerImpl> rootLayer(CCLayerImpl::create(0));
rootLayer->setBounds(IntSize(10, 10));
rootLayer->setAnchorPoint(FloatPoint(0, 0));
Modified: branches/chromium/1132/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp (117086 => 117087)
--- branches/chromium/1132/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp 2012-05-15 16:59:12 UTC (rev 117086)
+++ branches/chromium/1132/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp 2012-05-15 17:11:25 UTC (rev 117087)
@@ -840,7 +840,63 @@
runTestThreaded();
}
+// If the layerTreeHost says it can't draw, then we should not try to draw.
+// FIXME: Make this run in single threaded mode too. http://crbug.com/127481
+class CCLayerTreeHostTestCanDrawBlocksDrawing : public CCLayerTreeHostTestThreadOnly {
+public:
+ CCLayerTreeHostTestCanDrawBlocksDrawing()
+ : m_numCommits(0)
+ {
+ }
+ virtual void beginTest()
+ {
+ }
+
+ virtual void drawLayersOnCCThread(CCLayerTreeHostImpl* impl)
+ {
+ // Only the initial draw should bring us here.
+ EXPECT_TRUE(impl->canDraw());
+ EXPECT_EQ(0, impl->sourceFrameNumber());
+ }
+
+ virtual void commitCompleteOnCCThread(CCLayerTreeHostImpl* impl)
+ {
+ if (m_numCommits >= 1) {
+ // After the first commit, we should not be able to draw.
+ EXPECT_FALSE(impl->canDraw());
+ }
+ }
+
+ virtual void didCommitAndDrawFrame()
+ {
+ m_numCommits++;
+ if (m_numCommits == 1) {
+ // Make the viewport empty so the host says it can't draw.
+ m_layerTreeHost->setViewportSize(IntSize(0, 0));
+
+ OwnArrayPtr<char> pixels(adoptArrayPtr(new char[4]));
+ m_layerTreeHost->compositeAndReadback(static_cast<void*>(pixels.get()), IntRect(0, 0, 1, 1));
+ } else if (m_numCommits == 2) {
+ m_layerTreeHost->setNeedsCommit();
+ m_layerTreeHost->finishAllRendering();
+ endTest();
+ }
+ }
+
+ virtual void afterTest()
+ {
+ }
+
+private:
+ int m_numCommits;
+};
+
+TEST_F(CCLayerTreeHostTestCanDrawBlocksDrawing, runMultiThread)
+{
+ runTestThreaded();
+}
+
// beginLayerWrite should prevent draws from executing until a commit occurs
class CCLayerTreeHostTestWriteLayersRedraw : public CCLayerTreeHostTestThreadOnly {
public:
