From b715bc1076a33404fb922d4bc253c8f3683b9065 Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Tue, 7 Mar 2017 11:30:12 +0100 Subject: [PATCH] Bug 1343596 - Part 2: Refactor nsDisplayList sorting r=mattwoodrow MozReview-Commit-ID: DlUSf0iQm06 --HG-- extra : rebase_source : 20c5b58756d694149622cc145cc1d0d94ebb2cfa --- layout/painting/nsDisplayList.cpp | 111 ++++++++++++------------------ layout/painting/nsDisplayList.h | 26 +++++-- layout/tables/nsTableFrame.cpp | 14 ++-- 3 files changed, 69 insertions(+), 82 deletions(-) diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp index a8b493d22ad9..2ddca741dc09 100644 --- a/layout/painting/nsDisplayList.cpp +++ b/layout/painting/nsDisplayList.cpp @@ -2482,45 +2482,6 @@ void nsDisplayList::HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect, "How did we forget to pop some elements?"); } -static void Sort(nsDisplayList* aList, int32_t aCount, nsDisplayList::SortLEQ aCmp, - void* aClosure) { - if (aCount < 2) - return; - - nsDisplayList list1; - nsDisplayList list2; - int i; - int32_t half = aCount/2; - bool sorted = true; - nsDisplayItem* prev = nullptr; - for (i = 0; i < aCount; ++i) { - nsDisplayItem* item = aList->RemoveBottom(); - (i < half ? &list1 : &list2)->AppendToTop(item); - if (sorted && prev && !aCmp(prev, item, aClosure)) { - sorted = false; - } - prev = item; - } - if (sorted) { - aList->AppendToTop(&list1); - aList->AppendToTop(&list2); - return; - } - - Sort(&list1, half, aCmp, aClosure); - Sort(&list2, aCount - half, aCmp, aClosure); - - for (i = 0; i < aCount; ++i) { - if (list1.GetBottom() && - (!list2.GetBottom() || - aCmp(list1.GetBottom(), list2.GetBottom(), aClosure))) { - aList->AppendToTop(list1.RemoveBottom()); - } else { - aList->AppendToTop(list2.RemoveBottom()); - } - } -} - static nsIContent* FindContentInDocument(nsDisplayItem* aItem, nsIDocument* aDoc) { nsIFrame* f = aItem->Frame(); while (f) { @@ -2533,41 +2494,55 @@ static nsIContent* FindContentInDocument(nsDisplayItem* aItem, nsIDocument* aDoc return nullptr; } -static bool IsContentLEQ(nsDisplayItem* aItem1, nsDisplayItem* aItem2, - void* aClosure) { - nsIContent* commonAncestor = static_cast(aClosure); - // It's possible that the nsIContent for aItem1 or aItem2 is in a subdocument - // of commonAncestor, because display items for subdocuments have been - // mixed into the same list. Ensure that we're looking at content - // in commonAncestor's document. - nsIDocument* commonAncestorDoc = commonAncestor->OwnerDoc(); - nsIContent* content1 = FindContentInDocument(aItem1, commonAncestorDoc); - nsIContent* content2 = FindContentInDocument(aItem2, commonAncestorDoc); - if (!content1 || !content2) { - NS_ERROR("Document trees are mixed up!"); - // Something weird going on - return true; - } - return nsLayoutUtils::CompareTreePosition(content1, content2, commonAncestor) <= 0; -} +struct ZSortItem { + nsDisplayItem* item; + int32_t zIndex; -static bool IsZOrderLEQ(nsDisplayItem* aItem1, nsDisplayItem* aItem2, - void* aClosure) { - // Note that we can't just take the difference of the two - // z-indices here, because that might overflow a 32-bit int. - return aItem1->ZIndex() <= aItem2->ZIndex(); -} + explicit ZSortItem(nsDisplayItem* aItem) + : item(aItem), zIndex(aItem->ZIndex()) {} + + operator nsDisplayItem*() { + return item; + } +}; + +struct ZOrderComparator { + bool operator()(const ZSortItem& aLeft, const ZSortItem& aRight) const { + // Note that we can't just take the difference of the two + // z-indices here, because that might overflow a 32-bit int. + return aLeft.zIndex < aRight.zIndex; + } +}; void nsDisplayList::SortByZOrder() { - Sort(IsZOrderLEQ, nullptr); + Sort(ZOrderComparator()); } +struct ContentComparator { + nsIContent* mCommonAncestor; + + explicit ContentComparator(nsIContent* aCommonAncestor) + : mCommonAncestor(aCommonAncestor) {} + + bool operator()(nsDisplayItem* aLeft, nsDisplayItem* aRight) const { + // It's possible that the nsIContent for aItem1 or aItem2 is in a subdocument + // of commonAncestor, because display items for subdocuments have been + // mixed into the same list. Ensure that we're looking at content + // in commonAncestor's document. + nsIDocument* commonAncestorDoc = mCommonAncestor->OwnerDoc(); + nsIContent* content1 = FindContentInDocument(aLeft, commonAncestorDoc); + nsIContent* content2 = FindContentInDocument(aRight, commonAncestorDoc); + if (!content1 || !content2) { + NS_ERROR("Document trees are mixed up!"); + // Something weird going on + return true; + } + return nsLayoutUtils::CompareTreePosition(content1, content2, mCommonAncestor) < 0; + } +}; + void nsDisplayList::SortByContentOrder(nsIContent* aCommonAncestor) { - Sort(IsContentLEQ, aCommonAncestor); -} - -void nsDisplayList::Sort(SortLEQ aCmp, void* aClosure) { - ::Sort(this, Count(), aCmp, aClosure); + Sort(ContentComparator(aCommonAncestor)); } nsDisplayItem::nsDisplayItem(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame) diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h index ad3c1ed90960..1e8eaf6c6c15 100644 --- a/layout/painting/nsDisplayList.h +++ b/layout/painting/nsDisplayList.h @@ -2328,14 +2328,26 @@ public: void SortByContentOrder(nsIContent* aCommonAncestor); /** - * Generic stable sort. Take care, because some of the items might be nsDisplayLists - * themselves. - * aCmp(item1, item2) should return true if item1 <= item2. We sort the items - * into increasing order. + * Sort the display list using a stable sort. Take care, because some of the + * items might be nsDisplayLists themselves. + * aComparator(Item item1, Item item2) should return true if item1 should go + * before item2. + * We sort the items into increasing order. */ - typedef bool (* SortLEQ)(nsDisplayItem* aItem1, nsDisplayItem* aItem2, - void* aClosure); - void Sort(SortLEQ aCmp, void* aClosure); + template + void Sort(const Comparator& aComparator) { + nsTArray items; + + while (nsDisplayItem* item = RemoveBottom()) { + items.AppendElement(Item(item)); + } + + std::stable_sort(items.begin(), items.end(), aComparator); + + for (Item& item : items) { + AppendToTop(item); + } + } /** * Compute visiblity for the items in the list. diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index b452156c2abb..4541aa3d36a8 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1305,11 +1305,11 @@ GetTablePartRank(nsDisplayItem* aItem) return 3; } -static bool CompareByTablePartRank(nsDisplayItem* aItem1, nsDisplayItem* aItem2, - void* aClosure) -{ - return GetTablePartRank(aItem1) <= GetTablePartRank(aItem2); -} +struct TablePartRankComparator { + bool operator()(nsDisplayItem* aItem1, nsDisplayItem* aItem2) const { + return GetTablePartRank(aItem1) < GetTablePartRank(aItem2); + } +}; /* static */ void nsTableFrame::GenericTraversal(nsDisplayListBuilder* aBuilder, nsFrame* aFrame, @@ -1385,7 +1385,7 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, // Ensure that the table frame event background goes before the // table rowgroups event backgrounds, before the table row event backgrounds, // before everything else (cells and their blocks) - separatedCollection.BorderBackground()->Sort(CompareByTablePartRank, nullptr); + separatedCollection.BorderBackground()->Sort(TablePartRankComparator()); separatedCollection.MoveTo(aLists); } @@ -1402,7 +1402,7 @@ static inline bool FrameHasBorderOrBackground(nsTableFrame* tableFrame, nsIFrame } if (!f->StyleBackground()->IsTransparent(f) || f->StyleDisplay()->mAppearance) { - + nsTableCellFrame *cellFrame = do_QueryFrame(f); // We could also return false here if the current frame is the root // of a pseudo stacking context