Bug 214035 - [GTK][WinCairo] position:fixed layers are unnecessarily repainted by scrolling main content layer
Summary: [GTK][WinCairo] position:fixed layers are unnecessarily repainted by scrollin...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-07 00:55 PDT by Fujii Hironori
Modified: 2020-07-07 23:06 PDT (History)
13 users (show)

See Also:


Attachments
test content (204 bytes, text/html)
2020-07-07 00:55 PDT, Fujii Hironori
no flags Details
[Screenshot] GTK port (17.82 KB, image/png)
2020-07-07 01:00 PDT, Fujii Hironori
no flags Details
Patch (2.75 KB, patch)
2020-07-07 18:45 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
test content 2 (247 bytes, text/html)
2020-07-07 19:42 PDT, Fujii Hironori
no flags Details
test content (position:fixed element on other layer) (535 bytes, text/html)
2020-07-07 20:46 PDT, Fujii Hironori
no flags Details
WIP patch (917 bytes, patch)
2020-07-07 23:06 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.