[Webrender][X11] Opaque region does not get set / gets reset
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox81 | --- | fixed |
People
(Reporter: rmader, Assigned: rmader)
References
(Blocks 1 open bug)
Details
Attachments
(7 files)
Summary: when using WR we always use textures with alpha channel. Thus we need to set an opaque region to let the OS compositor know that it can skip painting everything behind the FF window (minus rounded corners). This is already implemented but does not work properly on X11 atm.
User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:73.0) Gecko/20100101 Firefox/73.0
Steps to reproduce:
Working on culling in Mutter (Gnome Shell), testing different applications.
Exact steps:
- use Mutter >= 3.35.91
- use the Wayland session
- start
weston-simple-damage --verbose
in a terminal (part of Weston demo clients) - hide the demo app behind other applications
- if the application sets an opaque region or doesn't have an alpha channel,
weston-simple-damage
will get culled out, therefore not receive frame callbacks any more -> output in terminal stops
Actual results:
demo app gets culled out behind Firefox on Wayland (basic, ogl, Webrender) and X11 (basic, ogl)
demo app does not get culled out behind Firefox X11 Webrender
Expected results:
Should get culled out, too.
I did some testing quick testing in nightly and apparently there's nothing wrong in the part in nsWindow.cpp
(it sets the region and does not unset it). On resize, Mutter first receives a valid region and then NULL
again. Now I'm stuck.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
While looking into bug 1648872 I stumbled over this again and it's still valid. Fixing this should allow us to remove the duplicated code in UpdateTopLevelOpaqueRegionWayland
, getting rid of one occurrence of gdk_window_invalidate_rect
which might be the cause for bug 1648872 (this is pure guessing atm).
Assignee | ||
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Without going through the steps in comment 0 (just running a local build on x11 with webrender enabled), I see gdk_window_set_opaque_region being called. Is this specific to a certain configuration?
Assignee | ||
Comment 3•4 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #2)
Without going through the steps in comment 0 (just running a local build on x11 with webrender enabled), I see gdk_window_set_opaque_region being called. Is this specific to a certain configuration?
Yes, it's getting called but it does not have the desired effect - I don't think it's specific to a configuration, it certainly applies to default settings (I also tested with and without title bar). AFAICS the problem is that GTK also calls gdk_window_set_opaque_region
internally, directly unsetting the region again - and I think the problem is that we don't call it the way it's intended by GTK (we don't seem to follow the docs).
I'm planning to look into this again until the weekend, just so you know.
P.S.: I also assume this affects all DEs (as I think it's a wrong handling of GTK), but didn't test yet
Assignee | ||
Comment 4•4 years ago
|
||
In order to make it easier to validate opaque region usage I created https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1372, which will allow us to get an easy to use visual feedback (if accepted of course).
I was actually wrong about the layers/opengl render - it's also affected. The default basic/software render work fine.
Assignee | ||
Comment 5•4 years ago
|
||
Assignee | ||
Comment 6•4 years ago
|
||
Assignee | ||
Comment 7•4 years ago
|
||
Assignee | ||
Comment 8•4 years ago
|
||
Looking at the code this is roughly how it should look like: cut out corners in the top-left and top-right (basic mode uses Xshape - we don't want that when using WR :) ). Just like most GTK3 applications.
Comment 9•4 years ago
|
||
(Is bug 1572625 comment 13 relevant? Would disabling layers.gpu-process.enabled help on X11?)
Comment 10•4 years ago
|
||
See darkspirit's question. We intend to disable the GPU process by default, and I'm curious if this is another sync issue we are seeing....seems likely given the others issues we have seen with i3, which you have managed to see with mutter/gnome. You can disable the GPU process by flipping the pref layers.gpu-process.enabled to false.
Can you attach your about:support? Mostly I want to know if you are using Wayland, or XWayland.
Assignee | ||
Comment 11•4 years ago
|
||
(In reply to Andrew Osmond [:aosmond] from comment #10)
See darkspirit's question. We intend to disable the GPU process by default, and I'm curious if this is another sync issue we are seeing....seems likely given the others issues we have seen with i3, which you have managed to see with mutter/gnome. You can disable the GPU process by flipping the pref layers.gpu-process.enabled to false.
Can you attach your about:support? Mostly I want to know if you are using Wayland, or XWayland.
Haven't pulled the patch adding XWayland to about:support, but I do run on Xwayland. Just tried with layers.gpu-process.enabled=false
and it doesn't change anything - but as I said before, I'm quite sure it's GTK related.
FTR: I'm trying to debug this right now, just finalizing a patch that is I think is good to have beforehand, https://phabricator.services.mozilla.com/D82804
Assignee | ||
Comment 12•4 years ago
|
||
Cleans up the code a bit to make actually used fallback code easier to spot
and update the required GTK version so deprecation warnings are more accurate.
Also make gdk_window_set_opaque_region
always available - we can now assume
it to be present in all supported versions.
Updated•4 years ago
|
Assignee | ||
Comment 13•4 years ago
|
||
- always update the opaque region if the toplevel window allocation
changes - this fixes opaque regions on X11 with WR and OGL renderers,
as GTK resets the opaque region in this case. The previous solution
was modeled similar to what's done on Windows - we have to follow GTK
though. - use
gdk_window_set_opaque_region
also for Wayland - this fixes
occasional glitches cases where the opaque region was not properly
updated. Now we let GTK handle that. - remove a failed attempt to work around bug 1615098 from
MozContainerWayland - repurpose
widget.wayland.use-opaque-region
for subsurface opaque
regions and disable it by default. We want to enable it again eventually
and not all Wayland compositors suffer from bug 1615098 - some cleanups - some optimizations should not be needed anymore now
Depends on D82804
Depends on D82804
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 14•4 years ago
|
||
Assignee | ||
Comment 15•4 years ago
|
||
All green now. Sorry for the noise, I just like the opaque region color feature :)
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 16•4 years ago
|
||
Note: gdk_window_set_opaque_region
on X11 only affects EWMH capable window managers - simple WMs / compositors will likely ignore it. See https://developer.gnome.org/wm-spec/#idm45384480021120
Assignee | ||
Comment 17•4 years ago
|
||
Another note: this affects full screen usage. AFAIK firefox requests compositor bypass in that case (at least for video playback etc.), increasing performance at the risk of tearing. Before this patch, I assume most compositors just ignored that request, due to the texture being potentially transparent. So this patch might turn on fullscreen unredirect for some users, which again might introduce tearing for them, if something goes wrong with VSync. Didn't test that yet, but in case reports appear: it's expected and would need to be solved differently.
Assignee | ||
Updated•4 years ago
|
Comment 18•4 years ago
|
||
Comment 19•4 years ago
|
||
bugherder |
Assignee | ||
Comment 20•4 years ago
|
||
Martin, I think I have a somewhat clean an liable solution now, but I'm struggling with the title bar case. Are you aware of any reliable way to get its size? Unfortunately it looks like we need to set the opaque region for it as well.
Alternatively we can just ignore that case for now.
Comment 21•4 years ago
|
||
(In reply to Robert Mader [:rmader] from comment #20)
Martin, I think I have a somewhat clean an liable solution now, but I'm struggling with the title bar case. Are you aware of any reliable way to get its size? Unfortunately it looks like we need to set the opaque region for it as well.
Alternatively we can just ignore that case for now.
Which titlebar do you mean?
Comment 22•4 years ago
|
||
Is that what do you mean?
I tested the latest version with MOZ_GTK_TITLEBAR_DECORATION=client and MOZ_GTK_TITLEBAR_DECORATION=system on X11, as well as Wayland. I all cases things should now work nicely if no classic title bar is used. If it is used, it will not be set as opaque in combination with WR or layers - which is still better then the current status, so I'd prefer to get this reviewed and pushed already.
Assignee | ||
Comment 23•4 years ago
|
||
(In reply to Martin Stránský [:stransky] from comment #22)
Is that what do you mean?
Yes, the classic title bar, !mDrawInTitlebar
. As far as I can see the calculation for it is different each time for MOZ_GTK_TITLEBAR_DECORATION=client
on X11, MOZ_GTK_TITLEBAR_DECORATION=system
on X11 and Wayland. If I understand things correctly, normally GTK would handle that for us - but the opaque region gets unset for the whole window / surface on allocation change. So we'd need to include the title bar in our opaque region.
For me on Fedora 32 it works when just I expand the opaque region 37px to the top - but I struggle to find a way to calculate these 37px reliably in the different cases.
Comment 24•4 years ago
|
||
(In reply to Robert Mader [:rmader] from comment #23)
(In reply to Martin Stránský [:stransky] from comment #22)
Is that what do you mean?
Yes, the classic title bar,
!mDrawInTitlebar
. As far as I can see the calculation for it is different each time forMOZ_GTK_TITLEBAR_DECORATION=client
on X11,MOZ_GTK_TITLEBAR_DECORATION=system
on X11 and Wayland. If I understand things correctly, normally GTK would handle that for us - but the opaque region gets unset for the whole window / surface on allocation change. So we'd need to include the title bar in our opaque region.
Can we force Gtk somehow to re-set the opaque region after the allocation change or can we just set whole widget (mShell) opaque?
Assignee | ||
Comment 25•4 years ago
|
||
Martin, given that bug 1653612 has landed I guess we should make sure D84024 also lands in time for FF 80 - even when missing out on title bars. Depending on the scenario, opaque regions can heavily influence composition times, so especially for bandwidth constrained GPUs like weak intel iGPUs this is very important to not regress in comparison to software rendering. I'll try to update as soon as possible if you find more issues to be addressed :)
Comment 26•4 years ago
|
||
(In reply to Robert Mader [:rmader] from comment #25)
Martin, given that bug 1653612 has landed I guess we should make sure D84024 also lands in time for FF 80 - even when missing out on title bars. Depending on the scenario, opaque regions can heavily influence composition times, so especially for bandwidth constrained GPUs like weak intel iGPUs this is very important to not regress in comparison to software rendering. I'll try to update as soon as possible if you find more issues to be addressed :)
Yes I understand you. OTOH I'm bit scared of such radical changes in beta without testing at nightly as our ability to fix beta is limited. So I'd rated ship that at 81 first even with the possible performance pennality. IMHO any GPU based rendering is faster than SW compositing/rendering.
Assignee | ||
Comment 27•4 years ago
|
||
(In reply to Martin Stránský [:stransky] from comment #26)
Yes I understand you. OTOH I'm bit scared of such radical changes in beta without testing at nightly as our ability to fix beta is limited.
So I'd rated ship that at 81 first even with the possible performance pennality.
Agreed :)
IMHO any GPU based rendering is faster than SW compositing/rendering.
Unfortunately I can guarantee you that this is not true for certain scenarios. Some users will hit bandwidth limits and therefore frame-drops because of this, especially when having multiple windows stacked on top of each other. As long as people don't do that they should be fine though.
Comment 28•4 years ago
|
||
Comment 29•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 30•4 years ago
|
||
FTR: I just accidentally verified that Mutter 3.37/3.38 does now use fullscreen unredirect for fullscreen videos with this - so just in case people start to see tearing with WR now they should disable that.
Comment 31•4 years ago
|
||
Re-enabling widget.wayland.use-opaque-region
after this patch reintroduces bug 1615098 (or similar symptoms) on Mutter 3.36.4 (latest Nightly, Wayland, WebRender).
Is this expected?
Assignee | ||
Comment 32•4 years ago
|
||
(In reply to Ivan Shapovalov from comment #31)
Re-enabling
widget.wayland.use-opaque-region
after this patch reintroduces bug 1615098 (or similar symptoms) on Mutter 3.36.4 (latest Nightly, Wayland, WebRender).Is this expected?
Yep :) I re-purposed that setting for the subsurface, as that's where things can break. That's also why the setting is switched off by default, still.
Updated•3 years ago
|
Description
•