From 7b1b52b19b82ed0101f3e783909ab5d74bf1502d Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 11 Jun 2020 18:46:04 +0000 Subject: [PATCH] Bug 1592739 - Stop using the vibrant region as the transparent region. r=mattwoodrow This code was assuming that the only non-opaque parts of compositor rendering would be the parts of the window that had vibrancy. But now that the default window background is transparent, we can have non-vibrant parts where we render into transparency. Dialog windows such as sheet windows are an example of this. So instead of using the non-vibrant region of the window as its opaque region, we now use the region that is covered by opaque Gecko layers. This region is a lot more conservative: For example, the main browser chrome is now entirely transparent, because the chrome's opaque parts share a layer with its transparent parts. As a result, this change slightly affects the CALayer partitioning in the main browser window: The entire browser chrome is now transparent, not just the tab bar. The web content area is still opaque. I think this will be fine. The thing I'm most concerned about is that scrolling inside web content might cause invalidations of pixels from the chrome, because then we'd recomposite the CALayers that cover the vibrant tab bar. This doesn't seem to happen most of the time though, from what I can tell. Differential Revision: https://phabricator.services.mozilla.com/D51466 --- .../composite/LayerManagerComposite.cpp | 6 +---- widget/CompositorWidget.h | 18 --------------- widget/InProcessCompositorWidget.cpp | 6 ----- widget/InProcessCompositorWidget.h | 3 --- widget/cocoa/nsChildView.h | 7 ------ widget/cocoa/nsChildView.mm | 23 ------------------- widget/nsBaseWidget.h | 3 --- 7 files changed, 1 insertion(+), 65 deletions(-) diff --git a/gfx/layers/composite/LayerManagerComposite.cpp b/gfx/layers/composite/LayerManagerComposite.cpp index f742191ee120..1e8772470c73 100644 --- a/gfx/layers/composite/LayerManagerComposite.cpp +++ b/gfx/layers/composite/LayerManagerComposite.cpp @@ -1184,11 +1184,7 @@ bool LayerManagerComposite::Render(const nsIntRegion& aInvalidRegion, // opaque parts of the window are covered by different layers and we can // update those parts separately. IntRegion opaqueRegion; -#ifdef XP_MACOSX - opaqueRegion = - mCompositor->GetWidget()->GetOpaqueWidgetRegion().ToUnknownRegion(); -#endif - opaqueRegion.AndWith(mRenderBounds); + opaqueRegion.And(aOpaqueRegion, mRenderBounds); // Limit the complexity of these regions. Usually, opaqueRegion should be // only one or two rects, so this SimplifyInward call will not change the diff --git a/widget/CompositorWidget.h b/widget/CompositorWidget.h index e23d0b3dc245..34237a288b13 100644 --- a/widget/CompositorWidget.h +++ b/widget/CompositorWidget.h @@ -231,24 +231,6 @@ class CompositorWidget { */ virtual already_AddRefed EndBackBufferDrawing(); -#ifdef XP_MACOSX - /** - * Return the opaque region of the widget. This is racy and can only be used - * on macOS, where the widget works around the raciness. - * Bug 1576491 tracks fixing this properly. - * The problem with this method is that it can return values "from the future" - * - the compositor might be working on frame N but the widget will return its - * opaque region from frame N + 1. - * It is believed that this won't lead to visible glitches on macOS due to the - * SuspendAsyncCATransactions call when the vibrant region changes or when the - * window resizes. Whenever the compositor uses an opaque region that's a - * frame ahead, the result it renders won't be shown on the screen; instead, - * the next composite will happen with the correct display list, and that's - * what's shown on the screen once the FlushRendering call completes. - */ - virtual LayoutDeviceIntRegion GetOpaqueWidgetRegion() { return {}; } -#endif - /** * Observe or unobserve vsync. */ diff --git a/widget/InProcessCompositorWidget.cpp b/widget/InProcessCompositorWidget.cpp index 7d7ef5a5f9e6..38b94ac0b4bc 100644 --- a/widget/InProcessCompositorWidget.cpp +++ b/widget/InProcessCompositorWidget.cpp @@ -96,12 +96,6 @@ uintptr_t InProcessCompositorWidget::GetWidgetKey() { nsIWidget* InProcessCompositorWidget::RealWidget() { return mWidget; } -#ifdef XP_MACOSX -LayoutDeviceIntRegion InProcessCompositorWidget::GetOpaqueWidgetRegion() { - return mWidget->GetOpaqueWidgetRegion(); -} -#endif - void InProcessCompositorWidget::ObserveVsync(VsyncObserver* aObserver) { if (RefPtr cvd = mWidget->GetCompositorVsyncDispatcher()) { diff --git a/widget/InProcessCompositorWidget.h b/widget/InProcessCompositorWidget.h index f85aaf1c7080..d40db7bed50c 100644 --- a/widget/InProcessCompositorWidget.h +++ b/widget/InProcessCompositorWidget.h @@ -33,9 +33,6 @@ class InProcessCompositorWidget : public CompositorWidget { virtual bool InitCompositor(layers::Compositor* aCompositor) override; virtual LayoutDeviceIntSize GetClientSize() override; virtual uint32_t GetGLFrameBufferFormat() override; -#ifdef XP_MACOSX - virtual LayoutDeviceIntRegion GetOpaqueWidgetRegion() override; -#endif virtual void ObserveVsync(VsyncObserver* aObserver) override; virtual uintptr_t GetWidgetKey() override; diff --git a/widget/cocoa/nsChildView.h b/widget/cocoa/nsChildView.h index 64cb43ef4d71..bfcafd2529c8 100644 --- a/widget/cocoa/nsChildView.h +++ b/widget/cocoa/nsChildView.h @@ -491,8 +491,6 @@ class nsChildView final : public nsBaseWidget { virtual LayoutDeviceIntPoint GetClientOffset() override; - virtual LayoutDeviceIntRegion GetOpaqueWidgetRegion() override; - void DispatchAPZWheelInputEvent(mozilla::InputData& aEvent, bool aCanTriggerSwipe); nsEventStatus DispatchAPZInputEvent(mozilla::InputData& aEvent); @@ -533,8 +531,6 @@ class nsChildView final : public nsBaseWidget { void UpdateVibrancy(const nsTArray& aThemeGeometries); mozilla::VibrancyManager& EnsureVibrancyManager(); - void UpdateInternalOpaqueRegion(); - nsIWidget* GetWidgetForListenerEvents(); struct SwipeInfo { @@ -596,9 +592,6 @@ class nsChildView final : public nsBaseWidget { RefPtr mUnsuspendAsyncCATransactionsRunnable; - // The widget's opaque region. Written on the main thread, read on any thread. - mozilla::DataMutex mOpaqueRegion; - // This flag is only used when APZ is off. It indicates that the current pan // gesture was processed as a swipe. Sometimes the swipe animation can finish // before momentum events of the pan gesture have stopped firing, so this diff --git a/widget/cocoa/nsChildView.mm b/widget/cocoa/nsChildView.mm index f7e59934d5a7..21223a6bd561 100644 --- a/widget/cocoa/nsChildView.mm +++ b/widget/cocoa/nsChildView.mm @@ -236,7 +236,6 @@ nsChildView::nsChildView() mDrawing(false), mIsDispatchPaint(false), mPluginFocused{false}, - mOpaqueRegion("nsChildView::mOpaqueRegion"), mCurrentPanGestureBelongsToSwipe{false} {} nsChildView::~nsChildView() { @@ -506,8 +505,6 @@ void nsChildView::SetTransparencyMode(nsTransparencyMode aMode) { windowWidget->SetTransparencyMode(aMode); } - UpdateInternalOpaqueRegion(); - NS_OBJC_END_TRY_ABORT_BLOCK; } @@ -1434,11 +1431,6 @@ LayoutDeviceIntPoint nsChildView::WidgetToScreenOffset() { NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(LayoutDeviceIntPoint(0, 0)); } -LayoutDeviceIntRegion nsChildView::GetOpaqueWidgetRegion() { - auto opaqueRegion = mOpaqueRegion.Lock(); - return *opaqueRegion; -} - nsresult nsChildView::SetTitle(const nsAString& title) { // child views don't have titles return NS_OK; @@ -1900,8 +1892,6 @@ void nsChildView::UpdateVibrancy(const nsTArray& aThemeGeometries changed |= vm.UpdateVibrantRegion(VibrancyType::ACTIVE_SOURCE_LIST_SELECTION, activeSourceListSelectionRegion); - UpdateInternalOpaqueRegion(); - if (changed) { SuspendAsyncCATransactions(); } @@ -1915,19 +1905,6 @@ mozilla::VibrancyManager& nsChildView::EnsureVibrancyManager() { return *mVibrancyManager; } -void nsChildView::UpdateInternalOpaqueRegion() { - MOZ_RELEASE_ASSERT(NS_IsMainThread(), "This should only be called on the main thread."); - auto opaqueRegion = mOpaqueRegion.Lock(); - bool widgetIsOpaque = GetTransparencyMode() == eTransparencyOpaque; - if (!widgetIsOpaque) { - opaqueRegion->SetEmpty(); - } else if (VibrancyManager::SystemSupportsVibrancy()) { - opaqueRegion->Sub(mBounds, EnsureVibrancyManager().GetUnionOfVibrantRegions()); - } else { - *opaqueRegion = mBounds; - } -} - nsChildView::SwipeInfo nsChildView::SendMayStartSwipe( const mozilla::PanGestureInput& aSwipeStartEvent) { nsCOMPtr kungFuDeathGrip(this); diff --git a/widget/nsBaseWidget.h b/widget/nsBaseWidget.h index a5cec69ad988..fd3cafa6152f 100644 --- a/widget/nsBaseWidget.h +++ b/widget/nsBaseWidget.h @@ -465,9 +465,6 @@ class nsBaseWidget : public nsIWidget, public nsSupportsWeakReference { } virtual uint32_t GetGLFrameBufferFormat(); virtual bool CompositorInitiallyPaused() { return false; } -#ifdef XP_MACOSX - virtual LayoutDeviceIntRegion GetOpaqueWidgetRegion() { return {}; } -#endif protected: void ResolveIconName(const nsAString& aIconName, const nsAString& aIconSuffix,