Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3795)

Unified Diff: Source/platform/graphics/paint/DisplayItemList.cpp

Issue 847783003: New display item caching (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: New method, supporting partial paint Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/platform/graphics/paint/DisplayItemList.cpp
diff --git a/Source/platform/graphics/paint/DisplayItemList.cpp b/Source/platform/graphics/paint/DisplayItemList.cpp
index c2b800adb3cf2a7c2a151624e4769053f4c8d3ff..33b1e0c8391d82e52019de92510a1cbad12c6e2b 100644
--- a/Source/platform/graphics/paint/DisplayItemList.cpp
+++ b/Source/platform/graphics/paint/DisplayItemList.cpp
@@ -31,7 +31,9 @@ void DisplayItemList::add(WTF::PassOwnPtr<DisplayItem> displayItem)
void DisplayItemList::invalidate(DisplayItemClient client)
{
ASSERT(RuntimeEnabledFeatures::slimmingPaintEnabled());
- m_cachedClients.remove(client);
+ // Can only be called during layout/paintInvalidation, not during painting.
+ ASSERT(m_newPaints.isEmpty());
+ m_cachedDisplayItemsInfoByClient.remove(client);
}
void DisplayItemList::invalidateAll()
@@ -40,35 +42,57 @@ void DisplayItemList::invalidateAll()
// Can only be called during layout/paintInvalidation, not during painting.
ASSERT(m_newPaints.isEmpty());
m_paintList.clear();
- m_cachedClients.clear();
+ m_cachedDisplayItemsInfoByClient.clear();
}
-bool DisplayItemList::clientCacheIsValid(DisplayItemClient client) const
+size_t DisplayItemList::findNextMatchingCachedItem(const DisplayItem& displayItem)
{
- return RuntimeEnabledFeatures::slimmingPaintDisplayItemCacheEnabled() && m_cachedClients.contains(client);
+ ASSERT(displayItem.isCached() || displayItem.isSubtreeCached());
+ ASSERT(clientCacheIsValid(displayItem.client()));
+
+ DisplayItemsInfo& info = m_cachedDisplayItemsInfoByClient.find(displayItem.client())->value;
+ size_t offset = info.updateOffset;
+ while (offset < info.displayItemIndexes.size()) {
+ OwnPtr<DisplayItem>& existingItem = m_paintList[info.displayItemIndexes[offset]];
+ if (existingItem && existingItem->idsEqual(displayItem))
+ break;
+ ++offset;
+ }
+ // FIXME: We should assert index < info.displayItemIndexes.size(), but currently
+ // our paint invalidation doesn't always invalidate all objects needing repaint.
+ // We should fix them before enabling the assert. crbug.com/450725.
+ if (offset >= info.displayItemIndexes.size()) {
+ // The object needed repaint but wasn't invalidated. Should remove this condition
+ // after we fix all invalidation issues.
+ return kNotFound;
+ }
+ info.updateOffset = offset + 1;
+ return info.displayItemIndexes[offset];
}
-PaintList::iterator DisplayItemList::findNextMatchingCachedItem(PaintList::iterator begin, const DisplayItem& displayItem)
+void DisplayItemList::appendDisplayItem(PaintList& list, DisplayItemList::DisplayItemsInfoByClientMap& displayItemsInfoByClient, WTF::PassOwnPtr<DisplayItem> displayItem)
{
- PaintList::iterator end = m_paintList.end();
-
- if (!clientCacheIsValid(displayItem.client()))
- return end;
-
- for (PaintList::iterator it = begin; it != end; ++it) {
- DisplayItem& existing = **it;
- if (existing.idsEqual(displayItem))
- return it;
+ DisplayItemsInfoByClientMap::iterator it = displayItemsInfoByClient.find(displayItem->client());
+ if (it != displayItemsInfoByClient.end()) {
+ it->value.displayItemIndexes.append(list.size());
+ } else {
+ DisplayItemsInfo info;
+ info.displayItemIndexes.append(list.size());
+ displayItemsInfoByClient.add(displayItem->client(), info);
}
-
- ASSERT_NOT_REACHED();
- return end;
+ list.append(displayItem);
}
-static void appendDisplayItem(PaintList& list, HashSet<DisplayItemClient>& clients, WTF::PassOwnPtr<DisplayItem> displayItem)
+void DisplayItemList::copyCachedSubtree(const DisplayItem& subtreeCachedDisplayItem, PaintList& list, DisplayItemList::DisplayItemsInfoByClientMap& displayItemsInfoByClient)
{
- clients.add(displayItem->client());
- list.append(displayItem);
+ size_t index = findNextMatchingCachedItem(subtreeCachedDisplayItem);
+ ASSERT(index != kNotFound);
+ ASSERT(m_paintList[index]->isBeginSubtree());
+ do {
+ if (clientCacheIsValid(m_paintList[index].client())
+ appendDisplayItem(list, displayItemsInfoByClient, m_paintList[index].release());
+ ++index;
+ } while (!list.last()->isEndSubtree());
}
// Update the existing paintList by removing invalidated entries, updating
@@ -78,52 +102,40 @@ static void appendDisplayItem(PaintList& list, HashSet<DisplayItemClient>& clien
// the ordering implied by the existing paint list, extra treewalks are avoided.
void DisplayItemList::updatePaintList()
{
- if (!RuntimeEnabledFeatures::slimmingPaintDisplayItemCacheEnabled()) {
- m_paintList.clear();
- m_paintList.swap(m_newPaints);
- m_cachedClients.clear();
- return;
- }
-
PaintList updatedList;
- HashSet<DisplayItemClient> newCachedClients;
-
- PaintList::iterator paintListIt = m_paintList.begin();
- PaintList::iterator paintListEnd = m_paintList.end();
+ DisplayItemsInfoByClientMap newCachedDisplayItemsInfoByClient;
for (OwnPtr<DisplayItem>& newDisplayItem : m_newPaints) {
- PaintList::iterator cachedItemIt = findNextMatchingCachedItem(paintListIt, *newDisplayItem);
- if (cachedItemIt != paintListEnd) {
- // Copy all of the existing items over until we hit the matching cached item.
- for (; paintListIt != cachedItemIt; ++paintListIt) {
- if (clientCacheIsValid((*paintListIt)->client()))
- appendDisplayItem(updatedList, newCachedClients, paintListIt->release());
- }
-
- // Use the cached item for the new display item.
- appendDisplayItem(updatedList, newCachedClients, cachedItemIt->release());
- ++paintListIt;
- } else {
- // If the new display item is a cached placeholder, we should have found
- // the cached display item.
- ASSERT(!newDisplayItem->isCached());
-
- // Copy over the new item.
- appendDisplayItem(updatedList, newCachedClients, newDisplayItem.release());
+ if (newDisplayItem->isSubtreeCached()) {
+ copyCachedSubtree(*newDisplayItem, updatedList, newCachedDisplayItemsInfoByClient);
+ continue;
}
- }
- // Copy over any remaining items that are validly cached.
- for (; paintListIt != paintListEnd; ++paintListIt) {
- if (clientCacheIsValid((*paintListIt)->client()))
- appendDisplayItem(updatedList, newCachedClients, paintListIt->release());
+ if (newDisplayItem->isCached()) {
+ size_t index = findNextMatchingCachedItem(*newDisplayItem);
+ if (index != kNotFound) {
+ // Use the cached item for the new display item.
+ newDisplayItem = m_paintList[index].release();
+ } else {
+ // FIXME: The object needed repaint but wasn't invalidated. Should remove this condition
+ // after we fix all invalidation issues. crbug.com/450725.
+ WTF_LOG_ERROR("Object needed repaint but wasn't invalidated.");
+#ifdef NDEBUG
+ continue; // Ignore the item in release build.
+#else
+ WTF_LOG_ERROR("displayItem=%s", newDisplayItem->asDebugString().utf8().data());
+ // The CachedDisplayItem will be appended to visually show error indicator.
+#endif
+ }
+ }
+ appendDisplayItem(updatedList, newCachedDisplayItemsInfoByClient, newDisplayItem.release());
}
m_newPaints.clear();
m_paintList.clear();
m_paintList.swap(updatedList);
- m_cachedClients.clear();
- m_cachedClients.swap(newCachedClients);
+ m_cachedDisplayItemsInfoByClient.clear();
+ m_cachedDisplayItemsInfoByClient.swap(newCachedDisplayItemsInfoByClient);
}
#ifndef NDEBUG
@@ -131,12 +143,15 @@ void DisplayItemList::updatePaintList()
WTF::String DisplayItemList::paintListAsDebugString(const PaintList& list) const
{
StringBuilder stringBuilder;
- bool isFirst = true;
- for (auto& displayItem : list) {
- if (!isFirst)
+ for (size_t i = 0; i < list.size(); ++i) {
+ const OwnPtr<DisplayItem>& displayItem = list[i];
+ if (i)
stringBuilder.append(",\n");
- isFirst = false;
- stringBuilder.append('{');
+ if (!displayItem) {
+ stringBuilder.append("null");
+ continue;
+ }
+ stringBuilder.append(String::format("{index: %d, ", (int)i));
displayItem->dumpPropertiesAsDebugString(stringBuilder);
stringBuilder.append(", cacheIsValid: ");
stringBuilder.append(clientCacheIsValid(displayItem->client()) ? "true" : "false");
@@ -145,13 +160,33 @@ WTF::String DisplayItemList::paintListAsDebugString(const PaintList& list) const
return stringBuilder.toString();
}
+WTF::String DisplayItemList::cachedDisplayItemsInfoByClientAsDebugString() const
+{
+ StringBuilder stringBuilder;
+ bool isFirst = true;
+ for (auto& item : m_cachedDisplayItemsInfoByClient) {
+ if (!isFirst)
+ stringBuilder.append(",\n");
+ isFirst = false;
+ stringBuilder.append(String::format("{client:%p, updateOffset:%d, displayItemIndexes:[", item.key, (int)item.value.updateOffset));
+ for (size_t i = 0; i < item.value.displayItemIndexes.size(); ++i) {
+ if (i)
+ stringBuilder.append(',');
+ stringBuilder.append(String::format("%d", (int)item.value.displayItemIndexes[i]));
+ }
+ stringBuilder.append("]}");
+ }
+ return stringBuilder.toString();
+}
+
void DisplayItemList::showDebugData() const
{
fprintf(stderr, "paint list: [%s]\n", paintListAsDebugString(m_paintList).utf8().data());
fprintf(stderr, "new paints: [%s]\n", paintListAsDebugString(m_newPaints).utf8().data());
+ fprintf(stderr, "cachedDisplayItemsInfo: [%s]\n", cachedDisplayItemsInfoByClientAsDebugString().utf8().data());
}
-#endif
+#endif // ifndef NDEBUG
void DisplayItemList::replay(GraphicsContext* context)
{

Powered by Google App Engine
This is Rietveld 408576698