commit 93fc4188c7c5f214eec33ce1bfb74ebc07c84f18
parent 9fcffbea18de868276fc53a5ee4bbc5cf23d3799
Author: Botond Ballo <botond@mozilla.com>
Date: Mon, 17 Nov 2025 22:59:08 +0000
Bug 2000271 - Use the sticky display item to compute bounds and reference frame offset in ClipManager::DefineStickyNode. r=mstange
As part of the refactor in bug 1730749, we tried to compute this
information based on the sticky frame only (specifically in the
patch https://phabricator.services.mozilla.com/D254153), but the
changes were not preserving the behaviour. Specifically:
* For a sticky frame that's also transformed, the sticky item's
bounds store the area rendered by the item in coordinates
outside the transform.
* toReferenceFrame() for such a sticky item returns an offset
to the reference frame enclosing the sticky item, whereas
aBuilder->toReferenceFrame(stickyFrame), our attempted
replacement, returned an offset to the sticky frame itself,
since the sticky frame was also transformed which made it
a reference frame of its own.
Differential Revision: https://phabricator.services.mozilla.com/D272919
Diffstat:
6 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/gfx/layers/wr/ClipManager.cpp b/gfx/layers/wr/ClipManager.cpp
@@ -8,6 +8,7 @@
#include "DisplayItemClipChain.h"
#include "FrameMetrics.h"
+#include "mozilla/ReverseIterator.h"
#include "mozilla/ScrollContainerFrame.h"
#include "mozilla/dom/Document.h"
#include "mozilla/layers/StackingContextHelper.h"
@@ -155,6 +156,15 @@ 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);
@@ -368,6 +378,19 @@ static nscoord NegativePart(nscoord min, nscoord max) {
return 0;
}
+const nsDisplayStickyPosition* ClipManager::FindStickyItemFromFrame(
+ const nsIFrame* aStickyFrame) const {
+ // Iterate in reverse order as the sticky item is more likely to be at
+ // the top of the stack.
+ for (const nsDisplayStickyPosition* item :
+ mozilla::Reversed(mStickyItemStack)) {
+ if (item->Frame() == aStickyFrame) {
+ return item;
+ }
+ }
+ return nullptr;
+}
+
Maybe<wr::WrSpatialId> ClipManager::DefineStickyNode(
nsDisplayListBuilder* aBuilder, Maybe<wr::WrSpatialId> aParentSpatialId,
const ActiveScrolledRoot* aASR, nsDisplayItem* aItem) {
@@ -393,7 +416,23 @@ Maybe<wr::WrSpatialId> ClipManager::DefineStickyNode(
float auPerDevPixel = stickyFrame->PresContext()->AppUnitsPerDevPixel();
- nsRect itemBounds = stickyFrame->GetRect();
+ nsRect itemBounds;
+ nsPoint toReferenceFrame;
+
+ const nsDisplayStickyPosition* stickyItem =
+ FindStickyItemFromFrame(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);
+ }
Maybe<float> topMargin;
Maybe<float> rightMargin;
@@ -410,7 +449,7 @@ Maybe<wr::WrSpatialId> ClipManager::DefineStickyNode(
nsPoint offset =
stickyScrollContainer->ScrollContainer()->GetOffsetToCrossDoc(
stickyFrame) +
- aBuilder->ToReferenceFrame(stickyFrame);
+ toReferenceFrame;
// Adjust the scrollPort coordinates to be relative to the reference frame,
// so that it is in the same space as everything else.
diff --git a/gfx/layers/wr/ClipManager.h b/gfx/layers/wr/ClipManager.h
@@ -17,6 +17,7 @@
namespace mozilla {
class nsDisplayItem;
+class nsDisplayStickyPosition;
struct ActiveScrolledRoot;
struct DisplayItemClipChain;
@@ -68,6 +69,9 @@ 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);
@@ -80,6 +84,8 @@ class ClipManager {
Maybe<wr::WrSpatialId> DefineStickyNode(
nsDisplayListBuilder* aBuilder, Maybe<wr::WrSpatialId> aParentSpatialId,
const ActiveScrolledRoot* aASR, nsDisplayItem* aItem);
+ const nsDisplayStickyPosition* FindStickyItemFromFrame(
+ const nsIFrame* aStickyFrame) const;
Maybe<wr::WrClipChainId> DefineClipChain(const DisplayItemClipChain* aChain,
int32_t aAppUnitsPerDevPixel);
@@ -121,6 +127,11 @@ 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,6 +1992,16 @@ 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,
@@ -2039,6 +2049,12 @@ 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
@@ -51,6 +51,7 @@ fuzzy(0-1,0-220) == block-in-inline-3.html block-in-inline-3-ref.html
== iframe-1.html iframe-1-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
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-3-ref.html b/layout/reftests/position-sticky/transformed-3-ref.html
@@ -0,0 +1,18 @@
+<style>
+ html, body {
+ margin: 0;
+ padding: 0;
+ scrollbar-width: none;
+ }
+ body {
+ height: 200vh;
+ }
+ #sticky {
+ background: grey;
+ position: absolute;
+ width: 100px;
+ height: 100px;
+ top: 200px;
+ }
+</style>
+<div id="sticky"></div>
diff --git a/layout/reftests/position-sticky/transformed-3.html b/layout/reftests/position-sticky/transformed-3.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: translateY(100px);
+ }
+</style>
+<div id="sticky"></div>
