commit a22fae43e3022fe93b695b8931343cfb8139262c
parent 6c08e3a2d61f5667e682ea7359cb6eb70233a94e
Author: Botond Ballo <botond@mozilla.com>
Date: Tue, 25 Nov 2025 03:52:41 +0000
Bug 2001866 - Stop using the sticky display item in ClipManager::DefineStickyNode r=mstange
Differential Revision: https://phabricator.services.mozilla.com/D273733
Diffstat:
6 files changed, 55 insertions(+), 113 deletions(-)
diff --git a/gfx/layers/wr/ClipManager.cpp b/gfx/layers/wr/ClipManager.cpp
@@ -8,7 +8,6 @@
#include "DisplayItemClipChain.h"
#include "FrameMetrics.h"
-#include "mozilla/ReverseIterator.h"
#include "mozilla/ScrollContainerFrame.h"
#include "mozilla/dom/Document.h"
#include "mozilla/layers/StackingContextHelper.h"
@@ -16,6 +15,7 @@
#include "mozilla/StaticPrefs_apz.h"
#include "mozilla/webrender/WebRenderAPI.h"
#include "nsDisplayList.h"
+#include "nsLayoutUtils.h"
#include "nsRefreshDriver.h"
#include "nsStyleStructInlines.h"
#include "UnitTransforms.h"
@@ -156,15 +156,6 @@ void ClipManager::PopOverrideForASR(const ActiveScrolledRoot* aASR) {
}
}
-void ClipManager::PushStickyItem(const nsDisplayStickyPosition* aItem) {
- mStickyItemStack.push_back(aItem);
-}
-
-void ClipManager::PopStickyItem() {
- MOZ_ASSERT(!mStickyItemStack.empty());
- mStickyItemStack.pop_back();
-}
-
wr::WrSpatialId ClipManager::SpatialIdAfterOverride(
const wr::WrSpatialId& aSpatialId) {
auto it = mASROverride.find(aSpatialId);
@@ -386,56 +377,6 @@ static nscoord NegativePart(nscoord min, nscoord max) {
return 0;
}
-const nsDisplayStickyPosition* ClipManager::FindStickyItem(
- nsDisplayItem* aItemWithStickyASR, const nsIFrame* aStickyFrame) const {
- // Most common case: the sticky item is the item with the sticky ASR.
- if (aItemWithStickyASR->GetType() == DisplayItemType::TYPE_STICKY_POSITION &&
- aItemWithStickyASR->Frame() == aStickyFrame) {
- return static_cast<nsDisplayStickyPosition*>(aItemWithStickyASR);
- }
-
- // Next most common case: the item with the sticky ASR is a descendant of the
- // sticky item. We've pushed sticky items we've entered onto a stack,
- // so iterate it backwards to find our sticky item.
- for (const nsDisplayStickyPosition* item :
- mozilla::Reversed(mStickyItemStack)) {
- if (item->Frame() == aStickyFrame) {
- return item;
- }
- }
-
- // Edge case: the item with the sticky ASR is a wrapper display item wrapping
- // the sticky item (e.g. an nsDisplayBlendMode). Fish out the sticky item.
- if (aItemWithStickyASR->Frame() == aStickyFrame) {
- nsDisplayItem* item = aItemWithStickyASR;
- while (item) {
- nsDisplayList* children = item->GetChildren();
- if (!children) {
- return nullptr;
- }
- nsDisplayItem* onlyChild = nullptr;
- for (nsDisplayItem* child : *children) {
- if (!onlyChild) {
- onlyChild = child;
- } else {
- // More than one child
- return nullptr;
- }
- }
- if (!onlyChild || onlyChild->Frame() != aStickyFrame) {
- // Not a wrapping display item for the same frame.
- return nullptr;
- }
- if (onlyChild->GetType() == DisplayItemType::TYPE_STICKY_POSITION) {
- return static_cast<nsDisplayStickyPosition*>(onlyChild);
- }
- // Unwrap and keep looking.
- item = onlyChild;
- }
- }
- return nullptr;
-}
-
Maybe<wr::WrSpatialId> ClipManager::DefineStickyNode(
nsDisplayListBuilder* aBuilder, Maybe<wr::WrSpatialId> aParentSpatialId,
const ActiveScrolledRoot* aASR, nsDisplayItem* aItem) {
@@ -462,22 +403,21 @@ Maybe<wr::WrSpatialId> ClipManager::DefineStickyNode(
float auPerDevPixel = stickyFrame->PresContext()->AppUnitsPerDevPixel();
nsRect itemBounds;
- nsPoint toReferenceFrame;
-
- const nsDisplayStickyPosition* stickyItem =
- FindStickyItem(aItem, stickyFrame);
- if (stickyItem) {
- bool snap;
- itemBounds = stickyItem->GetBounds(aBuilder, &snap);
- toReferenceFrame = stickyItem->ToReferenceFrame();
- } else {
- MOZ_ASSERT(false,
- "Cannot find sticky item in ClipManager::DefineStickyNode, "
- "using fallback bounds which may be inaccurate");
- itemBounds = stickyFrame->GetRect() +
- aBuilder->ToReferenceFrame(stickyFrame->GetParent());
- toReferenceFrame = aBuilder->ToReferenceFrame(stickyFrame);
- }
+
+ // Make both itemBounds and scrollPort be relative to the reference frame
+ // of the sticky frame's parent.
+ nsRect scrollPort =
+ stickyScrollContainer->ScrollContainer()->GetScrollPortRect();
+ const nsIFrame* referenceFrame =
+ aBuilder->FindReferenceFrameFor(stickyFrame->GetParent());
+ nsRect transformedBounds = stickyFrame->GetRectRelativeToSelf();
+ DebugOnly transformResult = nsLayoutUtils::TransformRect(
+ stickyFrame, referenceFrame, transformedBounds);
+ MOZ_ASSERT(transformResult == nsLayoutUtils::TRANSFORM_SUCCEEDED);
+ itemBounds = transformedBounds;
+ scrollPort = scrollPort +
+ stickyScrollContainer->ScrollContainer()->GetOffsetToCrossDoc(
+ referenceFrame);
Maybe<float> topMargin;
Maybe<float> rightMargin;
@@ -491,17 +431,6 @@ Maybe<wr::WrSpatialId> ClipManager::DefineStickyNode(
nsRectAbsolute inner;
stickyScrollContainer->GetScrollRanges(stickyFrame, &outer, &inner);
- nsPoint offset =
- stickyScrollContainer->ScrollContainer()->GetOffsetToCrossDoc(
- stickyFrame) +
- toReferenceFrame;
-
- // Adjust the scrollPort coordinates to be relative to the reference frame,
- // so that it is in the same space as everything else.
- nsRect scrollPort =
- stickyScrollContainer->ScrollContainer()->GetScrollPortRect();
- scrollPort += offset;
-
// The following computations make more sense upon understanding the
// semantics of "inner" and "outer", which is explained in the comment on
// SetStickyPositionData in Layers.h.
diff --git a/gfx/layers/wr/ClipManager.h b/gfx/layers/wr/ClipManager.h
@@ -69,9 +69,6 @@ class ClipManager {
const wr::WrSpatialId& aSpatialId);
void PopOverrideForASR(const ActiveScrolledRoot* aASR);
- void PushStickyItem(const nsDisplayStickyPosition* aItem);
- void PopStickyItem();
-
private:
wr::WrSpatialId SpatialIdAfterOverride(const wr::WrSpatialId& aSpatialId);
wr::WrSpatialId GetSpatialId(const ActiveScrolledRoot* aASR);
@@ -84,8 +81,6 @@ class ClipManager {
Maybe<wr::WrSpatialId> DefineStickyNode(
nsDisplayListBuilder* aBuilder, Maybe<wr::WrSpatialId> aParentSpatialId,
const ActiveScrolledRoot* aASR, nsDisplayItem* aItem);
- const nsDisplayStickyPosition* FindStickyItem(
- nsDisplayItem* aItemWithStickyASR, const nsIFrame* aStickyFrame) const;
Maybe<wr::WrClipChainId> DefineClipChain(const DisplayItemClipChain* aChain,
int32_t aAppUnitsPerDevPixel);
@@ -127,11 +122,6 @@ class ClipManager {
// is why we need a stack.
std::unordered_map<wr::WrSpatialId, std::stack<wr::WrSpatialId>> mASROverride;
- // A stack of sticky display items that WebRender display list building has
- // entered. This is used to find the sticky display item corresponding to
- // a sticky frame when creating a sticky spatial node in DefineStickyNode.
- std::vector<const nsDisplayStickyPosition*> mStickyItemStack;
-
// This holds some clip state for a single nsDisplayItem
struct ItemClips {
ItemClips(const ActiveScrolledRoot* aASR,
diff --git a/gfx/layers/wr/WebRenderCommandBuilder.cpp b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -1992,16 +1992,6 @@ struct MOZ_STACK_CLASS WebRenderCommandBuilder::AutoOpaqueRegionStateTracker {
}
};
-struct MOZ_STACK_CLASS AutoEnterStickyItem {
- ClipManager& mClipManager;
-
- AutoEnterStickyItem(ClipManager& aClipManager, nsDisplayStickyPosition* aItem)
- : mClipManager(aClipManager) {
- mClipManager.PushStickyItem(aItem);
- }
- ~AutoEnterStickyItem() { mClipManager.PopStickyItem(); }
-};
-
void WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(
nsDisplayList* aDisplayList, nsDisplayItem* aWrappingItem,
nsDisplayListBuilder* aDisplayListBuilder, const StackingContextHelper& aSc,
@@ -2049,12 +2039,6 @@ void WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(
// applied.
Maybe<AutoDisplayItemCacheSuppressor> cacheSuppressor;
- Maybe<AutoEnterStickyItem> autoStickyItem;
- if (itemType == DisplayItemType::TYPE_STICKY_POSITION) {
- autoStickyItem.emplace(mClipManager,
- static_cast<nsDisplayStickyPosition*>(item));
- }
-
if (itemType == DisplayItemType::TYPE_OPACITY) {
nsDisplayOpacity* opacity = static_cast<nsDisplayOpacity*>(item);
diff --git a/layout/reftests/position-sticky/reftest.list b/layout/reftests/position-sticky/reftest.list
@@ -52,6 +52,7 @@ fuzzy(0-1,0-220) == block-in-inline-3.html block-in-inline-3-ref.html
== transformed-1.html transformed-1-ref.html
skip-if(useDrawSnapshot) fails-if(useDrawSnapshot) fuzzy-if(Android,0-8,0-10) fuzzy-if(cocoaWidget,7-8,18-42) fuzzy-if(cocoaWidget&&isDebugBuild&&!swgl,0-14,0-31) fuzzy-if(gtkWidget,5-21,12-32) == transformed-2.html transformed-2-ref.html # Bug 1604644, Bug 1934906
== transformed-3.html transformed-3-ref.html
+== transformed-4.html transformed-4-ref.html
skip-if(useDrawSnapshot) fails-if(useDrawSnapshot) fuzzy-if(Android,0-14,0-17) fuzzy-if(cocoaWidget,13-16,20-44) fuzzy-if(cocoaWidget&&isDebugBuild&&!swgl,0-29,0-36) fuzzy-if(gtkWidget,8-37,12-32) == nested-sticky-1.html nested-sticky-1-ref.html # Bug 1604644, Bug 1934906
skip-if(useDrawSnapshot) fails-if(useDrawSnapshot) fuzzy-if(Android,0-14,0-96) fuzzy-if(cocoaWidget,13-16,20-44) fuzzy-if(cocoaWidget&&isDebugBuild&&!swgl,0-29,0-36) fuzzy-if(gtkWidget,8-37,12-32) == nested-sticky-2.html nested-sticky-2-ref.html # Bug 1604644, Bug 1934906
skip == fixed-inside-sticky-clip.html fixed-inside-sticky-clip-ref.html # should be fixed by bug 1730749
diff --git a/layout/reftests/position-sticky/transformed-4-ref.html b/layout/reftests/position-sticky/transformed-4-ref.html
@@ -0,0 +1,19 @@
+<style>
+ html, body {
+ margin: 0;
+ padding: 0;
+ scrollbar-width: none;
+ }
+ body {
+ height: 200vh;
+ }
+ #sticky {
+ background: grey;
+ position: absolute;
+ width: 200px;
+ height: 200px;
+ left: -50px;
+ top: 50px;
+ }
+</style>
+<div id="sticky"></div>
diff --git a/layout/reftests/position-sticky/transformed-4.html b/layout/reftests/position-sticky/transformed-4.html
@@ -0,0 +1,19 @@
+<style>
+ html, body {
+ margin: 0;
+ padding: 0;
+ scrollbar-width: none;
+ }
+ body {
+ height: 200vh;
+ }
+ #sticky {
+ background: grey;
+ position: sticky;
+ width: 100px;
+ height: 100px;
+ top: 100px;
+ transform: scale(2);
+ }
+</style>
+<div id="sticky"></div>
