[prev in list] [next in list] [prev in thread] [next in thread]
List: webkit-changes
Subject: [webkit-changes] [109263] trunk/Source
From: commit-queue () webkit ! org
Date: 2012-02-29 22:35:01
Message-ID: 20120229223501.DFB1557D89E1 () lists ! macosforge ! org
[Download RAW message or body]
[Attachment #2 (text/html)]
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[109263] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: \
verdana,arial,helvetica,sans-serif; font-size: 10pt; } #msg dl a { font-weight: \
bold} #msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: \
bold; } #msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: \
6px; } #logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em \
0; } #logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg \
h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; } \
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; \
} #logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: \
-1.5em; padding-left: 1.5em; } #logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em \
1em 0 1em; background: white;} #logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid \
#fa0; border-bottom: 1px solid #fa0; background: #fff; } #logmsg table th { \
text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted \
#fa0; } #logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: \
0.2em 0.5em; } #logmsg table thead th { text-align: center; border-bottom: 1px solid \
#fa0; } #logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: \
6px; } #patch { width: 100%; }
#patch h4 {font-family: \
verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, \
#patch .copfile {border:1px solid #ccc;margin:10px 0;} #patch ins \
{background:#dfd;text-decoration:none;display:block;padding:0 10px;} #patch del \
{background:#fdd;text-decoration:none;display:block;padding:0 10px;} #patch .lines, \
.info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a \
href="http://trac.webkit.org/projects/webkit/changeset/109263">109263</a></dd> \
<dt>Author</dt> <dd>commit-queue@webkit.org</dd> <dt>Date</dt> <dd>2012-02-29 \
14:35:01 -0800 (Wed, 29 Feb 2012)</dd> </dl>
<h3>Log Message</h3>
<pre>[chromium] Don't let invalidation for next frame prevent tile upload
https://bugs.webkit.org/show_bug.cgi?id=79841
Patch by Dana Jansens <danakj@chromium.org> on 2012-02-29
Reviewed by James Robinson.
Source/WebCore:
We currently don't push dirty tiles to the impl thread so there are no
tiles with garbage data on the impl thread. However, this judgement is
overzealous and blocks tiles that get invalidated by WebKit for the
next frame during the paint of the current frame.
Instead, check if a tile is dirty and was not painted for the current
frame when deciding to push the tile to the impl thread. This requires
that we know if a tile was painted during the current frame, which we
can do if we always reset m_updateRect to be empty each frame.
New unit tests: TiledLayerChromiumTest.pushTilesMarkedDirtyDuringPaint
TiledLayerChromiumTest.pushTilesLayerMarkedDirtyDuringPaintOnNextLayer
TiledLayerChromiumTest.pushTilesLayerMarkedDirtyDuringPaintOnPreviousLayer
* platform/graphics/chromium/TiledLayerChromium.cpp:
(WebCore::UpdatableTile::isDirtyForCurrentFrame):
(WebCore::TiledLayerChromium::pushPropertiesTo):
(WebCore::TiledLayerChromium::prepareToUpdateTiles):
(WebCore::TiledLayerChromium::resetUpdateState):
(WebCore):
(WebCore::TiledLayerChromium::prepareToUpdate):
* platform/graphics/chromium/TiledLayerChromium.h:
(TiledLayerChromium):
Source/WebKit/chromium:
* tests/TiledLayerChromiumTest.cpp:
(WTF::TEST):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicschromiumTiledLayerChromiumcpp">trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicschromiumTiledLayerChromiumh">trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.h</a></li>
<li><a href="#trunkSourceWebKitchromiumChangeLog">trunk/Source/WebKit/chromium/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitchromiumtestsTiledLayerChromiumTestcpp">trunk/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (109262 => \
109263)</h4> <pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2012-02-29 22:30:33 UTC (rev \
109262)
+++ trunk/Source/WebCore/ChangeLog 2012-02-29 22:35:01 UTC (rev 109263)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+2012-02-29 Dana Jansens <danakj@chromium.org>
+
+ [chromium] Don't let invalidation for next frame prevent tile upload
+ https://bugs.webkit.org/show_bug.cgi?id=79841
+
+ Reviewed by James Robinson.
+
+ We currently don't push dirty tiles to the impl thread so there are no
+ tiles with garbage data on the impl thread. However, this judgement is
+ overzealous and blocks tiles that get invalidated by WebKit for the
+ next frame during the paint of the current frame.
+
+ Instead, check if a tile is dirty and was not painted for the current
+ frame when deciding to push the tile to the impl thread. This requires
+ that we know if a tile was painted during the current frame, which we
+ can do if we always reset m_updateRect to be empty each frame.
+
+ New unit tests: TiledLayerChromiumTest.pushTilesMarkedDirtyDuringPaint
+ \
TiledLayerChromiumTest.pushTilesLayerMarkedDirtyDuringPaintOnNextLayer + \
TiledLayerChromiumTest.pushTilesLayerMarkedDirtyDuringPaintOnPreviousLayer +
+ * platform/graphics/chromium/TiledLayerChromium.cpp:
+ (WebCore::UpdatableTile::isDirtyForCurrentFrame):
+ (WebCore::TiledLayerChromium::pushPropertiesTo):
+ (WebCore::TiledLayerChromium::prepareToUpdateTiles):
+ (WebCore::TiledLayerChromium::resetUpdateState):
+ (WebCore):
+ (WebCore::TiledLayerChromium::prepareToUpdate):
+ * platform/graphics/chromium/TiledLayerChromium.h:
+ (TiledLayerChromium):
+
</ins><span class="cx"> 2012-02-29 Tommy Widenflycht <tommyw@google.com>
</span><span class="cx">
</span><span class="cx"> MediaStream API: MediaStreamTrackList out-of-bounds \
access fix </span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicschromiumTiledLayerChromiumcpp"></a>
<div class="modfile"><h4>Modified: \
trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp (109262 => \
109263)</h4> <pre class="diff"><span>
<span class="info">--- \
trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp 2012-02-29 \
22:30:33 UTC (rev 109262)
+++ trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp 2012-02-29 \
22:35:01 UTC (rev 109263) </span><span class="lines">@@ -67,6 +67,7 @@
</span><span class="cx"> m_updateRect = m_dirtyRect;
</span><span class="cx"> m_dirtyRect = IntRect();
</span><span class="cx"> }
</span><ins>+ bool isDirtyForCurrentFrame() { return !m_dirtyRect.isEmpty() \
&& m_updateRect.isEmpty(); } </ins><span class="cx">
</span><span class="cx"> IntRect m_dirtyRect;
</span><span class="cx"> IntRect m_updateRect;
</span><span class="lines">@@ -263,7 +264,7 @@
</span><span class="cx"> invalidTiles.append(tile);
</span><span class="cx"> continue;
</span><span class="cx"> }
</span><del>- if (tile->isDirty())
</del><ins>+ if (tile->isDirtyForCurrentFrame())
</ins><span class="cx"> continue;
</span><span class="cx">
</span><span class="cx"> tiledLayer->pushTileProperties(i, j, \
tile->managedTexture()->textureId(), tile->m_opaqueRect); </span><span \
class="lines">@@ -398,13 +399,6 @@ </span><span class="cx">
</span><span class="cx"> void TiledLayerChromium::prepareToUpdateTiles(bool idle, int \
left, int top, int right, int bottom) </span><span class="cx"> {
</span><del>- // Reset m_updateRect for all tiles.
- for (CCLayerTilingData::TileMap::const_iterator iter = \
m_tiler->tiles().begin(); iter != m_tiler->tiles().end(); \
++iter) {
- UpdatableTile* tile = \
static_cast<UpdatableTile*>(iter->second.get());
- tile->m_updateRect = IntRect();
- tile->m_partialUpdate = false;
- }
-
</del><span class="cx"> createTextureUpdaterIfNeeded();
</span><span class="cx">
</span><span class="cx"> // Create tiles as needed, expanding a dirty rect to \
contain all </span><span class="lines">@@ -566,6 +560,17 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void TiledLayerChromium::resetUpdateState()
+{
+ // Reset m_updateRect for all tiles.
+ CCLayerTilingData::TileMap::const_iterator end = m_tiler->tiles().end();
+ for (CCLayerTilingData::TileMap::const_iterator iter = \
m_tiler->tiles().begin(); iter != end; ++iter) { + UpdatableTile* tile = \
static_cast<UpdatableTile*>(iter->second.get()); + \
tile->m_updateRect = IntRect(); + tile->m_partialUpdate = false;
+ }
+}
+
</ins><span class="cx"> void TiledLayerChromium::prepareToUpdate(const IntRect& \
layerRect) </span><span class="cx"> {
</span><span class="cx"> m_skipsDraw = false;
</span><span class="lines">@@ -575,6 +580,8 @@
</span><span class="cx">
</span><span class="cx"> updateBounds();
</span><span class="cx">
</span><ins>+ resetUpdateState();
+
</ins><span class="cx"> if (layerRect.isEmpty() || !m_tiler->numTiles())
</span><span class="cx"> return;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicschromiumTiledLayerChromiumh"></a>
<div class="modfile"><h4>Modified: \
trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.h (109262 => \
109263)</h4> <pre class="diff"><span>
<span class="info">--- \
trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.h 2012-02-29 \
22:30:33 UTC (rev 109262)
+++ trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.h 2012-02-29 \
22:35:01 UTC (rev 109263) </span><span class="lines">@@ -86,6 +86,9 @@
</span><span class="cx"> // Set invalidations to be potentially repainted during \
update(). </span><span class="cx"> void invalidateRect(const IntRect& \
layerRect); </span><span class="cx">
</span><ins>+ // Reset state on tiles that will be used for updating the layer.
+ void resetUpdateState();
+
</ins><span class="cx"> // Prepare data needed to update textures that intersect \
with layerRect. </span><span class="cx"> void prepareToUpdate(const IntRect& \
layerRect); </span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKitchromiumChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/chromium/ChangeLog (109262 => \
109263)</h4> <pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/chromium/ChangeLog 2012-02-29 22:30:33 UTC \
(rev 109262)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-02-29 22:35:01 UTC (rev 109263)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2012-02-29 Dana Jansens <danakj@chromium.org>
+
+ [chromium] Don't let invalidation for next frame prevent tile upload
+ https://bugs.webkit.org/show_bug.cgi?id=79841
+
+ Reviewed by James Robinson.
+
+ * tests/TiledLayerChromiumTest.cpp:
+ (WTF::TEST):
+
</ins><span class="cx"> 2012-02-28 Sheriff Bot <webkit.review.bot@gmail.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r107917 and r109188.
</span></span></pre></div>
<a id="trunkSourceWebKitchromiumtestsTiledLayerChromiumTestcpp"></a>
<div class="modfile"><h4>Modified: \
trunk/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp (109262 => 109263)</h4> \
<pre class="diff"><span> <span class="info">--- \
trunk/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp 2012-02-29 22:30:33 UTC \
(rev 109262)
+++ trunk/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp 2012-02-29 22:35:01 \
UTC (rev 109263) </span><span class="lines">@@ -318,7 +318,103 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+TEST(TiledLayerChromiumTest, pushTilesMarkedDirtyDuringPaint)
+{
+ OwnPtr<TextureManager> textureManager = \
TextureManager::create(4*1024*1024, 2*1024*1024, 1024); + \
RefPtr<FakeTiledLayerChromium> layer = adoptRef(new \
FakeTiledLayerChromium(textureManager.get())); + DebugScopedSetImplThread \
implThread; + OwnPtr<FakeCCTiledLayerImpl> layerImpl(adoptPtr(new \
FakeCCTiledLayerImpl(0))); </ins><span class="cx">
</span><ins>+ FakeTextureAllocator textureAllocator;
+ CCTextureUpdater updater(&textureAllocator);
+
+ // The tile size is 100x100, so this invalidates and then paints two tiles.
+ // However, during the paint, we invalidate one of the tiles. This should
+ // not prevent the tile from being pushed.
+ layer->setBounds(IntSize(100, 200));
+ layer->invalidateRect(IntRect(0, 0, 100, 200));
+ layer->fakeLayerTextureUpdater()->setRectToInvalidate(IntRect(0, 50, 100, \
50), layer.get()); + layer->prepareToUpdate(IntRect(0, 0, 100, 200));
+ layer->updateCompositorResources(0, updater);
+ layer->pushPropertiesTo(layerImpl.get());
+
+ // We should have both tiles on the impl side.
+ EXPECT_TRUE(layerImpl->hasTileAt(0, 0));
+ EXPECT_TRUE(layerImpl->hasTileAt(0, 1));
+}
+
+TEST(TiledLayerChromiumTest, pushTilesLayerMarkedDirtyDuringPaintOnNextLayer)
+{
+ OwnPtr<TextureManager> textureManager = \
TextureManager::create(4*1024*1024, 2*1024*1024, 1024); + \
RefPtr<FakeTiledLayerChromium> layer1 = adoptRef(new \
FakeTiledLayerChromium(textureManager.get())); + \
RefPtr<FakeTiledLayerChromium> layer2 = adoptRef(new \
FakeTiledLayerChromium(textureManager.get())); + DebugScopedSetImplThread \
implThread; + OwnPtr<FakeCCTiledLayerImpl> layer1Impl(adoptPtr(new \
FakeCCTiledLayerImpl(0))); + OwnPtr<FakeCCTiledLayerImpl> \
layer2Impl(adoptPtr(new FakeCCTiledLayerImpl(0))); +
+ FakeTextureAllocator textureAllocator;
+ CCTextureUpdater updater(&textureAllocator);
+
+ layer1->setBounds(IntSize(100, 200));
+ layer1->invalidateRect(IntRect(0, 0, 100, 200));
+ layer2->setBounds(IntSize(100, 200));
+ layer2->invalidateRect(IntRect(0, 0, 100, 200));
+
+ layer1->prepareToUpdate(IntRect(0, 0, 100, 200));
+
+ // Invalidate a tile on layer1
+ layer2->fakeLayerTextureUpdater()->setRectToInvalidate(IntRect(0, 50, 100, \
50), layer1.get()); + layer2->prepareToUpdate(IntRect(0, 0, 100, 200));
+
+ layer1->updateCompositorResources(0, updater);
+ layer2->updateCompositorResources(0, updater);
+
+ layer1->pushPropertiesTo(layer1Impl.get());
+ layer2->pushPropertiesTo(layer2Impl.get());
+
+ // We should have both tiles on the impl side for all layers.
+ EXPECT_TRUE(layer1Impl->hasTileAt(0, 0));
+ EXPECT_TRUE(layer1Impl->hasTileAt(0, 1));
+ EXPECT_TRUE(layer2Impl->hasTileAt(0, 0));
+ EXPECT_TRUE(layer2Impl->hasTileAt(0, 1));
+}
+
+TEST(TiledLayerChromiumTest, pushTilesLayerMarkedDirtyDuringPaintOnPreviousLayer)
+{
+ OwnPtr<TextureManager> textureManager = \
TextureManager::create(4*1024*1024, 2*1024*1024, 1024); + \
RefPtr<FakeTiledLayerChromium> layer1 = adoptRef(new \
FakeTiledLayerChromium(textureManager.get())); + \
RefPtr<FakeTiledLayerChromium> layer2 = adoptRef(new \
FakeTiledLayerChromium(textureManager.get())); + DebugScopedSetImplThread \
implThread; + OwnPtr<FakeCCTiledLayerImpl> layer1Impl(adoptPtr(new \
FakeCCTiledLayerImpl(0))); + OwnPtr<FakeCCTiledLayerImpl> \
layer2Impl(adoptPtr(new FakeCCTiledLayerImpl(0))); +
+ FakeTextureAllocator textureAllocator;
+ CCTextureUpdater updater(&textureAllocator);
+
+ layer1->setBounds(IntSize(100, 200));
+ layer1->invalidateRect(IntRect(0, 0, 100, 200));
+ layer2->setBounds(IntSize(100, 200));
+ layer2->invalidateRect(IntRect(0, 0, 100, 200));
+
+ // Invalidate a tile on layer2
+ layer1->fakeLayerTextureUpdater()->setRectToInvalidate(IntRect(0, 50, 100, \
50), layer2.get()); + layer1->prepareToUpdate(IntRect(0, 0, 100, 200));
+
+ layer2->prepareToUpdate(IntRect(0, 0, 100, 200));
+
+ layer1->updateCompositorResources(0, updater);
+ layer2->updateCompositorResources(0, updater);
+
+ layer1->pushPropertiesTo(layer1Impl.get());
+ layer2->pushPropertiesTo(layer2Impl.get());
+
+ // We should have both tiles on the impl side for all layers.
+ EXPECT_TRUE(layer1Impl->hasTileAt(0, 0));
+ EXPECT_TRUE(layer1Impl->hasTileAt(0, 1));
+ EXPECT_TRUE(layer2Impl->hasTileAt(0, 0));
+ EXPECT_TRUE(layer2Impl->hasTileAt(0, 1));
+}
+
</ins><span class="cx"> TEST(TiledLayerChromiumTest, idlePaintOutOfMemory)
</span><span class="cx"> {
</span><span class="cx"> // The tile size is 100x100. Setup 5x5 tiles with one \
1x1 visible tile in the center. </span><span class="lines">@@ -486,6 +582,7 @@
</span><span class="cx"> // Invalidate the entire layer again, but do not paint. \
All tiles should be gone now from the </span><span class="cx"> // impl side.
</span><span class="cx"> layer->setNeedsDisplay();
</span><ins>+ layer->prepareToUpdate(IntRect(0, 0, 0, 0));
</ins><span class="cx"> layer->updateCompositorResources(0, updater);
</span><span class="cx"> layer->pushPropertiesTo(layerImpl.get());
</span><span class="cx"> EXPECT_FALSE(layerImpl->hasTileAt(0, 0));
</span></span></pre>
</div>
</div>
</body>
</html>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic