Addons overlays not painted on webrender
Categories
(Core :: Graphics: WebRender, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox81 | --- | unaffected |
firefox82 | --- | unaffected |
firefox83 | --- | disabled |
firefox84 | --- | fixed |
People
(Reporter: kubrick, Assigned: sotaro)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: nightly-community, regression)
Attachments
(5 files, 1 obsolete file)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta-
|
Details |
Since rev 604e2561311f2aa2ba70fbccdfb54127506ad8b5, with webrender enabled, on wayland (Gnome Arch), addon windows/overlays don't get painted and remain white.
Reporter | ||
Comment 1•4 years ago
|
||
reverting D92445 and D93855 fixes the issue.
Comment 2•4 years ago
|
||
Gnome Wayland, Debian Testing, Intel/Mesa
Confirmed. Thanks for the report!
$ MOZ_ENABLE_WAYLAND=1 mozregression --launch 2020-10-19 --pref gfx.webrender.all:true -a https://addons.mozilla.org/firefox/addon/ublock-origin/
(I also noticed that with MOZ_X11_EGL=1 and GLX, uBlock panel content is often shown with a delay of 1.5 seconds, sometimes it's instant. IIRC this occurs with Xwayland, but not with X11.)
https://hg.mozilla.org/mozilla-central/shortlog/604e2561311f2aa2ba70fbccdfb54127506ad8b5
last good (panel content always visible):
$ MOZ_ENABLE_WAYLAND=1 mozregression --repo autoland --launch b5017b0a0b5d10f00957b15fa9081d3f755dad76 --pref gfx.webrender.all:true -a https://addons.mozilla.org/firefox/addon/ublock-origin/
first bad (panel content is intermittently white/empty or shown. If its white/empty, hovering does not help.):
$ MOZ_ENABLE_WAYLAND=1 mozregression --repo autoland --launch 604e2561311f2aa2ba70fbccdfb54127506ad8b5 --pref gfx.webrender.all:true -a https://addons.mozilla.org/firefox/addon/ublock-origin/
Regressed by:
604e2561311f2aa2ba70fbccdfb54127506ad8b5 Sean Feng - Bug 1669239 - Call Tick to paint in PuppetWidget::Paint instead of using WillPaintWindow r=emilio,mattwoodrow
Updated•4 years ago
|
Updated•4 years ago
|
Comment 4•4 years ago
|
||
Can you confirm this only happens with uBlock? What I see is that uBlock somehow has an opacity: 0
rule applying to the <body>
.
Reporter | ||
Comment 5•4 years ago
|
||
No it happens with all extensions (tested Bitwarden, µBlock, Feedbro, Video Download Helper, etc.)
Comment 6•4 years ago
|
||
Ok, so I think I found out what's going on, but it's a bit sad and I think we just found a pre-existing bug that was getting papered over.
So when we're on WR+Wayland and open a popup like uBlock's (or others for that matter), we hit this chunk of code, which causes the first frame to fail to draw.
Now the main issue is that before the regressing patch Invalidate
ended up doing a forceful transaction eventually, but now we just keep waiting for the reply from the compositor (which will never arrive) and keep early-returning here for eternity...
So I suspect this is a pre-existing bug, but I'm not familiar enough with the WebRender compositor code to know what the right solution is.
Should the transaction that the refresh driver is waiting for be acknowledged even though it failed? (that seems like the right fix to me).
Comment 7•4 years ago
|
||
Not for landing, probably...
Comment 8•4 years ago
|
||
Sotaro, I'm told that you know the relevant code much better, can you take a look at comment 6 and maybe say what's supposed to happen when BeginFrame fails?
We also hit this line of code, which seems problematic.
I thought a patch like this would fix it and be the correct fix per that:
diff --git a/gfx/webrender_bindings/RenderThread.cpp b/gfx/webrender_bindings/RenderThread.cpp
index 7e3922054737..92fb1c4b1248 100644
--- a/gfx/webrender_bindings/RenderThread.cpp
+++ b/gfx/webrender_bindings/RenderThread.cpp
@@ -518,7 +518,7 @@ void RenderThread::UpdateAndRender(
renderer->WaitForGPU();
}
- if (!aRender) {
+ if (!aRender || !latestFrameId.IsValid()) {
// Update frame id for NotifyPipelinesUpdated() when rendering does not
// happen.
latestFrameId = renderer->UpdateFrameId();
But it doesn't seem to be (it fixes the assertion failure though). Any advice? Alternatively we could possibly land my wallpaper, but I'd prefer not having to.
Assignee | ||
Comment 9•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)
Sotaro, I'm told that you know the relevant code much better, can you take a look at comment 6 and maybe say what's supposed to happen when BeginFrame fails?
I could reproduce the problem. I am going to look into it.
Assignee | ||
Comment 10•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)
Ok, so I think I found out what's going on, but it's a bit sad and I think we just found a pre-existing bug that was getting papered over.
So when we're on WR+Wayland and open a popup like uBlock's (or others for that matter), we hit this chunk of code, which causes the first frame to fail to draw.
It seems like a different problem. I created Bug 1672565.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
It seems that PuppetWidget's WebRenderLayerManager of the add-on seemed not do rendering. WebRenderLayerManager::EndTransactionWithoutLayer() was not called when the problem happened.
Assignee | ||
Comment 12•4 years ago
|
||
When I re-added WillPaintWindow(). The problem was addressed for me. Then it seems not like a graphics problem.
Comment 13•4 years ago
|
||
How not? WillPaintWindow effectively wallpapers it in the same way as comment 7, but that just means that the content process got stuck waiting for a transaction to finish that didn't finish.
Assignee | ||
Comment 14•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #13)
How not?
Sorry, it seems to related to graphics area.
Assignee | ||
Comment 15•4 years ago
|
||
The patch seems to address the problem, though the initial pause is necessary for wayland egl.
Then the initial pause seems to trigger the problem.
Assignee | ||
Comment 16•4 years ago
|
||
This patch also addressed the problem for me.
Assignee | ||
Comment 18•4 years ago
|
||
When the problem happened, pop-up widget's content process's WebRenderBridgeParent(non-root) was attached to different CompositorBridgeParent. Then the WebRenderBridgeParent was moved to correct CompositorBridgeParent by CompositorBridgeParent::RecvAdoptChild() call. It triggered NotifyPipelineRendered().
In this case, SendDidComposite() have to be called, since WebRenderBridgeParent(non-root) is moving to different WebRender instance. But it is not called when CompositorBridgeParent. is paused.
Assignee | ||
Comment 19•4 years ago
|
||
Updated•4 years ago
|
Reporter | ||
Comment 20•4 years ago
|
||
D94552 fixes the issue for me, but then hovering on links doesn't trigger events any more (no change of mouse pointer, etc.)
Assignee | ||
Comment 21•4 years ago
|
||
(In reply to Francois Guerraz from comment #20)
D94552 fixes the issue for me, but then hovering on links doesn't trigger events any more (no change of mouse pointer, etc.)
Thank you for checking! Can you create a bug for it?
Comment 22•4 years ago
|
||
Comment 23•4 years ago
|
||
bugherder |
Comment 24•4 years ago
|
||
Version 83 is affected by this issue. Are there any plans to uplift the fix to beta?
Comment 25•4 years ago
|
||
(In reply to Danilo from comment #24)
Version 83 is affected by this issue. Are there any plans to uplift the fix to beta?
Sotaro may be able to assess how risky is that better than me.
Assignee | ||
Comment 26•4 years ago
|
||
Comment on attachment 9183382 [details]
Bug 1672139 - Remove pause handlig from CompositorBridgeParent::NotifyPipelineRendered()
The change only to
Beta/Release Uplift Approval Request
- User impact if declined: Addon popup might becomes white with Linux EGL WebRender.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: none
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The change affects only to WebRender on Android and EGL WebRender on linux. And on android, such use case seems not happen. It changed as to send DidComposite even when WebRender is pause state and CompositorBridgeParent::RecvAdoptChild() called. It does not happen on normal use case. And send DidComposite at correct timing seems a low risk.
- String changes made/needed: none
Comment 27•4 years ago
|
||
Comment on attachment 9183382 [details]
Bug 1672139 - Remove pause handlig from CompositorBridgeParent::NotifyPipelineRendered()
Webrender is disabled by default on Linux for Mozilla builds on the release channel, I think this can ride the 84 train, thanks.
Updated•4 years ago
|
Comment 29•4 years ago
|
||
It would be great if this could make it to ff83. Firefox is the only usable native Wayland browser.
Comment 32•4 years ago
|
||
As a reminder, this is the configuration everyone is using to get VAAPI hardware video decoding. This broke WebRender for all Wayland users.
Comment 35•4 years ago
|
||
Uplifting this would have been a lot easier than dealing with those duplicates and bug reports on reddit etc.
Updated•3 years ago
|
Description
•