Bug 1359808 - Don't do empty transactions for scroll updates if there are already pending transforms in the layer tree. r?mstange
The pending transforms must have been computed using the older scroll offset
values, which means that updating the scroll offsets without recomputing the
transforms will make them wrong. If we do an empty transaction for the scroll
offset updates, the transforms will not get computed. This patch catches this
scenario and schedules a full paint instead of the empty transaction instead.
The case where the scroll offset is modified *before* the transform is already
handled by code in nsIFrame::TryUpdateTransformOnly.
MozReview-Commit-ID: I5s5J7BS1ru
--- a/gfx/layers/Layers.cpp
+++ b/gfx/layers/Layers.cpp
@@ -2486,21 +2486,30 @@ LayerManager::ClearDisplayItemLayers()
}
/*static*/ bool
LayerManager::IsLogEnabled()
{
return MOZ_LOG_TEST(GetLog(), LogLevel::Debug);
}
-void
+bool
LayerManager::SetPendingScrollUpdateForNextTransaction(FrameMetrics::ViewID aScrollId,
const ScrollUpdateInfo& aUpdateInfo)
{
+ Layer* withPendingTransform = DepthFirstSearch<ForwardIterator>(GetRoot(),
+ [](Layer* aLayer) {
+ return aLayer->HasPendingTransform();
+ });
+ if (withPendingTransform) {
+ return false;
+ }
+
mPendingScrollUpdates[aScrollId] = aUpdateInfo;
+ return true;
}
Maybe<ScrollUpdateInfo>
LayerManager::GetPendingScrollInfoUpdate(FrameMetrics::ViewID aScrollId)
{
auto it = mPendingScrollUpdates.find(aScrollId);
if (it != mPendingScrollUpdates.end()) {
return Some(it->second);
--- a/gfx/layers/Layers.h
+++ b/gfx/layers/Layers.h
@@ -797,17 +797,17 @@ private:
FramesTimingRecording mRecording;
public:
/*
* Methods to store/get/clear a "pending scroll info update" object on a
* per-scrollid basis. This is used for empty transactions that push over
* scroll position updates to the APZ code.
*/
- void SetPendingScrollUpdateForNextTransaction(FrameMetrics::ViewID aScrollId,
+ bool SetPendingScrollUpdateForNextTransaction(FrameMetrics::ViewID aScrollId,
const ScrollUpdateInfo& aUpdateInfo);
Maybe<ScrollUpdateInfo> GetPendingScrollInfoUpdate(FrameMetrics::ViewID aScrollId);
void ClearPendingScrollInfoUpdate();
private:
std::map<FrameMetrics::ViewID,ScrollUpdateInfo> mPendingScrollUpdates;
// Display items are only valid during this transaction.
// At the end of the transaction, we have to go and clear out
@@ -1397,16 +1397,18 @@ public:
int32_t GetFixedPositionSides() { return mSimpleAttrs.FixedPositionSides(); }
FrameMetrics::ViewID GetStickyScrollContainerId() { return mSimpleAttrs.StickyScrollContainerId(); }
const LayerRect& GetStickyScrollRangeOuter() { return mSimpleAttrs.StickyScrollRangeOuter(); }
const LayerRect& GetStickyScrollRangeInner() { return mSimpleAttrs.StickyScrollRangeInner(); }
FrameMetrics::ViewID GetScrollbarTargetContainerId() { return mSimpleAttrs.ScrollbarTargetContainerId(); }
const ScrollThumbData& GetScrollThumbData() const { return mSimpleAttrs.ThumbData(); }
bool IsScrollbarContainer() { return mSimpleAttrs.IsScrollbarContainer(); }
Layer* GetMaskLayer() const { return mMaskLayer; }
+ bool HasPendingTransform() const { return mPendingTransform; }
+
void CheckCanary() const { mCanary.Check(); }
// Ancestor mask layers are associated with FrameMetrics, but for simplicity
// in maintaining the layer tree structure we attach them to the layer.
size_t GetAncestorMaskLayerCount() const {
return mAncestorMaskLayers.Length();
}
Layer* GetAncestorMaskLayerAt(size_t aIndex) const {
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -2868,27 +2868,31 @@ ScrollFrameHelper::ScrollToImpl(nsPoint
if (LastScrollOrigin() == nsGkAtoms::apz) {
schedulePaint = false;
PAINT_SKIP_LOG("Skipping due to APZ scroll\n");
} else if (mScrollableByAPZ) {
nsIWidget* widget = presContext->GetNearestWidget();
LayerManager* manager = widget ? widget->GetLayerManager() : nullptr;
if (manager) {
mozilla::layers::FrameMetrics::ViewID id;
- DebugOnly<bool> success = nsLayoutUtils::FindIDFor(content, &id);
+ bool success = nsLayoutUtils::FindIDFor(content, &id);
MOZ_ASSERT(success); // we have a displayport, we better have an ID
// Schedule an empty transaction to carry over the scroll offset update,
// instead of a full transaction. This empty transaction might still get
// squashed into a full transaction if something happens to trigger one.
- schedulePaint = false;
- manager->SetPendingScrollUpdateForNextTransaction(id,
+ success = manager->SetPendingScrollUpdateForNextTransaction(id,
{ mScrollGeneration, CSSPoint::FromAppUnits(GetScrollPosition()) });
- mOuter->SchedulePaint(nsIFrame::PAINT_COMPOSITE_ONLY);
- PAINT_SKIP_LOG("Skipping due to APZ-forwarded main-thread scroll\n");
+ if (success) {
+ schedulePaint = false;
+ mOuter->SchedulePaint(nsIFrame::PAINT_COMPOSITE_ONLY);
+ PAINT_SKIP_LOG("Skipping due to APZ-forwarded main-thread scroll\n");
+ } else {
+ PAINT_SKIP_LOG("Failed to set pending scroll update on layer manager\n");
+ }
}
}
}
}
}
if (schedulePaint) {
mOuter->SchedulePaint();