From 5fac61e3d1bbb30332f99651febd83fa65619e10 Mon Sep 17 00:00:00 2001 From: Botond Ballo Date: Wed, 7 Oct 2020 22:35:07 +0000 Subject: [PATCH] Bug 1664101 - Store displayport margins in unadjusted form. r=kats Where an adjustment (to reflect a delta between the APZ and layout scroll offsets) is necessary, the inputs needed to compute the adjustment are stored with the margins, and the adjustment is applied at query time. A couple of notes on this patch: * Storing DisplayPortMargins::mLayoutOffset is probably unnecessary, we should be able to just query the scroll frame's layout offset when applying the margins. * Some callers of DisplayPortMargins::WithNoAdjustment() may be incorrect, in that they pass in margins that are relative to the visual viewport but do not make a corresponding adjustment. This is a pre-existing issue that this patch just makes clearer. As this is a regression-prone area, this patch is careful to avoid making any functional changes, leaving the above issues to be addressed in future bugs. Differential Revision: https://phabricator.services.mozilla.com/D92506 --- dom/base/nsDOMWindowUtils.cpp | 5 +- gfx/layers/apz/util/APZCCallbackHelper.cpp | 42 +++++-------- gfx/layers/apz/util/APZCCallbackHelper.h | 6 -- layout/base/DisplayPortUtils.cpp | 73 +++++++++++++++++----- layout/base/DisplayPortUtils.h | 44 +++++++++++-- layout/generic/nsGfxScrollFrame.cpp | 8 ++- 6 files changed, 123 insertions(+), 55 deletions(-) diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp index ecc371b95c27..b4e2573966e8 100644 --- a/dom/base/nsDOMWindowUtils.cpp +++ b/dom/base/nsDOMWindowUtils.cpp @@ -528,8 +528,9 @@ nsDOMWindowUtils::SetDisplayPortMarginsForElement( ScreenMargin displayportMargins(aTopMargin, aRightMargin, aBottomMargin, aLeftMargin); - DisplayPortUtils::SetDisplayPortMargins(aElement, presShell, - displayportMargins, aPriority); + DisplayPortUtils::SetDisplayPortMargins( + aElement, presShell, + DisplayPortMargins::WithNoAdjustment(displayportMargins), aPriority); return NS_OK; } diff --git a/gfx/layers/apz/util/APZCCallbackHelper.cpp b/gfx/layers/apz/util/APZCCallbackHelper.cpp index 39a2e976ab25..846bf6abce8f 100644 --- a/gfx/layers/apz/util/APZCCallbackHelper.cpp +++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp @@ -48,16 +48,6 @@ using dom::BrowserParent; uint64_t APZCCallbackHelper::sLastTargetAPZCNotificationInputBlock = uint64_t(-1); -ScreenMargin APZCCallbackHelper::AdjustDisplayPortForScrollDelta( - ScreenMargin aMargins, ScreenPoint aScrollDelta) { - ScreenMargin margins = aMargins; - margins.left -= aScrollDelta.x; - margins.right += aScrollDelta.x; - margins.top -= aScrollDelta.y; - margins.bottom += aScrollDelta.y; - return margins; -} - static ScreenMargin RecenterDisplayPort(const ScreenMargin& aDisplayPort) { ScreenMargin margins = aDisplayPort; margins.right = margins.left = margins.LeftRight() / 2; @@ -157,8 +147,8 @@ static CSSPoint ScrollFrameTo(nsIScrollableFrame* aFrame, * update the callback-transform stored on the content, and return a new * display port. */ -static ScreenMargin ScrollFrame(nsIContent* aContent, - const RepaintRequest& aRequest) { +static DisplayPortMargins ScrollFrame(nsIContent* aContent, + const RepaintRequest& aRequest) { // Scroll the window to the desired spot nsIScrollableFrame* sf = nsLayoutUtils::FindScrollableFrameFor(aRequest.GetScrollId()); @@ -182,7 +172,8 @@ static ScreenMargin ScrollFrame(nsIContent* aContent, } } bool scrollUpdated = false; - ScreenMargin displayPortMargins = aRequest.GetDisplayPortMargins(); + auto displayPortMargins = + DisplayPortMargins::WithNoAdjustment(aRequest.GetDisplayPortMargins()); CSSPoint apzScrollOffset = aRequest.GetVisualScrollOffset(); CSSPoint actualScrollOffset = ScrollFrameTo(sf, aRequest, scrollUpdated); CSSPoint scrollDelta = apzScrollOffset - actualScrollOffset; @@ -201,9 +192,9 @@ static ScreenMargin ScrollFrame(nsIContent* aContent, } else { // Correct the display port due to the difference between the requested // and actual scroll offsets. - displayPortMargins = APZCCallbackHelper::AdjustDisplayPortForScrollDelta( - aRequest.GetDisplayPortMargins(), - scrollDelta * aRequest.DisplayportPixelsPerCSSPixel()); + displayPortMargins = DisplayPortMargins::WithAdjustment( + aRequest.GetDisplayPortMargins(), apzScrollOffset, actualScrollOffset, + aRequest.DisplayportPixelsPerCSSPixel()); } } else if (aRequest.IsRootContent() && apzScrollOffset != aRequest.GetLayoutScrollOffset()) { @@ -214,9 +205,9 @@ static ScreenMargin ScrollFrame(nsIContent* aContent, // account for a difference between the requested and actual scroll // offsets in repaints requested by // AsyncPanZoomController::NotifyLayersUpdated. - displayPortMargins = APZCCallbackHelper::AdjustDisplayPortForScrollDelta( - aRequest.GetDisplayPortMargins(), - scrollDelta * aRequest.DisplayportPixelsPerCSSPixel()); + displayPortMargins = DisplayPortMargins::WithAdjustment( + aRequest.GetDisplayPortMargins(), apzScrollOffset, actualScrollOffset, + aRequest.DisplayportPixelsPerCSSPixel()); } else { // For whatever reason we couldn't update the scroll offset on the scroll // frame, which means the data APZ used for its displayport calculation is @@ -224,7 +215,8 @@ static ScreenMargin ScrollFrame(nsIContent* aContent, // tile-align the recentered displayport because tile-alignment depends on // the scroll position, and the scroll position here is out of our control. // See bug 966507 comment 21 for a more detailed explanation. - displayPortMargins = RecenterDisplayPort(aRequest.GetDisplayPortMargins()); + displayPortMargins = DisplayPortMargins::WithNoAdjustment( + RecenterDisplayPort(aRequest.GetDisplayPortMargins())); } // APZ transforms inputs assuming we applied the exact scroll offset it @@ -250,7 +242,7 @@ static ScreenMargin ScrollFrame(nsIContent* aContent, } static void SetDisplayPortMargins(PresShell* aPresShell, nsIContent* aContent, - ScreenMargin aDisplayPortMargins, + const DisplayPortMargins& aDisplayPortMargins, CSSSize aDisplayPortBase) { if (!aContent) { return; @@ -352,7 +344,7 @@ void APZCCallbackHelper::UpdateRootFrame(const RepaintRequest& aRequest) { // Do this as late as possible since scrolling can flush layout. It also // adjusts the display port margins, so do it before we set those. - ScreenMargin displayPortMargins = ScrollFrame(content, aRequest); + DisplayPortMargins displayPortMargins = ScrollFrame(content, aRequest); SetDisplayPortMargins(presShell, content, displayPortMargins, aRequest.CalculateCompositedSizeInCssPixels()); @@ -370,7 +362,7 @@ void APZCCallbackHelper::UpdateSubFrame(const RepaintRequest& aRequest) { // We don't currently support zooming for subframes, so nothing extra // needs to be done beyond the tasks common to this and UpdateRootFrame. - ScreenMargin displayPortMargins = ScrollFrame(content, aRequest); + DisplayPortMargins displayPortMargins = ScrollFrame(content, aRequest); if (RefPtr presShell = GetPresShell(content)) { SetDisplayPortMargins(presShell, content, displayPortMargins, aRequest.CalculateCompositedSizeInCssPixels()); @@ -435,8 +427,8 @@ void APZCCallbackHelper::InitializeRootDisplayport(PresShell* aPresShell) { DisplayPortUtils::SetDisplayPortBaseIfNotSet(content, baseRect); // Note that we also set the base rect that goes with these margins in // nsRootBoxFrame::BuildDisplayList. - DisplayPortUtils::SetDisplayPortMargins(content, aPresShell, ScreenMargin(), - 0); + DisplayPortUtils::SetDisplayPortMargins(content, aPresShell, + DisplayPortMargins::Empty(), 0); DisplayPortUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors( content->GetPrimaryFrame()); } diff --git a/gfx/layers/apz/util/APZCCallbackHelper.h b/gfx/layers/apz/util/APZCCallbackHelper.h index c76923436ee8..ae56cd6eb55a 100644 --- a/gfx/layers/apz/util/APZCCallbackHelper.h +++ b/gfx/layers/apz/util/APZCCallbackHelper.h @@ -172,12 +172,6 @@ class APZCCallbackHelper { static void CancelAutoscroll(const ScrollableLayerGuid::ViewID& aScrollId); - /* Adjust the display-port margins by the difference between the requested - * scroll offset and the resulting scroll offset after setting the requested - * value. */ - static ScreenMargin AdjustDisplayPortForScrollDelta(ScreenMargin aMargins, - ScreenPoint aScrollDelta); - /* * Check if the scrollable frame is currently in the middle of a main thread * async or smooth scroll, or has already requested some other apz scroll that diff --git a/layout/base/DisplayPortUtils.cpp b/layout/base/DisplayPortUtils.cpp index d7e18e7048ae..d17362a5eb02 100644 --- a/layout/base/DisplayPortUtils.cpp +++ b/layout/base/DisplayPortUtils.cpp @@ -23,6 +23,8 @@ #include "nsSubDocumentFrame.h" #include "RetainedDisplayListBuilder.h" +#include + namespace mozilla { using gfx::gfxVars; @@ -38,6 +40,44 @@ typedef ScrollableLayerGuid::ViewID ViewID; static LazyLogModule sDisplayportLog("apz.displayport"); +/* static */ +DisplayPortMargins DisplayPortMargins::WithAdjustment( + const ScreenMargin& aMargins, const CSSPoint& aVisualOffset, + const CSSPoint& aLayoutOffset, const CSSToScreenScale2D& aScale) { + return DisplayPortMargins{aMargins, aVisualOffset, aLayoutOffset, aScale}; +} + +/* static */ +DisplayPortMargins DisplayPortMargins::WithNoAdjustment( + const ScreenMargin& aMargins) { + // Use values such that GetRelativeToLayoutViewport() will just return + // mMargins. + return WithAdjustment(aMargins, CSSPoint(), CSSPoint(), + CSSToScreenScale2D(1.0, 1.0)); +} + +ScreenMargin DisplayPortMargins::GetRelativeToLayoutViewport() const { + ScreenPoint scrollDelta = (mVisualOffset - mLayoutOffset) * mScale; + ScreenMargin margins = mMargins; + margins.left -= scrollDelta.x; + margins.right += scrollDelta.x; + margins.top -= scrollDelta.y; + margins.bottom += scrollDelta.y; + return margins; +} + +std::ostream& operator<<(std::ostream& aOs, + const DisplayPortMargins& aMargins) { + if (aMargins.mVisualOffset == CSSPoint() && + aMargins.mLayoutOffset == CSSPoint()) { + aOs << aMargins.mMargins; + } else { + aOs << "{" << aMargins.mMargins << "," << aMargins.mVisualOffset << "," + << aMargins.mLayoutOffset << "}"; + } + return aOs; +} + // Return the maximum displayport size, based on the LayerManager's maximum // supported texture size. The result is in app units. static nscoord GetMaxDisplayPortSize(nsIContent* aContent, @@ -191,6 +231,8 @@ static nsRect GetDisplayPortFromMarginsData( bool useWebRender = gfxVars::UseWebRender(); + ScreenMargin margins = aMarginsData->mMargins.GetRelativeToLayoutViewport(); + if (presShell->IsDisplayportSuppressed()) { alignment = ScreenSize(1, 1); } else if (useWebRender) { @@ -230,7 +272,7 @@ static nsRect GetDisplayPortFromMarginsData( if (StaticPrefs::layers_enable_tiles_AtStartup() || useWebRender) { // Expand the rect by the margins - screenRect.Inflate(aMarginsData->mMargins); + screenRect.Inflate(margins); } else { // Calculate the displayport to make sure we fit within the max texture size // when not tiling. @@ -250,7 +292,6 @@ static nsRect GetDisplayPortFromMarginsData( MAX_ALIGN_ROUNDING * alignment.height; // For each axis, inflate the margins up to the maximum size. - const ScreenMargin& margins = aMarginsData->mMargins; if (screenRect.height < maxHeightScreenPx) { int32_t budget = maxHeightScreenPx - screenRect.height; // Scale the margins down to fit into the budget if necessary, maintaining @@ -295,7 +336,7 @@ static nsRect GetDisplayPortFromMarginsData( // If we have non-zero margins, expand the displayport for the low-res buffer // if that's what we're drawing. If we have zero margins, we want the // displayport to reflect the scrollport. - if (aMarginsData->mMargins != ScreenMargin()) { + if (margins != ScreenMargin()) { result = ApplyRectMultiplier(result, aMultiplier); } @@ -402,7 +443,7 @@ static bool GetDisplayPortImpl( result = GetDisplayPortFromRectData(aContent, rectData, aMultiplier); } else if (isDisplayportSuppressed || nsLayoutUtils::ShouldDisableApzForElement(aContent)) { - DisplayPortMarginsPropertyData noMargins(ScreenMargin(), 1, + DisplayPortMarginsPropertyData noMargins(DisplayPortMargins::Empty(), 1, /*painted=*/false); result = GetDisplayPortFromMarginsData(aContent, &noMargins, aMultiplier); } else { @@ -546,7 +587,7 @@ void DisplayPortUtils::InvalidateForDisplayPortChange( bool DisplayPortUtils::SetDisplayPortMargins(nsIContent* aContent, PresShell* aPresShell, - const ScreenMargin& aMargins, + const DisplayPortMargins& aMargins, uint32_t aPriority, RepaintMode aRepaintMode) { MOZ_ASSERT(aContent); @@ -745,8 +786,10 @@ bool DisplayPortUtils::CalculateAndSetDisplayPortMargins( ScreenMargin displayportMargins = layers::apz::CalculatePendingDisplayPort( metrics, ParentLayerPoint(0.0f, 0.0f)); PresShell* presShell = frame->PresContext()->GetPresShell(); - return SetDisplayPortMargins(content, presShell, displayportMargins, 0, - aRepaintMode); + return SetDisplayPortMargins( + content, presShell, + DisplayPortMargins::WithNoAdjustment(displayportMargins), 0, + aRepaintMode); } bool DisplayPortUtils::MaybeCreateDisplayPort(nsDisplayListBuilder* aBuilder, @@ -813,7 +856,8 @@ void DisplayPortUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors( if (nsLayoutUtils::AsyncPanZoomEnabled(frame) && !HasDisplayPort(frame->GetContent())) { SetDisplayPortMargins(frame->GetContent(), frame->PresShell(), - ScreenMargin(), 0, RepaintMode::Repaint); + DisplayPortMargins::Empty(), 0, + RepaintMode::Repaint); } } } @@ -945,14 +989,13 @@ static void UpdateDisplayPortMarginsForPendingMetrics( CSSPoint frameScrollOffset = CSSPoint::FromAppUnits(frame->GetScrollPosition()); - CSSPoint scrollDelta = aMetrics.GetVisualScrollOffset() - frameScrollOffset; - ScreenMargin displayPortMargins = - APZCCallbackHelper::AdjustDisplayPortForScrollDelta( - aMetrics.GetDisplayPortMargins(), - scrollDelta * aMetrics.DisplayportPixelsPerCSSPixel()); - DisplayPortUtils::SetDisplayPortMargins(content, presShell, - displayPortMargins, 0); + DisplayPortUtils::SetDisplayPortMargins( + content, presShell, + DisplayPortMargins::WithAdjustment( + aMetrics.GetDisplayPortMargins(), aMetrics.GetVisualScrollOffset(), + frameScrollOffset, aMetrics.DisplayportPixelsPerCSSPixel()), + 0); } /* static */ diff --git a/layout/base/DisplayPortUtils.h b/layout/base/DisplayPortUtils.h index 5985334dc30b..b6f1068fbe19 100644 --- a/layout/base/DisplayPortUtils.h +++ b/layout/base/DisplayPortUtils.h @@ -12,6 +12,7 @@ #include "nsRect.h" #include +#include class nsIContent; class nsIFrame; @@ -63,11 +64,45 @@ struct DisplayPortPropertyData { bool mPainted; }; +struct DisplayPortMargins { + // The margins relative to the visual scroll offset. + ScreenMargin mMargins; + + // Some information captured at the time the margins are stored. + // This ensures that we can express the margins as being relative to + // the correct scroll offset when applying them. + + // APZ's visual scroll offset at the time it requested the margins. + CSSPoint mVisualOffset; + + // The scroll frame's layout scroll offset at the time the margins + // were saved. + CSSPoint mLayoutOffset; + + // The scale required to convert between the CSS cordinates of + // mVisualOffset and mLayoutOffset, and the Screen coordinates of mMargins. + CSSToScreenScale2D mScale; + + static DisplayPortMargins WithAdjustment(const ScreenMargin& aMargins, + const CSSPoint& aVisualOffset, + const CSSPoint& aLayoutOffset, + const CSSToScreenScale2D& aScale); + + static DisplayPortMargins WithNoAdjustment(const ScreenMargin& aMargins); + + static DisplayPortMargins Empty() { return WithNoAdjustment(ScreenMargin()); } + + ScreenMargin GetRelativeToLayoutViewport() const; + + friend std::ostream& operator<<(std::ostream& aOs, + const DisplayPortMargins& aMargins); +}; + struct DisplayPortMarginsPropertyData { - DisplayPortMarginsPropertyData(const ScreenMargin& aMargins, + DisplayPortMarginsPropertyData(const DisplayPortMargins& aMargins, uint32_t aPriority, bool aPainted) : mMargins(aMargins), mPriority(aPriority), mPainted(aPainted) {} - ScreenMargin mMargins; + DisplayPortMargins mMargins; uint32_t mPriority; bool mPainted; }; @@ -159,8 +194,9 @@ class DisplayPortUtils { * @return true if the new margins were applied. */ static bool SetDisplayPortMargins( - nsIContent* aContent, PresShell* aPresShell, const ScreenMargin& aMargins, - uint32_t aPriority = 0, RepaintMode aRepaintMode = RepaintMode::Repaint); + nsIContent* aContent, PresShell* aPresShell, + const DisplayPortMargins& aMargins, uint32_t aPriority = 0, + RepaintMode aRepaintMode = RepaintMode::Repaint); /** * Set the display port base rect for given element to be used with display diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index cc4a8a570b2e..fc60450bdffc 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -2246,8 +2246,9 @@ ScrollFrameHelper::ScrollFrameHelper(nsContainerFrame* aOuter, bool aIsRoot) // If we have tiling but no APZ, then set a 0-margin display port on // active scroll containers so that we paint by whole tile increments // when scrolling. - DisplayPortUtils::SetDisplayPortMargins( - mOuter->GetContent(), mOuter->PresShell(), ScreenMargin(), 0); + DisplayPortUtils::SetDisplayPortMargins(mOuter->GetContent(), + mOuter->PresShell(), + DisplayPortMargins::Empty(), 0); DisplayPortUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors( mOuter); } @@ -3936,7 +3937,8 @@ void ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder* aBuilder, // with the silent change, since we explicitly request partial updates // to be disabled on the next paint. DisplayPortUtils::SetDisplayPortMargins( - mOuter->GetContent(), mOuter->PresShell(), ScreenMargin(), 0, + mOuter->GetContent(), mOuter->PresShell(), + DisplayPortMargins::Empty(), 0, DisplayPortUtils::RepaintMode::DoNotRepaint); // Call DecideScrollableLayer to recompute mWillBuildScrollableLayer // and recompute the current animated geometry root if needed. It's