Bug 1422057 - Avoid caching clips across stacking contexts with non-identity transforms. r?mstange
When a clip is defined in WebRender, any transforms on the containing
stacking contexts are baked into the clip's position. Therefore, trying
to use a clip that was defined inside a transformed stacking context in
other parts of the WR display list doesn't work properly. This was a
latent bug in ScrollingLayersHelper that was previously not exposed
because in these cases Gecko itself creates separate
DisplayItemClipChain items. Now that we are going to deduplicate those
DisplayItemClipChain items, it exposes this latent bug which we need to
fix.
MozReview-Commit-ID: Icd7L1JuY8s
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -30,43 +30,51 @@ ScrollingLayersHelper::ScrollingLayersHe
void
ScrollingLayersHelper::BeginBuild(WebRenderLayerManager* aManager,
wr::DisplayListBuilder& aBuilder)
{
MOZ_ASSERT(!mManager);
mManager = aManager;
MOZ_ASSERT(!mBuilder);
mBuilder = &aBuilder;
- MOZ_ASSERT(mCache.empty());
+ MOZ_ASSERT(mCacheStack.empty());
+ mCacheStack.emplace_back();
MOZ_ASSERT(mScrollParents.empty());
MOZ_ASSERT(mItemClipStack.empty());
}
void
ScrollingLayersHelper::EndBuild()
{
mBuilder = nullptr;
mManager = nullptr;
- mCache.clear();
+ mCacheStack.pop_back();
+ MOZ_ASSERT(mCacheStack.empty());
mScrollParents.clear();
MOZ_ASSERT(mItemClipStack.empty());
}
void
-ScrollingLayersHelper::BeginList()
+ScrollingLayersHelper::BeginList(const StackingContextHelper& aStackingContext)
{
+ if (aStackingContext.IsReferenceFrame()) {
+ mCacheStack.emplace_back();
+ }
mItemClipStack.emplace_back(nullptr, nullptr);
}
void
-ScrollingLayersHelper::EndList()
+ScrollingLayersHelper::EndList(const StackingContextHelper& aStackingContext)
{
MOZ_ASSERT(!mItemClipStack.empty());
mItemClipStack.back().Unapply(mBuilder);
mItemClipStack.pop_back();
+ if (aStackingContext.IsReferenceFrame()) {
+ mCacheStack.pop_back();
+ }
}
void
ScrollingLayersHelper::BeginItem(nsDisplayItem* aItem,
const StackingContextHelper& aStackingContext)
{
SLH_LOG("processing item %p\n", aItem);
@@ -221,24 +229,25 @@ ScrollingLayersHelper::RecurseAndDefineC
const StackingContextHelper& aSc)
{
MOZ_ASSERT(aChain);
// This will hold our return value
std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>> ids;
if (mBuilder->HasExtraClip()) {
- // We can't use mCache directly. However if there's an out-of-band clip that
+ // We can't use the clip cache directly. However if there's an out-of-band clip that
// was pushed on top of aChain, we should return the id for that OOB clip,
// so that anything we want to define as a descendant of aChain we actually
// end up defining as a descendant of the OOB clip.
ids.second = mBuilder->GetCacheOverride(aChain);
} else {
- auto it = mCache.find(aChain);
- if (it != mCache.end()) {
+ const ClipIdMap& cache = mCacheStack.back();
+ auto it = cache.find(aChain);
+ if (it != cache.end()) {
ids.second = Some(it->second);
}
}
if (ids.second) {
// If we've already got an id for this clip, we can early-exit
if (aAsr) {
FrameMetrics::ViewID scrollId = aAsr->GetViewId();
MOZ_ASSERT(mBuilder->IsScrollLayerDefined(scrollId));
@@ -323,17 +332,17 @@ ScrollingLayersHelper::RecurseAndDefineC
nsTArray<wr::ComplexClipRegion> wrRoundedRects;
aChain->mClip.ToComplexClipRegions(aAppUnitsPerDevPixel, aSc, wrRoundedRects);
// Define the clip
wr::WrClipId clipId = mBuilder->DefineClip(
ancestorIds.first, ancestorIds.second,
aSc.ToRelativeLayoutRect(clip), &wrRoundedRects);
if (!mBuilder->HasExtraClip()) {
- mCache[aChain] = clipId;
+ mCacheStack.back()[aChain] = clipId;
}
ids.second = Some(clipId);
return ids;
}
std::pair<Maybe<FrameMetrics::ViewID>, Maybe<wr::WrClipId>>
ScrollingLayersHelper::RecurseAndDefineAsr(nsDisplayItem* aItem,
@@ -368,28 +377,29 @@ ScrollingLayersHelper::RecurseAndDefineA
auto it = mScrollParents.find(scrollId);
if (it != mScrollParents.end()) {
const DisplayItemClipChain* scrollParent = it->second;
if (DisplayItemClipChain::Equal(scrollParent, aChain)) {
canonicalChain = scrollParent;
}
}
- auto it2 = mCache.find(canonicalChain);
- // If |it == mCache.end()| here then we have run into a case where the
+ const ClipIdMap& cache = mCacheStack.back();
+ auto it2 = cache.find(canonicalChain);
+ // If |it2 == 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 == mCache.end()) {
+ 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);
}
@@ -481,17 +491,17 @@ ScrollingLayersHelper::EnclosingClipAndS
}
}
return Nothing();
}
ScrollingLayersHelper::~ScrollingLayersHelper()
{
MOZ_ASSERT(!mBuilder);
- MOZ_ASSERT(mCache.empty());
+ MOZ_ASSERT(mCacheStack.empty());
MOZ_ASSERT(mItemClipStack.empty());
}
ScrollingLayersHelper::ItemClips::ItemClips(const ActiveScrolledRoot* aAsr,
const DisplayItemClipChain* aChain)
: mAsr(aAsr)
, mChain(aChain)
{
--- a/gfx/layers/wr/ScrollingLayersHelper.h
+++ b/gfx/layers/wr/ScrollingLayersHelper.h
@@ -34,18 +34,18 @@ class ScrollingLayersHelper
{
public:
ScrollingLayersHelper();
void BeginBuild(WebRenderLayerManager* aManager,
wr::DisplayListBuilder& aBuilder);
void EndBuild();
- void BeginList();
- void EndList();
+ void BeginList(const StackingContextHelper& aStackingContext);
+ void EndList(const StackingContextHelper& aStackingContext);
void BeginItem(nsDisplayItem* aItem,
const StackingContextHelper& aStackingContext);
~ScrollingLayersHelper();
private:
typedef std::pair<FrameMetrics::ViewID, Maybe<wr::WrClipId>> ClipAndScroll;
@@ -81,17 +81,26 @@ private:
// 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;
- ClipIdMap mCache;
+ // 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);
--- a/gfx/layers/wr/StackingContextHelper.h
+++ b/gfx/layers/wr/StackingContextHelper.h
@@ -69,16 +69,17 @@ public:
gfx::Size GetInheritedScale() const { return mScale; }
const gfx::Matrix& GetInheritedTransform() const
{
return mInheritedTransform;
}
bool IsBackfaceVisible() const { return mTransform.IsBackfaceVisible(); }
+ bool IsReferenceFrame() const { return !mTransform.IsIdentity(); }
private:
wr::DisplayListBuilder* mBuilder;
gfx::Matrix4x4 mTransform;
gfx::Size mScale;
gfx::Matrix mInheritedTransform;
};
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -106,17 +106,17 @@ WebRenderCommandBuilder::BuildWebRenderC
void
WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(nsDisplayList* aDisplayList,
nsDisplayListBuilder* aDisplayListBuilder,
const StackingContextHelper& aSc,
wr::DisplayListBuilder& aBuilder,
wr::IpcResourceUpdateQueue& aResources)
{
- mScrollingHelper.BeginList();
+ mScrollingHelper.BeginList(aSc);
bool apzEnabled = mManager->AsyncPanZoomEnabled();
EventRegions eventRegions;
FlattenedDisplayItemIterator iter(aDisplayListBuilder, aDisplayList);
while (nsDisplayItem* i = iter.GetNext()) {
nsDisplayItem* item = i;
DisplayItemType itemType = item->GetType();
@@ -256,17 +256,17 @@ WebRenderCommandBuilder::CreateWebRender
// the most recently created layer data will have been created by an item
// with matching ASRs.
if (!eventRegions.IsEmpty()) {
MOZ_ASSERT(apzEnabled);
MOZ_ASSERT(!mLayerScrollData.empty());
mLayerScrollData.back().AddEventRegions(eventRegions);
}
- mScrollingHelper.EndList();
+ mScrollingHelper.EndList(aSc);
}
Maybe<wr::ImageKey>
WebRenderCommandBuilder::CreateImageKey(nsDisplayItem* aItem,
ImageContainer* aContainer,
mozilla::wr::DisplayListBuilder& aBuilder,
mozilla::wr::IpcResourceUpdateQueue& aResources,
const StackingContextHelper& aSc,