Bug 214035

Summary: [GTK][WinCairo] position:fixed layers are unnecessarily repainted by scrolling main content layer
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: WebKit2Assignee: Fujii Hironori <Hironori.Fujii>
Status: NEW ---    
Severity: Normal CC: bugs-noreply, cgarcia, changseok, cmarcelo, ews-watchlist, gyuyoung.kim, luiz, magomez, noam, ryuan.choi, sergio, zan, zeno
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
test content
none
[Screenshot] GTK port
none
Patch
none
test content 2
none
test content (position:fixed element on other layer)
none
WIP patch none

Description Fujii Hironori 2020-07-07 00:55:58 PDT
Created attachment 403671 [details]
test content

[GTK][WinCairo] position:fixed layers are unnecessarily repainted by scrolling main content layer

1. Start MiniBrowser
2. Enable repainting counter
   [WinCairo] Menu → Debug  → Show Compositing Borders
   [GTK] prefereces... → Draw compositing indicaters
3. Open the test content
4. Scroll down by mouse wheel

Actual: The repainting counter of position:fixed layer is rapidly increasing.
Expected: No such repainting
Comment 1 Fujii Hironori 2020-07-07 00:57:25 PDT
I confirmed Mac port MiniBrowser doesn't have this issue.
Comment 2 Fujii Hironori 2020-07-07 01:00:01 PDT
Created attachment 403672 [details]
[Screenshot] GTK port
Comment 3 Fujii Hironori 2020-07-07 01:14:53 PDT
DrawingAreaCoordinatedGraphics::updatePreferences is enabling settings.setAcceleratedCompositingForFixedPositionEnabled if acceleratedCompositingEnabled.
https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp?rev=263259#L251

>     // Fixed position elements need to be composited and create stacking contexts
>     // in order to be scrolled by the ScrollingCoordinator.
>     settings.setAcceleratedCompositingForFixedPositionEnabled(settings.acceleratedCompositingEnabled());


This code was added by
Bug 154100 – [ThreadedCompositor] position:fixed elements do not have their own layers when threaded compositor is enabled.
Comment 4 Fujii Hironori 2020-07-07 01:22:36 PDT
WebChromeClient::createScrollingCoordinator always returns nullptr in GTK port.
Is ScrollingCoordinator really used?
Comment 5 Carlos Garcia Campos 2020-07-07 05:55:19 PDT
(In reply to Fujii Hironori from comment #4)
> WebChromeClient::createScrollingCoordinator always returns nullptr in GTK
> port.
> Is ScrollingCoordinator really used?

I think tt's used when AC mode is forced, try with WEBKIT_FORCE_COMPOSITING_MODE.
Comment 6 Fujii Hironori 2020-07-07 17:27:57 PDT
Good to know. Thank you for letting me know that.
I confirmed GTK port doesn't exhibits this issue if WEBKIT_FORCE_COMPOSITING_MODE=1.
Comment 7 Fujii Hironori 2020-07-07 18:45:26 PDT
Created attachment 403750 [details]
Patch
Comment 8 Fujii Hironori 2020-07-07 18:49:28 PDT
This is the simplest solution 😅
Comment 9 Fujii Hironori 2020-07-07 19:42:29 PDT
Created attachment 403752 [details]
test content 2

position:fixed doesn't work fine in this test case with the proposed patch.
This seems a regression of the patch.
Comment 10 Fujii Hironori 2020-07-07 19:53:00 PDT
By testing the patch with WinCairo, I found more problems for it:

https://www.youtube.com/
The side menu isn't solidly fixed while scrolling the main part.

https://www.yomiuri.co.jp/
It still exhibits the unnecessary repainting issue for position:fixed elements.
Comment 11 Fujii Hironori 2020-07-07 20:46:51 PDT
Created attachment 403755 [details]
test content (position:fixed element on other layer)

(In reply to Fujii Hironori from comment #10)
> https://www.yomiuri.co.jp/
> It still exhibits the unnecessary repainting issue for position:fixed
> elements.

If a position:fixed element is placed on other layer, it has own layer even if setAcceleratedCompositingForFixedPositionEnabled is off.
Comment 12 Fujii Hironori 2020-07-07 23:06:52 PDT
Created attachment 403761 [details]
WIP patch

If acceleratedCompositingForFixedPositionEnabled is on, it's simple because all position:fixed elements are compositing.
However, if acceleratedCompositingForFixedPositionEnabled is off, only some position:fixed elements are compositing. It makes things complicated.

This WIP patch works fine for WinCairo WK2 because its acceleratedCompositingForFixedPositionEnabled is on.
But, WinCairo WK1 still exhibits the issue for Comment 11 test case.