Bug 221067 – [GTK] Page scrolling by wheel events doesn't work in non-AC mode pages if Hardware Acceleration Policy is changed dynamically from "always" to "never"

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Scrolling

[Fujii Hironori]

Page scrolling by wheel events doesn't work in non-AC mode pages if async scrolling is enabled (WinCairo)

I'm trying to enable Async Scrolling for WinCairo. Attachment#418496 [details] is the WIP patch.

1. Start MiniBrowser
2. Go to a non-AC mode page. For example, https://trac.webkit.org/wiki
   Or, disable Accelerated Compositing by using menu.
3. Rolling the wheel of the mouse
4. the page doesn't scroll as expected

Mac port is setting setForceCompositingMode(true).

There was a similar issue:
Bug 219547 – REGRESSION(r270425) [GTK] wheel scrolling stopped working

[Fujii Hironori]

Created attachment 418598 [details]
WIP patch

[Fujii Hironori]

GTK port disables Threaded Scrolling in the default mode WEBKIT_HARDWARE_ACCELERATION_POLICY_ON_DEMAND.
https://github.com/WebKit/WebKit/blob/409d2a1d7c100ee0b7996b26a3b12ec2207609fc/Source/WebKit/UIProcess/gtk/WebPreferencesGtk.cpp#L49

So, a scrolling tree isn't generated for the page.
https://github.com/WebKit/WebKit/blob/409d2a1d7c100ee0b7996b26a3b12ec2207609fc/Source/WebKit/WebProcess/WebPage/WebPage.cpp#L742

[Fujii Hironori]

See also:
 Bug 202449 – [GTK][WPE] Enable async scrolling

[Fujii Hironori]

setForceCompositingMode
|       setThreadedScrollingEnabled
|       |
v       v
true   true   : Good (Mac, GTK, WPE)
false  false  : Good (GTK)
false  true   : Bad  (WinCairo)

It seems that WinCairo should align with other ports.

There are WKPreferencesSetAcceleratedCompositingEnabled and WKPreferencesSetThreadedScrollingEnabled.
However, no API corresponding to setForceCompositingMode.

[Fujii Hironori]

Created attachment 418609 [details]
WIP patch

[Fujii Hironori]

Comment on attachment 418609 [details]
WIP patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418609&action=review

> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp:256
> +    settings.setForceCompositingMode(store.getBoolValueForKey(WebPreferencesKey::acceleratedCompositingEnabledKey()));

This patch doesn't work well in the case of disabling AC in runtime by
using WKPreferencesSetAcceleratedCompositingEnabled(pref, false)
because the WebKit::WebPage already has a scrolling tree.

This problem can be reproduced with GTK MiniBrowser by switching
compositing policy from 'always' to 'never' in runtime.

[Fujii Hironori]

(In reply to Fujii Hironori from comment #6)
> This problem can be reproduced with GTK MiniBrowser by switching
> compositing policy from 'always' to 'never' in runtime.

I'm going to fix the GTK problem in this bug ticket.

1. Start GTK MiniBrowser
2. Set Hardware Acceleration Policy to "always" in Preferences
3. Go to a non-AC mode page. For example, https://trac.webkit.org/wiki
4. Set Hardware Acceleration Policy to "never" in Preferences
5. Reload
6. Roll the mouse wheel
7. The page scrolling doesn't work as expected.

[Fujii Hironori]

Created attachment 419553 [details]
Patch

[Carlos Garcia Campos]

hmm, does this mean that it would be possible to always use the async scrolling for AC mode? We currently enable it only when forced, assuming that it was needed to be enabled from the very beginning and it couldn't be enabled/disabled on demand.

[Simon Fraser (smfr)]

It would be much more reliable if AC were always enabled in GTK.

[Fujii Hironori]

(In reply to Carlos Garcia Campos from comment #9)
> hmm, does this mean that it would be possible to always use the async
> scrolling for AC mode? We currently enable it only when forced, assuming
> that it was needed to be enabled from the very beginning and it couldn't be
> enabled/disabled on demand.

Good point. But, I don't know the answer. I will test and check more the case.
Do you think that dynamically changing Hardware Acceleration Policy isn't supported in GTK port?

[Fujii Hironori]

(In reply to Simon Fraser (smfr) from comment #10)
> It would be much more reliable if AC were always enabled in GTK.

Yeah, I know Appple devs want to remove non-AC mode support from WebKit2.
However, GTK, WPE, PlayStation and WinCairo is still using non-AC mode in WebKit2.

WPE port is disabling accelerated compositing if it failed to create EGL context.
https://github.com/WebKit/WebKit/blob/f0f02bbbc09b40caf023955b32ca5c37639346f2/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp#L251,L252

[Fujii Hironori]

(In reply to Fujii Hironori from comment #11)
> (In reply to Carlos Garcia Campos from comment #9)
> > hmm, does this mean that it would be possible to always use the async
> > scrolling for AC mode? We currently enable it only when forced, assuming
> > that it was needed to be enabled from the very beginning and it couldn't be
> > enabled/disabled on demand.
> 
> Good point. But, I don't know the answer. I will test and check more the
> case.

I tested dynamically enabling/disabling force compositing mode by
changing Hardware Acceleration Policy in GTK MiniBrowser. It
works nicely.

Also tested setThreadedScrollingEnabled(true) in "ondemand"
Hardware Acceleration Policy. It also works nicely.

[Carlos Garcia Campos]

(In reply to Simon Fraser (smfr) from comment #10)
> It would be much more reliable if AC were always enabled in GTK.

We tried once and it didn't work for several reasons. First problem was memory consumption, the GL context created in every web process increases the memory usage. That's not a problem when you have just a few web views, but it became a problem when the browser had lots of tabs opened. Second problem was dealing with graphics drivers in linux. Our plan is to switch to AC mode only when building with GTK4, and try to improve the memory consumption (with tab suspension, for example). Or maybe the memory consumption is no longer a problem when using the GPU process. But for now, we have to keep the ondemand mode (note also that now ondemand never goes back to non-AC for a page after entering AC mode).

[Carlos Garcia Campos]

(In reply to Fujii Hironori from comment #13)
> (In reply to Fujii Hironori from comment #11)
> > (In reply to Carlos Garcia Campos from comment #9)
> > > hmm, does this mean that it would be possible to always use the async
> > > scrolling for AC mode? We currently enable it only when forced, assuming
> > > that it was needed to be enabled from the very beginning and it couldn't be
> > > enabled/disabled on demand.
> > 
> > Good point. But, I don't know the answer. I will test and check more the
> > case.
> 
> I tested dynamically enabling/disabling force compositing mode by
> changing Hardware Acceleration Policy in GTK MiniBrowser. It
> works nicely.
> 
> Also tested setThreadedScrollingEnabled(true) in "ondemand"
> Hardware Acceleration Policy. It also works nicely.

That's great, we should always enable async scrolling then, after this patch lands. Thanks!

[Fujii Hironori]

Darin asked me no to add a platform prefix "[GTK]" in the bug title for cross-platform code change.

[Fujii Hironori]

Created attachment 420247 [details]
Patch

[Fujii Hironori]

Comment on attachment 420247 [details]
Patch

Clearing flags on attachment: 420247

Committed r272867 (234101@main): <https://commits.webkit.org/234101@main>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/74357679>

[Truitt Savell]

It looks like the changes in https://trac.webkit.org/changeset/272867/webkit

broke 50+ tests on Mac Debug wk2

https://build.webkit.org/results/Apple-BigSur-Debug-WK2-Tests/r272879%20(158)/results.html

I am able to reproduce this on r272867 but not r272866

[Truitt Savell]

Reverted r272867 for reason:

Broke 50+ fast/layoutformattingcontext/ tests on Mac Debug WK2

Committed r272891 (234124@main): <https://commits.webkit.org/234124@main>

[Fujii Hironori]

Created attachment 420581 [details]
absolute-positioned-box-with-inline-sibling-crash-log.txt

Thank you for reverting my patch.
fast/layoutformattingcontext tests were failing because WebPage::enterAcceleratedCompositingMode was called twice.
The first time by RenderLayerCompositor::attachRootLayer, the second time by LayerController::attachRootLayer.
These tests are setting LayoutFormattingContextEnabled=true in the test header.

[Fujii Hironori]

I'm still investigating the assertion failure. I need more time.
I'm going to split the patch and land the part of it.
Bug 222529 – EventDispatcher::wheelEvent is accessing m_scrollingTrees without locking m_scrollingTreesMutex since r271235

[Fujii Hironori]

*** This bug has been marked as a duplicate of bug 225512 ***