Bug 1422057 - Remove now-unnecessary ::Equals checks. r?mstange
Part of this is a backout of
bug 1421054 which is no longer needed now
that we can assume equivalent clip items have identical pointer values.
MozReview-Commit-ID: BhnLVmVr4TX
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -32,28 +32,26 @@ ScrollingLayersHelper::BeginBuild(WebRen
wr::DisplayListBuilder& aBuilder)
{
MOZ_ASSERT(!mManager);
mManager = aManager;
MOZ_ASSERT(!mBuilder);
mBuilder = &aBuilder;
MOZ_ASSERT(mCacheStack.empty());
mCacheStack.emplace_back();
- MOZ_ASSERT(mScrollParents.empty());
MOZ_ASSERT(mItemClipStack.empty());
}
void
ScrollingLayersHelper::EndBuild()
{
mBuilder = nullptr;
mManager = nullptr;
mCacheStack.pop_back();
MOZ_ASSERT(mCacheStack.empty());
- mScrollParents.clear();
MOZ_ASSERT(mItemClipStack.empty());
}
void
ScrollingLayersHelper::BeginList(const StackingContextHelper& aStackingContext)
{
if (aStackingContext.IsReferenceFrame()) {
mCacheStack.emplace_back();
@@ -361,54 +359,30 @@ ScrollingLayersHelper::RecurseAndDefineA
FrameMetrics::ViewID scrollId = aAsr->GetViewId();
if (mBuilder->IsScrollLayerDefined(scrollId)) {
// If we've already defined this scroll layer before, we can early-exit
ids.first = Some(scrollId);
if (aChain) {
if (mBuilder->HasExtraClip()) {
ids.second = mBuilder->GetCacheOverride(aChain);
} else {
- // Since the scroll layer was already defined, find the clip (if any)
- // that it was previously defined as a child of. If that clip is
- // equivalent to |aChain|, then we should use that one in the mCache
- // lookup as it is more likely to produce a result. This happens because
- // of how we can have two DisplayItemClipChain items that are ::Equal
- // but not ==, and mCache only does == checking. In the hunk below,
- // |canonicalChain| can be thought of as the clip chain instance that is
- // equivalent to |aChain| but has the best chance of being found in
- // mCache.
- const DisplayItemClipChain* canonicalChain = aChain;
- auto it = mScrollParents.find(scrollId);
- if (it != mScrollParents.end()) {
- const DisplayItemClipChain* scrollParent = it->second;
- if (DisplayItemClipChain::Equal(scrollParent, aChain)) {
- canonicalChain = scrollParent;
- }
- }
-
const ClipIdMap& cache = mCacheStack.back();
- auto it2 = cache.find(canonicalChain);
- // If |it2 == cache.end()| here then we have run into a case where the
+ auto it = cache.find(aChain);
+ // If |it == cache.end()| here then we have run into a case where the
// scroll layer was previously defined with a specific parent clip, and
// now here it has a different parent clip. Gecko can create display
// lists like this because it treats the ASR chain and clipping chain
// more independently, but we can't yet represent this in WR. This is
// tracked by bug 1409442. For now we'll just leave ids.second as
// Nothing() which will effectively ignore the clip |aChain|. Once WR
// supports multiple ancestors on a scroll layer we can deal with this
// better. The layout/reftests/text/wordwrap-08.html has a Text display
// item that exercises this case.
- if (it2 == cache.end()) {
- // leave ids.second as Nothing(). This should only happen if we didn't
- // pick up a better canonicalChain above, either because it didn't
- // exist, or because it was not ::Equal to aChain. Therefore
- // canonicalChain must still be equal to aChain here.
- MOZ_ASSERT(canonicalChain == aChain);
- } else {
- ids.second = Some(it2->second);
+ if (it != cache.end()) {
+ ids.second = Some(it->second);
}
}
}
return ids;
}
// If not, recurse to ensure all the ancestors are defined
auto ancestorIds = DefineClipChain(
@@ -444,21 +418,16 @@ ScrollingLayersHelper::RecurseAndDefineA
ancestorIds.second = Nothing();
}
}
// At most one of the ancestor pair should be defined here, and the one that
// is defined will be the parent clip for the new scrollframe that we're
// defining.
MOZ_ASSERT(!(ancestorIds.first && ancestorIds.second));
- if (ancestorIds.second) {
- MOZ_ASSERT(aChain);
- mScrollParents[scrollId] = aChain;
- }
-
LayoutDeviceRect contentRect =
metrics.GetExpandedScrollableRect() * metrics.GetDevPixelsPerCSSPixel();
// TODO: check coordinate systems are sane here
LayoutDeviceRect clipBounds =
LayoutDeviceRect::FromUnknownRect(metrics.GetCompositionBounds().ToUnknownRect());
// The content rect that we hand to PushScrollLayer should be relative to
// the same origin as the clipBounds that we hand to PushScrollLayer - that
// is, both of them should be relative to the stacking context `aSc`.
--- a/gfx/layers/wr/ScrollingLayersHelper.h
+++ b/gfx/layers/wr/ScrollingLayersHelper.h
@@ -68,44 +68,31 @@ private:
const ActiveScrolledRoot* aAsr,
const DisplayItemClipChain* aChain,
int32_t aAppUnitsPerDevPixel,
const StackingContextHelper& aSc);
const DisplayItemClipChain* ExtendChain(const DisplayItemClipChain* aClip);
Maybe<ClipAndScroll> EnclosingClipAndScroll() const;
- // Note: two DisplayItemClipChain* A and B might actually be "equal" (as per
- // DisplayItemClipChain::Equal(A, B)) even though they are not the same pointer
- // (A != B). In this hopefully-rare case, they will get separate entries
- // in this map when in fact we could collapse them. However, to collapse
- // them involves writing a custom hash function for the pointer type such that
- // A and B hash to the same things whenever DisplayItemClipChain::Equal(A, B)
- // is true, and that will incur a performance penalty for all the hashmap
- // operations, so is probably not worth it. With the current code we might
- // end up creating multiple clips in WR that are effectively identical but
- // have separate clip ids. Hopefully this won't happen very often.
typedef std::unordered_map<const DisplayItemClipChain*, wr::WrClipId> ClipIdMap;
WebRenderLayerManager* MOZ_NON_OWNING_REF mManager;
wr::DisplayListBuilder* mBuilder;
// Stack of clip caches. There is one entry in the stack for each reference
// frame that is currently pushed in WR (a reference frame is a stacking
// context with a non-identity transform). Each entry contains a map that
// maps gecko DisplayItemClipChain objects to webrender WrClipIds, which
// allows us to avoid redefining identical clips in WR. We need to keep a
// separate cache per reference frame because the DisplayItemClipChain items
// themselves get deduplicated without regard to reference frames, but on the
// WR side we need to create different clips if they are in different
// reference frames.
std::vector<ClipIdMap> mCacheStack;
- typedef std::unordered_map<FrameMetrics::ViewID, const DisplayItemClipChain*> ScrollParentMap;
- ScrollParentMap mScrollParents;
-
struct ItemClips {
ItemClips(const ActiveScrolledRoot* aAsr,
const DisplayItemClipChain* aChain);
const ActiveScrolledRoot* mAsr;
const DisplayItemClipChain* mChain;
Maybe<FrameMetrics::ViewID> mScrollId;
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -2747,17 +2747,17 @@ PaintedLayerDataNode::FindPaintedLayerFo
if (!mPaintedLayerDataStack.IsEmpty()) {
PaintedLayerData* lowestUsableLayer = nullptr;
for (auto& data : Reversed(mPaintedLayerDataStack)) {
if (data.mVisibleAboveRegion.Intersects(aVisibleRect)) {
break;
}
if (data.mBackfaceHidden == aBackfaceHidden &&
data.mASR == aASR &&
- DisplayItemClipChain::Equal(data.mClipChain, aClipChain)) {
+ data.mClipChain == aClipChain) {
lowestUsableLayer = &data;
}
// Also check whether the event-regions intersect the visible rect,
// unless we're in an inactive layer, in which case the event-regions
// will be hoisted out into their own layer.
// For performance reasons, we check the intersection with the bounds
// of the event-regions.
if (!mTree.ContState().IsInInactiveLayer() &&