commit 1d4c1c557d5e330235f03b9d3d482cd639a1c53c
parent f6c606113e585b446b774a8bfd9d0b060ec7a826
Author: Dan Robertson <dan@dlrobertson.com>
Date: Thu, 13 Nov 2025 04:59:07 +0000
Bug 1730749 - Rework handling of displayport clips for sticky content. r=mstange
To avoid sticky content checkerboarding when we've scrolled farther
than the displayport, we do not apply the displayport clip to it.
This was previously implemented in a hacky way in ClipManager, but
the introduction of sticky ASRs complicates that implementation.
Instead, this patch implements the omission of displayport clips
for sticky content in the Gecko display list itself.
For simplicity, only the case where the displayport clip is the
first element in a sticky item's clip chain is handled.
Differential Revision: https://phabricator.services.mozilla.com/D254156
Diffstat:
8 files changed, 62 insertions(+), 39 deletions(-)
diff --git a/gfx/layers/wr/ClipManager.cpp b/gfx/layers/wr/ClipManager.cpp
@@ -189,13 +189,6 @@ wr::WrSpaceAndClipChain ClipManager::SwitchItem(nsDisplayListBuilder* aBuilder,
// the sticky item.
auto* sticky = static_cast<nsDisplayStickyPosition*>(aItem);
asr = sticky->GetContainerASR();
-
- // If the leafmost clip for the sticky item is just the displayport clip,
- // then skip it. This allows sticky items to remain visible even if the
- // rest of the content in the enclosing scrollframe is checkerboarding.
- if (sticky->IsClippedToDisplayPort() && clip && clip->mASR == asr) {
- clip = clip->mParent;
- }
}
CLIP_LOG("processing item %p (%s) asr %p clip %p, inherited = %p\n", aItem,
diff --git a/layout/generic/ScrollContainerFrame.cpp b/layout/generic/ScrollContainerFrame.cpp
@@ -4011,13 +4011,15 @@ void ScrollContainerFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
scrolledRectClip = scrolledRectClip.Intersect(visibleRect);
clippedToDisplayPort = scrolledRectClip.IsEqualEdges(visibleRect);
}
- scrolledRectClipState.ClipContainingBlockDescendants(
- scrolledRectClip + aBuilder->ToReferenceFrame(this));
if (clippedToDisplayPort) {
+ scrolledRectClipState.ClipToDisplayPort(
+ scrolledRectClip + aBuilder->ToReferenceFrame(this));
+ } else {
// We have to do this after the ClipContainingBlockDescendants call
// above, otherwise that call will clobber the flag set by this call
// to SetClippedToDisplayPort.
- scrolledRectClipState.SetClippedToDisplayPort();
+ scrolledRectClipState.ClipContainingBlockDescendants(
+ scrolledRectClip + aBuilder->ToReferenceFrame(this));
}
nsRect visibleRectForChildren = visibleRect;
diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp
@@ -3483,6 +3483,11 @@ void nsIFrame::BuildDisplayListForStackingContext(
nsDisplayListBuilder::AutoInEventsOnly inEventsSetter(
aBuilder, opacityItemForEventsOnly);
+ DisplayListClipState::AutoSaveRestore stickyItemNestedClipState(aBuilder);
+ if (useStickyPosition && !shouldFlattenStickyItem) {
+ stickyItemNestedClipState.MaybeRemoveDisplayportClip();
+ }
+
// If we have a mask, compute a clip to bound the masked content.
// This is necessary in case the content moves with an ancestor
// ASR of the mask.
@@ -3919,14 +3924,15 @@ void nsIFrame::BuildDisplayListForStackingContext(
// that on the display item as the "container ASR" (i.e. the normal ASR of
// the container item, excluding the special behaviour induced by fixed
// descendants).
+ DisplayListClipState::AutoSaveRestore stickyItemClipState(aBuilder);
+ stickyItemClipState.MaybeRemoveDisplayportClip();
const ActiveScrolledRoot* stickyItemASR = ActiveScrolledRoot::PickAncestor(
containerItemASR, aBuilder->CurrentActiveScrolledRoot());
auto* stickyItem = MakeDisplayItem<nsDisplayStickyPosition>(
aBuilder, this, &resultList, stickyItemASR,
nsDisplayItem::ContainerASRType::AncestorOfContained,
- aBuilder->CurrentActiveScrolledRoot(),
- clipState.IsClippedToDisplayPort());
+ aBuilder->CurrentActiveScrolledRoot());
stickyItem->SetShouldFlatten(shouldFlattenStickyItem);
diff --git a/layout/painting/DisplayItemClipChain.h b/layout/painting/DisplayItemClipChain.h
@@ -84,10 +84,15 @@ struct DisplayItemClipChain {
{
}
+ bool IsDisplayportClip() const { return mKind == ClipKind::Displayport; }
+
+ enum class ClipKind : uint8_t { Displayport, Other };
+
DisplayItemClip mClip;
const ActiveScrolledRoot* mASR;
RefPtr<const DisplayItemClipChain> mParent;
uint32_t mRefCount = 0;
+ ClipKind mKind = ClipKind::Other;
DisplayItemClipChain* mNextClipChainToDestroy;
#if defined(DEBUG) || defined(MOZ_DIAGNOSTIC_ASSERT_ENABLED)
bool mOnStack;
diff --git a/layout/painting/DisplayListClipState.cpp b/layout/painting/DisplayListClipState.cpp
@@ -6,6 +6,7 @@
#include "DisplayListClipState.h"
+#include "DisplayItemClipChain.h"
#include "nsDisplayList.h"
namespace mozilla {
@@ -33,8 +34,11 @@ static void ApplyClip(nsDisplayListBuilder* aBuilder,
const ActiveScrolledRoot* aASR,
DisplayItemClipChain& aClipChainOnStack) {
aClipChainOnStack.mASR = aASR;
- if (aClipToModify && aClipToModify->mASR == aASR) {
+ if (aClipToModify && aClipToModify->mASR == aASR &&
+ !aClipChainOnStack.IsDisplayportClip()) {
// Intersect with aClipToModify and replace the clip chain item.
+ // Do not apply this optimization to displayport clips, because it would
+ // break our ability to skip them in MaybeRemoveDisplayportClip().
aClipChainOnStack.mClip.IntersectWith(aClipToModify->mClip);
aClipChainOnStack.mParent = aClipToModify->mParent;
aClipToModify = &aClipChainOnStack;
@@ -76,6 +80,17 @@ void DisplayListClipState::ClipContainingBlockDescendants(
InvalidateCurrentCombinedClipChain(asr);
}
+void DisplayListClipState::ClipToDisplayPort(
+ nsDisplayListBuilder* aBuilder, const nsRect& aRect,
+ DisplayItemClipChain& aClipChainOnStack) {
+ aClipChainOnStack.mClip.SetTo(aRect);
+ aClipChainOnStack.mKind = DisplayItemClipChain::ClipKind::Displayport;
+ const ActiveScrolledRoot* asr = aBuilder->CurrentActiveScrolledRoot();
+ ApplyClip(aBuilder, mClipChainContainingBlockDescendants, asr,
+ aClipChainOnStack);
+ InvalidateCurrentCombinedClipChain(asr);
+}
+
void DisplayListClipState::ClipContentDescendants(
nsDisplayListBuilder* aBuilder, const nsRect& aRect,
const nsRectCornerRadii* aRadii, DisplayItemClipChain& aClipChainOnStack) {
diff --git a/layout/painting/DisplayListClipState.h b/layout/painting/DisplayListClipState.h
@@ -26,11 +26,7 @@ class DisplayListClipState {
: mClipChainContentDescendants(nullptr),
mClipChainContainingBlockDescendants(nullptr),
mCurrentCombinedClipChain(nullptr),
- mCurrentCombinedClipChainIsValid(false),
- mClippedToDisplayPort(false) {}
-
- void SetClippedToDisplayPort() { mClippedToDisplayPort = true; }
- bool IsClippedToDisplayPort() const { return mClippedToDisplayPort; }
+ mCurrentCombinedClipChainIsValid(false) {}
/**
* Returns intersection of mClipChainContainingBlockDescendants and
@@ -85,6 +81,9 @@ class DisplayListClipState {
const nsRectCornerRadii* aRadii,
DisplayItemClipChain& aClipChainOnStack);
+ void ClipToDisplayPort(nsDisplayListBuilder* aBuilder, const nsRect& aRect,
+ DisplayItemClipChain& aClipChainOnStack);
+
void ClipContentDescendants(nsDisplayListBuilder* aBuilder,
const nsRect& aRect,
const nsRectCornerRadii* aRadii,
@@ -184,6 +183,15 @@ class DisplayListClipState::AutoSaveRestore {
mState.ClipContainingBlockDescendants(mBuilder, aRect, aRadii, mClipChain);
}
+ void ClipToDisplayPort(const nsRect& aRect) {
+ NS_ASSERTION(!mRestored, "Already restored!");
+ NS_ASSERTION(!mClipUsed, "mClip already used");
+#ifdef DEBUG
+ mClipUsed = true;
+#endif
+ mState.ClipToDisplayPort(mBuilder, aRect, mClipChain);
+ }
+
void ClipContentDescendants(const nsRect& aRect,
const nsRectCornerRadii* aRadii = nullptr) {
NS_ASSERTION(!mRestored, "Already restored!");
@@ -224,9 +232,19 @@ class DisplayListClipState::AutoSaveRestore {
mClipChain, aFlags);
}
- void SetClippedToDisplayPort() { mState.SetClippedToDisplayPort(); }
- bool IsClippedToDisplayPort() const {
- return mState.IsClippedToDisplayPort();
+ void MaybeRemoveDisplayportClip() {
+ if (!mState.mClipChainContainingBlockDescendants) return;
+
+ // Only remove the displayport clip in the case where it's the first
+ // element of the clip chain. This covers the vast majority of cases,
+ // and handling displayport clips later in the clip chain would introduce
+ // significant added complexity.
+ if (mState.mClipChainContainingBlockDescendants->IsDisplayportClip()) {
+ auto* displayportClipItem = mState.mClipChainContainingBlockDescendants;
+ mState.mClipChainContainingBlockDescendants =
+ mState.mClipChainContainingBlockDescendants->mParent;
+ mState.InvalidateCurrentCombinedClipChain(displayportClipItem->mASR);
+ }
}
protected:
diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp
@@ -5806,12 +5806,10 @@ void nsDisplayTableFixedPosition::Destroy(nsDisplayListBuilder* aBuilder) {
nsDisplayStickyPosition::nsDisplayStickyPosition(
nsDisplayListBuilder* aBuilder, nsIFrame* aFrame, nsDisplayList* aList,
const ActiveScrolledRoot* aActiveScrolledRoot,
- ContainerASRType aContainerASRType, const ActiveScrolledRoot* aContainerASR,
- bool aClippedToDisplayPort)
+ ContainerASRType aContainerASRType, const ActiveScrolledRoot* aContainerASR)
: nsDisplayOwnLayer(aBuilder, aFrame, aList, aActiveScrolledRoot,
aContainerASRType),
mContainerASR(aContainerASR),
- mClippedToDisplayPort(aClippedToDisplayPort),
mShouldFlatten(false) {
MOZ_COUNT_CTOR(nsDisplayStickyPosition);
}
diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h
@@ -5737,13 +5737,11 @@ class nsDisplayStickyPosition final : public nsDisplayOwnLayer {
nsDisplayList* aList,
const ActiveScrolledRoot* aActiveScrolledRoot,
ContainerASRType aContainerASRType,
- const ActiveScrolledRoot* aContainerASR,
- bool aClippedToDisplayPort);
+ const ActiveScrolledRoot* aContainerASR);
nsDisplayStickyPosition(nsDisplayListBuilder* aBuilder,
const nsDisplayStickyPosition& aOther)
: nsDisplayOwnLayer(aBuilder, aOther),
mContainerASR(aOther.mContainerASR),
- mClippedToDisplayPort(aOther.mClippedToDisplayPort),
mShouldFlatten(false) {
MOZ_COUNT_CTOR(nsDisplayStickyPosition);
}
@@ -5753,7 +5751,6 @@ class nsDisplayStickyPosition final : public nsDisplayOwnLayer {
const DisplayItemClip& GetClip() const override {
return DisplayItemClip::NoClip();
}
- bool IsClippedToDisplayPort() const { return mClippedToDisplayPort; }
NS_DISPLAY_DECL_NAME("StickyPosition", TYPE_STICKY_POSITION)
void Paint(nsDisplayListBuilder* aBuilder, gfxContext* aCtx) override {
@@ -5802,17 +5799,6 @@ class nsDisplayStickyPosition final : public nsDisplayOwnLayer {
// has no fixed descendants. This may be the same as the ASR returned by
// GetActiveScrolledRoot(), or it may be a descendant of that.
RefPtr<const ActiveScrolledRoot> mContainerASR;
- // This flag tracks if this sticky item is just clipped to the enclosing
- // scrollframe's displayport, or if there are additional clips in play. In
- // the former case, we can skip setting the displayport clip as the scrolled-
- // clip of the corresponding layer. This allows sticky items to remain
- // unclipped when the enclosing scrollframe is scrolled past the displayport.
- // i.e. when the rest of the scrollframe checkerboards, the sticky item will
- // not. This makes sense to do because the sticky item has abnormal scrolling
- // behavior and may still be visible even if the rest of the scrollframe is
- // checkerboarded. Note that the sticky item will still be subject to the
- // scrollport clip.
- bool mClippedToDisplayPort;
// True if this item should be flattened away.
bool mShouldFlatten;
