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

Side by Side Diff: third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp

Issue 2184363002: Handle removed display items in under-invalidation checking in cached subsequences (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Make test expectations suitable for both debug and release+dcheck Created 4 years, 4 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 unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "platform/graphics/paint/PaintController.h" 5 #include "platform/graphics/paint/PaintController.h"
6 6
7 #include "platform/TraceEvent.h" 7 #include "platform/TraceEvent.h"
8 #include "platform/graphics/GraphicsLayer.h" 8 #include "platform/graphics/GraphicsLayer.h"
9 #include "platform/graphics/paint/DrawingDisplayItem.h" 9 #include "platform/graphics/paint/DrawingDisplayItem.h"
10 #include "third_party/skia/include/core/SkPictureAnalyzer.h" 10 #include "third_party/skia/include/core/SkPictureAnalyzer.h"
(...skipping 121 matching lines...) Expand 10 before | Expand all | Expand 10 after
132 return; 132 return;
133 133
134 #if DCHECK_IS_ON() 134 #if DCHECK_IS_ON()
135 // Also remove the index pointing to the removed display item. 135 // Also remove the index pointing to the removed display item.
136 DisplayItemIndicesByClientMap::iterator it = m_newDisplayItemIndicesByClient .find(&m_newDisplayItemList.last().client()); 136 DisplayItemIndicesByClientMap::iterator it = m_newDisplayItemIndicesByClient .find(&m_newDisplayItemList.last().client());
137 if (it != m_newDisplayItemIndicesByClient.end()) { 137 if (it != m_newDisplayItemIndicesByClient.end()) {
138 Vector<size_t>& indices = it->value; 138 Vector<size_t>& indices = it->value;
139 if (!indices.isEmpty() && indices.last() == (m_newDisplayItemList.size() - 1)) 139 if (!indices.isEmpty() && indices.last() == (m_newDisplayItemList.size() - 1))
140 indices.removeLast(); 140 indices.removeLast();
141 } 141 }
142
143 if (RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled() && isCheckingUnderInvalidation()) {
144 if (m_skippedProbableUnderInvalidationCount) {
145 --m_skippedProbableUnderInvalidationCount;
146 } else {
147 DCHECK(m_underInvalidationCheckingBegin);
148 --m_underInvalidationCheckingBegin;
149 }
150 }
142 #endif 151 #endif
143 m_newDisplayItemList.removeLast(); 152 m_newDisplayItemList.removeLast();
144 153
145 if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) 154 if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
146 m_newPaintChunks.decrementDisplayItemIndex(); 155 m_newPaintChunks.decrementDisplayItemIndex();
147 } 156 }
148 157
149 void PaintController::processNewItem(DisplayItem& displayItem) 158 void PaintController::processNewItem(DisplayItem& displayItem)
150 { 159 {
151 DCHECK(!m_constructionDisabled); 160 DCHECK(!m_constructionDisabled);
(...skipping 155 matching lines...) Expand 10 before | Expand all | Expand 10 after
307 if (item.isCacheable()) { 316 if (item.isCacheable()) {
308 #if DCHECK_IS_ON() 317 #if DCHECK_IS_ON()
309 ++m_numIndexedItems; 318 ++m_numIndexedItems;
310 #endif 319 #endif
311 addItemToIndexIfNeeded(item, i, m_outOfOrderItemIndices); 320 addItemToIndexIfNeeded(item, i, m_outOfOrderItemIndices);
312 } 321 }
313 } 322 }
314 323
315 #ifndef NDEBUG 324 #ifndef NDEBUG
316 showDebugData(); 325 showDebugData();
317 LOG(ERROR) << id.client.debugName() << ":" << DisplayItem::typeAsDebugString (id.type) << " not found in current display item list"; 326 LOG(ERROR) << id.client.debugName() << ":" << DisplayItem::typeAsDebugString (id.type);
318 #endif 327 #endif
319 NOTREACHED(); 328 NOTREACHED() << "Can't find cached display item";
320 // We did not find the cached display item. This should be impossible, but m ay occur if there is a bug 329 // We did not find the cached display item. This should be impossible, but m ay occur if there is a bug
321 // in the system, such as under-invalidation, incorrect cache checking or du plicate display ids. 330 // in the system, such as under-invalidation, incorrect cache checking or du plicate display ids.
322 // In this case, the caller should fall back to repaint the display item. 331 // In this case, the caller should fall back to repaint the display item.
323 return kNotFound; 332 return kNotFound;
324 } 333 }
325 334
326 // Copies a cached subsequence from current list to the new list. On return, 335 // Copies a cached subsequence from current list to the new list. On return,
327 // |cachedItemIndex| points to the item after the EndSubsequence item of the sub sequence. 336 // |cachedItemIndex| points to the item after the EndSubsequence item of the sub sequence.
328 // When paintUnderInvaldiationCheckingEnabled() we'll not actually copy the subs equence, 337 // When paintUnderInvaldiationCheckingEnabled() we'll not actually copy the subs equence,
329 // but mark the begin and end of the subsequence for under-invalidation checking . 338 // but mark the begin and end of the subsequence for under-invalidation checking .
330 void PaintController::copyCachedSubsequence(size_t& cachedItemIndex) 339 void PaintController::copyCachedSubsequence(size_t& cachedItemIndex)
331 { 340 {
332 DisplayItem* cachedItem = &m_currentPaintArtifact.getDisplayItemList()[cache dItemIndex]; 341 DisplayItem* cachedItem = &m_currentPaintArtifact.getDisplayItemList()[cache dItemIndex];
333 #if DCHECK_IS_ON() 342 #if DCHECK_IS_ON()
334 DCHECK(cachedItem->getType() == DisplayItem::Subsequence); 343 DCHECK(cachedItem->getType() == DisplayItem::Subsequence);
335 if (RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled()) { 344 if (RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled()) {
336 DCHECK(!isCheckingUnderInvalidation()); 345 DCHECK(!isCheckingUnderInvalidation());
337 m_underInvalidationCheckingBegin = cachedItemIndex; 346 m_underInvalidationCheckingBegin = cachedItemIndex;
338 #ifndef NDEBUG 347 m_underInvalidationMessagePrefix = "(In cached subsequence of " + cached Item->client().debugName() + ")";
339 m_underInvalidationMessagePrefix = "(In CachedSubsequence of " + cachedI tem->clientDebugString() + ")";
340 #else
341 m_underInvalidationMessagePrefix = "(In CachedSubsequence)";
342 #endif
343 } 348 }
344 #endif 349 #endif
345 350
346 DisplayItem::Id endSubsequenceId(cachedItem->client(), DisplayItem::EndSubse quence); 351 DisplayItem::Id endSubsequenceId(cachedItem->client(), DisplayItem::EndSubse quence);
347 Vector<PaintChunk>::const_iterator cachedChunk; 352 Vector<PaintChunk>::const_iterator cachedChunk;
348 if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) { 353 if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
349 cachedChunk = m_currentPaintArtifact.findChunkByDisplayItemIndex(cachedI temIndex); 354 cachedChunk = m_currentPaintArtifact.findChunkByDisplayItemIndex(cachedI temIndex);
350 updateCurrentPaintChunkProperties(cachedChunk->id ? &*cachedChunk->id : nullptr, cachedChunk->properties); 355 updateCurrentPaintChunkProperties(cachedChunk->id ? &*cachedChunk->id : nullptr, cachedChunk->properties);
351 } else { 356 } else {
352 // This is to avoid compilation error about uninitialized variable on Wi ndows. 357 // This is to avoid compilation error about uninitialized variable on Wi ndows.
(...skipping 41 matching lines...) Expand 10 before | Expand all | Expand 10 after
394 return enclosingIntRect(visualRect); 399 return enclosingIntRect(visualRect);
395 } 400 }
396 401
397 void PaintController::resetCurrentListIndices() 402 void PaintController::resetCurrentListIndices()
398 { 403 {
399 m_nextItemToMatch = 0; 404 m_nextItemToMatch = 0;
400 m_nextItemToIndex = 0; 405 m_nextItemToIndex = 0;
401 #if DCHECK_IS_ON() 406 #if DCHECK_IS_ON()
402 m_underInvalidationCheckingBegin = 0; 407 m_underInvalidationCheckingBegin = 0;
403 m_underInvalidationCheckingEnd = 0; 408 m_underInvalidationCheckingEnd = 0;
409 m_skippedProbableUnderInvalidationCount = 0;
404 #endif 410 #endif
405 } 411 }
406 412
407 void PaintController::commitNewDisplayItems(const LayoutSize& offsetFromLayoutOb ject) 413 void PaintController::commitNewDisplayItems(const LayoutSize& offsetFromLayoutOb ject)
408 { 414 {
409 TRACE_EVENT2("blink,benchmark", "PaintController::commitNewDisplayItems", 415 TRACE_EVENT2("blink,benchmark", "PaintController::commitNewDisplayItems",
410 "current_display_list_size", (int)m_currentPaintArtifact.getDisplayItemL ist().size(), 416 "current_display_list_size", (int)m_currentPaintArtifact.getDisplayItemL ist().size(),
411 "num_non_cached_new_items", (int)m_newDisplayItemList.size() - m_numCach edNewItems); 417 "num_non_cached_new_items", (int)m_newDisplayItemList.size() - m_numCach edNewItems);
412 m_numCachedNewItems = 0; 418 m_numCachedNewItems = 0;
413 419
(...skipping 83 matching lines...) Expand 10 before | Expand all | Expand 10 after
497 void PaintController::appendDebugDrawingAfterCommit(const DisplayItemClient& dis playItemClient, PassRefPtr<SkPicture> picture, const LayoutSize& offsetFromLayou tObject) 503 void PaintController::appendDebugDrawingAfterCommit(const DisplayItemClient& dis playItemClient, PassRefPtr<SkPicture> picture, const LayoutSize& offsetFromLayou tObject)
498 { 504 {
499 DCHECK(m_newDisplayItemList.isEmpty()); 505 DCHECK(m_newDisplayItemList.isEmpty());
500 DrawingDisplayItem& displayItem = m_currentPaintArtifact.getDisplayItemList( ).allocateAndConstruct<DrawingDisplayItem>(displayItemClient, DisplayItem::Debug Drawing, picture); 506 DrawingDisplayItem& displayItem = m_currentPaintArtifact.getDisplayItemList( ).allocateAndConstruct<DrawingDisplayItem>(displayItemClient, DisplayItem::Debug Drawing, picture);
501 displayItem.setSkippedCache(); 507 displayItem.setSkippedCache();
502 m_currentPaintArtifact.getDisplayItemList().appendVisualRect(visualRectForDi splayItem(displayItem, offsetFromLayoutObject)); 508 m_currentPaintArtifact.getDisplayItemList().appendVisualRect(visualRectForDi splayItem(displayItem, offsetFromLayoutObject));
503 } 509 }
504 510
505 #if DCHECK_IS_ON() 511 #if DCHECK_IS_ON()
506 512
507 void PaintController::showUnderInvalidationError(const char* reason, const Displ ayItem& newItem, const DisplayItem& oldItem) const 513 void PaintController::showUnderInvalidationError(const char* reason, const Displ ayItem& newItem, const DisplayItem* oldItem) const
508 { 514 {
509 LOG(ERROR) << m_underInvalidationMessagePrefix << " " << reason; 515 LOG(ERROR) << m_underInvalidationMessagePrefix << " " << reason;
510 #ifndef NDEBUG 516 #ifndef NDEBUG
511 LOG(ERROR) << "New display item: " << newItem.asDebugString(); 517 LOG(ERROR) << "New display item: " << newItem.asDebugString();
512 LOG(ERROR) << "Old display item: " << oldItem.asDebugString(); 518 LOG(ERROR) << "Old display item: " << (oldItem ? oldItem->asDebugString() : "None");
513 #else 519 #else
514 LOG(ERROR) << "Run debug build to get more details."; 520 LOG(ERROR) << "Run debug build to get more details.";
515 #endif 521 #endif
516 LOG(ERROR) << "See http://crbug.com/619103."; 522 LOG(ERROR) << "See http://crbug.com/619103.";
517 523
518 #ifndef NDEBUG 524 #ifndef NDEBUG
519 if (newItem.isDrawing()) { 525 const SkPicture* newPicture = newItem.isDrawing() ? static_cast<const Drawin gDisplayItem&>(newItem).picture() : nullptr;
520 RefPtr<const SkPicture> newPicture = static_cast<const DrawingDisplayIte m&>(newItem).picture(); 526 const SkPicture* oldPicture = oldItem && oldItem->isDrawing() ? static_cast< const DrawingDisplayItem*>(oldItem)->picture() : nullptr;
521 RefPtr<const SkPicture> oldPicture = static_cast<const DrawingDisplayIte m&>(oldItem).picture(); 527 LOG(INFO) << "new picture:\n" << (newPicture ? pictureAsDebugString(newPictu re) : "None");
522 LOG(INFO) << "new picture:\n" << (newPicture ? pictureAsDebugString(newP icture.get()) : "None"); 528 LOG(INFO) << "old picture:\n" << (oldPicture ? pictureAsDebugString(oldPictu re) : "None");
523 LOG(INFO) << "old picture:\n" << (oldPicture ? pictureAsDebugString(oldP icture.get()) : "None"); 529
524 } 530 showDebugData();
525 #endif // NDEBUG 531 #endif // NDEBUG
526 } 532 }
527 533
528 void PaintController::checkUnderInvalidation() 534 void PaintController::checkUnderInvalidation()
529 { 535 {
530 DCHECK(RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled ()); 536 DCHECK(RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled ());
531 537
532 if (!isCheckingUnderInvalidation()) 538 if (!isCheckingUnderInvalidation())
533 return; 539 return;
534 540
535 const DisplayItem& newItem = m_newDisplayItemList.last(); 541 const DisplayItem& newItem = m_newDisplayItemList.last();
536 const DisplayItem& oldItem = m_currentPaintArtifact.getDisplayItemList()[m_u nderInvalidationCheckingBegin++]; 542 size_t oldItemIndex = m_underInvalidationCheckingBegin + m_skippedProbableUn derInvalidationCount;
543 const DisplayItem* oldItem = oldItemIndex < m_currentPaintArtifact.getDispla yItemList().size() ? &m_currentPaintArtifact.getDisplayItemList()[oldItemIndex] : nullptr;
537 544
538 if (newItem.isCacheable() && !clientCacheIsValid(newItem.client())) { 545 if (newItem.isCacheable() && !clientCacheIsValid(newItem.client())) {
539 showUnderInvalidationError("under-invalidation of PaintLayer: invalidate d in cached subsequence", newItem, oldItem); 546 showUnderInvalidationError("under-invalidation of PaintLayer: invalidate d in cached subsequence", newItem, oldItem);
540 NOTREACHED(); 547 NOTREACHED();
541 } 548 }
542 if (!newItem.equals(oldItem)) { 549
543 showUnderInvalidationError("under-invalidation: display item changed", n ewItem, oldItem); 550 bool oldAndNewEqual = oldItem && newItem.equals(*oldItem);
551 if (!oldAndNewEqual) {
552 if (newItem.isBegin()) {
553 // Temporarily skip mismatching begin display item which may be remo ved when we remove a no-op pair.
554 ++m_skippedProbableUnderInvalidationCount;
555 return;
556 }
557 if (newItem.isDrawing() && m_skippedProbableUnderInvalidationCount == 1) {
558 DCHECK_GE(m_newDisplayItemList.size(), 2u);
559 if (m_newDisplayItemList[m_newDisplayItemList.size() - 2].getType() == DisplayItem::BeginCompositing) {
560 // This might be a drawing item between a pair of begin/end comp ositing display items that will be folded
561 // into a single drawing display item.
562 ++m_skippedProbableUnderInvalidationCount;
563 return;
564 }
565 }
566 }
567
568 if (m_skippedProbableUnderInvalidationCount || !oldAndNewEqual) {
569 // If we ever skipped reporting any under-invalidations, report the earl iest one.
570 showUnderInvalidationError("under-invalidation: display item changed",
571 m_newDisplayItemList[m_newDisplayItemList.size() - m_skippedProbable UnderInvalidationCount - 1],
572 &m_currentPaintArtifact.getDisplayItemList()[m_underInvalidationChec kingBegin]);
544 NOTREACHED(); 573 NOTREACHED();
545 } 574 }
575
576 ++m_underInvalidationCheckingBegin;
546 } 577 }
547 578
548 #endif // DCHECK_IS_ON() 579 #endif // DCHECK_IS_ON()
549 580
550 #ifndef NDEBUG 581 #ifndef NDEBUG
551 582
552 String PaintController::displayItemListAsDebugString(const DisplayItemList& list ) const 583 String PaintController::displayItemListAsDebugString(const DisplayItemList& list ) const
553 { 584 {
554 StringBuilder stringBuilder; 585 StringBuilder stringBuilder;
555 size_t i = 0; 586 size_t i = 0;
(...skipping 20 matching lines...) Expand all
576 607
577 void PaintController::showDebugData() const 608 void PaintController::showDebugData() const
578 { 609 {
579 WTFLogAlways("current display item list: [%s]\n", displayItemListAsDebugStri ng(m_currentPaintArtifact.getDisplayItemList()).utf8().data()); 610 WTFLogAlways("current display item list: [%s]\n", displayItemListAsDebugStri ng(m_currentPaintArtifact.getDisplayItemList()).utf8().data());
580 WTFLogAlways("new display item list: [%s]\n", displayItemListAsDebugString(m _newDisplayItemList).utf8().data()); 611 WTFLogAlways("new display item list: [%s]\n", displayItemListAsDebugString(m _newDisplayItemList).utf8().data());
581 } 612 }
582 613
583 #endif // ifndef NDEBUG 614 #endif // ifndef NDEBUG
584 615
585 } // namespace blink 616 } // namespace blink
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698