commit 453decad1a322d0f4fed450cd44fb2cd68c4a1b1
parent e961ac0e3e6e8feac30630828a76c99795431fa4
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Wed, 17 Dec 2025 19:08:08 +0000
Bug 2006062 - Cache whole clip-chain IDs in ClipManager. r=gw,gfx-reviewers
Otherwise we define a lot of redundant clip-chains for the same list of
clips.
Reuse WebRender's hierarchy as well, rather than flattening all ancestor
clipchains into a single list of clips.
Differential Revision: https://phabricator.services.mozilla.com/D276456
Diffstat:
7 files changed, 86 insertions(+), 81 deletions(-)
diff --git a/gfx/layers/wr/ClipManager.cpp b/gfx/layers/wr/ClipManager.cpp
@@ -23,8 +23,7 @@
static mozilla::LazyLogModule sClipLog("wr.clip");
#define CLIP_LOG(...) MOZ_LOG(sClipLog, LogLevel::Debug, (__VA_ARGS__))
-namespace mozilla {
-namespace layers {
+namespace mozilla::layers {
ClipManager::ClipManager() : mManager(nullptr), mBuilder(nullptr) {}
@@ -202,13 +201,12 @@ wr::WrSpaceAndClipChain ClipManager::SwitchItem(nsDisplayListBuilder* aBuilder,
// In most cases we can combine the leaf of the clip chain with the clip rect
// of the display item. This reduces the number of clip items, which avoids
// some overhead further down the pipeline.
- bool separateLeaf = false;
- if (clip && clip->mASR == asr && clip->mClip.GetRoundedRectCount() == 0) {
- // Container display items are not currently supported because the clip
- // rect of a stacking context is not handled the same as normal display
- // items.
- separateLeaf = !aItem->GetChildren();
- }
+ // Container display items are not currently supported because the clip
+ // rect of a stacking context is not handled the same as normal display
+ // items.
+ const bool separateLeaf = clip && clip->mASR == asr &&
+ clip->mClip.GetRoundedRectCount() == 0 &&
+ !aItem->GetChildren();
// Zoom display items report their bounds etc using the parent document's
// APD because zoom items act as a conversion layer between the two different
@@ -672,59 +670,53 @@ Maybe<wr::WrSpatialId> ClipManager::DefineSpatialNodes(
Maybe<wr::WrClipChainId> ClipManager::DefineClipChain(
const DisplayItemClipChain* aChain, int32_t aAppUnitsPerDevPixel) {
MOZ_ASSERT(!mCacheStack.empty());
- AutoTArray<wr::WrClipId, 6> allClipIds;
- ClipIdMap& cache = mCacheStack.top();
- // Iterate through the clips in the current item's clip chain, define them
- // in WR, and put their IDs into |clipIds|.
- for (const DisplayItemClipChain* chain = aChain; chain;
- chain = chain->mParent) {
- MOZ_DIAGNOSTIC_ASSERT(chain->mOnStack || !chain->mASR ||
- chain->mASR->mFrame);
-
- if (!chain->mClip.HasClip()) {
- // This item in the chain is a no-op, skip over it
- continue;
- }
-
- auto emplaceResult = cache.try_emplace(chain);
- auto& chainClipIds = emplaceResult.first->second;
- if (!emplaceResult.second) {
- // Found it in the currently-active cache, so just use the id we have for
- // it.
- CLIP_LOG("cache[%p] => hit\n", chain);
- allClipIds.AppendElements(chainClipIds);
- continue;
- }
-
- LayoutDeviceRect clip = LayoutDeviceRect::FromAppUnits(
- chain->mClip.GetClipRect(), aAppUnitsPerDevPixel);
- AutoTArray<wr::ComplexClipRegion, 6> wrRoundedRects;
- chain->mClip.ToComplexClipRegions(aAppUnitsPerDevPixel, wrRoundedRects);
-
- wr::WrSpatialId space = GetSpatialId(chain->mASR);
- // Define the clip
- space = SpatialIdAfterOverride(space);
-
- auto rectClipId =
- mBuilder->DefineRectClip(Some(space), wr::ToLayoutRect(clip));
- CLIP_LOG("cache[%p] <= %zu\n", chain, rectClipId.id);
- chainClipIds.AppendElement(rectClipId);
-
- for (const auto& complexClip : wrRoundedRects) {
- auto complexClipId =
- mBuilder->DefineRoundedRectClip(Some(space), complexClip);
- CLIP_LOG("cache[%p] <= %zu\n", chain, complexClipId.id);
- chainClipIds.AppendElement(complexClipId);
- }
-
- allClipIds.AppendElements(chainClipIds);
- }
-
- if (allClipIds.IsEmpty()) {
+ if (!aChain) {
return Nothing();
}
- return Some(mBuilder->DefineClipChain(allClipIds));
+ ClipIdMap& cache = mCacheStack.top();
+ MOZ_DIAGNOSTIC_ASSERT(aChain->mOnStack || !aChain->mASR ||
+ aChain->mASR->mFrame);
+
+ if (auto iter = cache.find(aChain); iter != cache.end()) {
+ // Found it in the currently-active cache, so just use the id we have for
+ // it.
+ CLIP_LOG("cache[%p] => hit\n", aChain);
+ return iter->second.mWrChainID;
+ }
+
+ const auto parentChain =
+ DefineClipChain(aChain->mParent, aAppUnitsPerDevPixel);
+ if (!aChain->mClip.HasClip()) {
+ cache[aChain] = {parentChain};
+ // This item in the chain is a no-op, skip over it
+ return parentChain;
+ }
+
+ auto clip = LayoutDeviceRect::FromAppUnits(aChain->mClip.GetClipRect(),
+ aAppUnitsPerDevPixel);
+ AutoTArray<wr::ComplexClipRegion, 6> wrRoundedRects;
+ aChain->mClip.ToComplexClipRegions(aAppUnitsPerDevPixel, wrRoundedRects);
+ wr::WrSpatialId space = GetSpatialId(aChain->mASR);
+ // Define the clip
+ space = SpatialIdAfterOverride(space);
+ // Iterate through the clips in the current item's clip chain, define them
+ // in WR, and put their IDs into |clipIds|.
+ AutoTArray<wr::WrClipId, 6> clipChainClipIds;
+ auto rectClipId =
+ mBuilder->DefineRectClip(Some(space), wr::ToLayoutRect(clip));
+ CLIP_LOG("cache[%p] <= %zu\n", aChain, rectClipId.id);
+ clipChainClipIds.AppendElement(rectClipId);
+
+ for (const auto& complexClip : wrRoundedRects) {
+ auto complexClipId =
+ mBuilder->DefineRoundedRectClip(Some(space), complexClip);
+ CLIP_LOG("cache[%p] <= %zu\n", aChain, complexClipId.id);
+ clipChainClipIds.AppendElement(complexClipId);
+ }
+ auto id = Some(mBuilder->DefineClipChain(clipChainClipIds, parentChain));
+ cache[aChain] = {id};
+ return id;
}
ClipManager::~ClipManager() {
@@ -778,5 +770,4 @@ wr::WrSpaceAndClipChain ClipManager::ItemClips::GetSpaceAndClipChain() const {
return spaceAndClipChain;
}
-} // namespace layers
-} // namespace mozilla
+} // namespace mozilla::layers
diff --git a/gfx/layers/wr/ClipManager.h b/gfx/layers/wr/ClipManager.h
@@ -100,8 +100,11 @@ class ClipManager {
// general we need to do this anytime PushOverrideForASR is called, as that is
// called for the same set of conditions for which we cannot deduplicate
// clips.
- using ClipIdMap = std::unordered_map<const DisplayItemClipChain*,
- AutoTArray<wr::WrClipId, 4>>;
+ struct ClipChainCacheEntry {
+ Maybe<wr::WrClipChainId> mWrChainID;
+ };
+ using ClipIdMap =
+ std::unordered_map<const DisplayItemClipChain*, ClipChainCacheEntry>;
std::stack<ClipIdMap> mCacheStack;
// A map that holds the cache overrides created by (a) "out of band" clips,
diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -1276,14 +1276,10 @@ void DisplayListBuilder::PopStackingContext(bool aIsReferenceFrame) {
}
wr::WrClipChainId DisplayListBuilder::DefineClipChain(
- const nsTArray<wr::WrClipId>& aClips, bool aParentWithCurrentChain) {
+ Span<const wr::WrClipId> aClips, const Maybe<wr::WrClipChainId>& aParent) {
CancelGroup();
- const uint64_t* parent = nullptr;
- if (aParentWithCurrentChain &&
- mCurrentSpaceAndClipChain.clip_chain != wr::ROOT_CLIP_CHAIN) {
- parent = &mCurrentSpaceAndClipChain.clip_chain;
- }
+ const uint64_t* parent = aParent ? &aParent->id : nullptr;
uint64_t clipchainId = wr_dp_define_clipchain(
mWrState, parent, aClips.Elements(), aClips.Length());
if (MOZ_LOG_TEST(sWrDLLog, LogLevel::Debug)) {
@@ -1513,7 +1509,8 @@ void DisplayListBuilder::PushBackdropFilter(
ToString(aBounds).c_str(), ToString(clip).c_str());
auto clipId = DefineRoundedRectClip(Nothing(), aRegion);
- auto clipChainId = DefineClipChain({clipId}, true);
+ auto clipChainId =
+ DefineClipChain({&clipId, 1}, CurrentClipChainIdIfNotRoot());
auto spaceAndClip =
WrSpaceAndClipChain{mCurrentSpaceAndClipChain.space, clipChainId.id};
@@ -1790,7 +1787,8 @@ void DisplayListBuilder::SuspendClipLeafMerging() {
mSuspendedSpaceAndClipChain = Some(mCurrentSpaceAndClipChain);
auto clipId = DefineRectClip(Nothing(), *mClipChainLeaf);
- auto clipChainId = DefineClipChain({clipId}, true);
+ auto clipChainId =
+ DefineClipChain({&clipId, 1}, CurrentClipChainIdIfNotRoot());
mCurrentSpaceAndClipChain.clip_chain = clipChainId.id;
mClipChainLeaf = Nothing();
diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h
@@ -603,8 +603,8 @@ class DisplayListBuilder final {
const wr::RasterSpace& aRasterSpace);
void PopStackingContext(bool aIsReferenceFrame);
- wr::WrClipChainId DefineClipChain(const nsTArray<wr::WrClipId>& aClips,
- bool aParentWithCurrentChain = false);
+ wr::WrClipChainId DefineClipChain(Span<const wr::WrClipId> aClips,
+ const Maybe<wr::WrClipChainId>& aParent);
wr::WrClipId DefineImageMaskClip(const wr::ImageMask& aMask,
const nsTArray<wr::LayoutPoint>&,
@@ -831,6 +831,12 @@ class DisplayListBuilder final {
return mCurrentSpaceAndClipChain.clip_chain;
}
+ Maybe<wr::WrClipChainId> CurrentClipChainIdIfNotRoot() const {
+ return mCurrentSpaceAndClipChain.clip_chain != wr::ROOT_CLIP_CHAIN
+ ? Some(mCurrentSpaceAndClipChain.clip_chain)
+ : Nothing();
+ }
+
const wr::WrSpaceAndClipChain& CurrentSpaceAndClipChain() const {
return mCurrentSpaceAndClipChain;
}
diff --git a/layout/forms/nsFieldSetFrame.cpp b/layout/forms/nsFieldSetFrame.cpp
@@ -177,10 +177,12 @@ bool nsDisplayFieldSetBorder::CreateWebRenderCommands(
region.mode = wr::ClipMode::ClipOut;
region.radii = wr::EmptyBorderRadius();
- auto rect_clip = aBuilder.DefineRectClip(Nothing(), layoutRect);
- auto complex_clip = aBuilder.DefineRoundedRectClip(Nothing(), region);
- auto clipChain =
- aBuilder.DefineClipChain({rect_clip, complex_clip}, true);
+ std::array<wr::WrClipId, 2> clips = {
+ aBuilder.DefineRectClip(Nothing(), layoutRect),
+ aBuilder.DefineRoundedRectClip(Nothing(), region),
+ };
+ auto clipChain = aBuilder.DefineClipChain(
+ clips, aBuilder.CurrentClipChainIdIfNotRoot());
clipOut.emplace(aBuilder, clipChain);
}
} else {
diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp
@@ -8308,7 +8308,8 @@ static Maybe<wr::WrClipChainId> CreateSimpleClipRegion(
return Nothing();
}
- wr::WrClipChainId clipChainId = aBuilder.DefineClipChain({clipId}, true);
+ wr::WrClipChainId clipChainId = aBuilder.DefineClipChain(
+ {&clipId, 1}, aBuilder.CurrentClipChainIdIfNotRoot());
return Some(clipChainId);
}
@@ -8374,7 +8375,8 @@ static Maybe<wr::WrClipChainId> CreateWRClipPathAndMasks(
wr::WrClipId clipId =
aBuilder.DefineImageMaskClip(mask.ref(), points, fillRule);
- wr::WrClipChainId clipChainId = aBuilder.DefineClipChain({clipId}, true);
+ wr::WrClipChainId clipChainId = aBuilder.DefineClipChain(
+ {&clipId, 1}, aBuilder.CurrentClipChainIdIfNotRoot());
return Some(clipChainId);
}
@@ -8670,7 +8672,10 @@ bool nsDisplayFilters::CreateWebRenderCommands(
mFrame->PresContext()->AppUnitsPerDevPixel());
auto clipId =
aBuilder.DefineRectClip(Nothing(), wr::ToLayoutRect(devPxRect));
- clipChainId = aBuilder.DefineClipChain({clipId}, true).id;
+ clipChainId = aBuilder
+ .DefineClipChain({&clipId, 1},
+ aBuilder.CurrentClipChainIdIfNotRoot())
+ .id;
} else {
clipChainId = aBuilder.CurrentClipChainId();
}
diff --git a/layout/reftests/display-list/reftest.list b/layout/reftests/display-list/reftest.list
@@ -34,7 +34,7 @@ fuzzy(0-1,0-235200) == 1413073.html 1413073-ref.html
== 1417601-1.html 1417601-1-ref.html
== 1418945-1.html 1418945-1-ref.html
skip-if(Android) == 1428993-1.html 1428993-1-ref.html
-== 1420480-1.html 1420480-1-ref.html
+fuzzy(0-1,0-40000) == 1420480-1.html 1420480-1-ref.html
== 1428993-2.html 1428993-2-ref.html
needs-focus fuzzy(0-3,0-2) == 1429027-1.html 1429027-1-ref.html
== 1432553-1.html 1432553-1-ref.html
