Bug 202449 – [GTK][WPE] Enable async scrolling

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[GTK][WPE] Enable async scrolling

[Zan Dobersek]

Created attachment 380004 [details]
WIP Patch

[Carlos Garcia Campos]

Comment on attachment 380004 [details]
WIP Patch

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

LGTM

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3556
> +            changed = true;

This doesn't actually change the policy. Maybe we can simply call priv->preferences->setThreadedScrollingEnabled(true); right after priv->preferences->setForceCompositingMode(true); in the previous if.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3572
> +        if (priv->preferences->threadedScrollingEnabled()) {
> +            priv->preferences->setThreadedScrollingEnabled(false);
> +            changed = true;

Same here

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3586
> +        if (priv->preferences->threadedScrollingEnabled() && !HardwareAccelerationManager::singleton().forceHardwareAcceleration()) {
> +            priv->preferences->setThreadedScrollingEnabled(false);
> +            changed = true;

And here.

> Source/WebKit/UIProcess/gtk/WebPreferencesGtk.cpp:41
> +    struct {
> +        bool acceleratedCompositingEnabled { false };
> +        bool forceCompositingMode { false };
> +        bool threadedScrollingEnabled { false };
> +    } compositingState;

This would be easier to read using an enum with flags I think.

> Source/WebKit/UIProcess/gtk/WebPreferencesGtk.cpp:45
> +    else if (HardwareAccelerationManager::singleton().canUseHardwareAcceleration())

We still need to check canUseHardwareAcceleration() first, because if it returns false, we can't force it either.

[Zan Dobersek]

Comment on attachment 380004 [details]
WIP Patch

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

>> Source/WebKit/UIProcess/gtk/WebPreferencesGtk.cpp:41
>> +    } compositingState;
> 
> This would be easier to read using an enum with flags I think.

Unsure about enums. The values passed to the methods are still boolean, so I would just stick to those.

[Zan Dobersek]

Created attachment 380005 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Zan Dobersek]

Created attachment 380088 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 380088 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp:76
> +    auto* disableAsyncScrolling = getenv("WEBKIT_DISABLE_ASYNC_SCROLLING");
> +    if (disableAsyncScrolling && strcmp(disableAsyncScrolling, "0"))
> +        m_supportsAsyncScrolling = false;

We could do this only if m_supportsAsyncScrolling is true.

[Zan Dobersek]

Created attachment 380093 [details]
Patch for landing

[Zan Dobersek]

Comment on attachment 380093 [details]
Patch for landing

Clearing flags on attachment: 380093

Committed r250651: <https://trac.webkit.org/changeset/250651>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55941003>