Closed Bug 1578464 Opened 5 months ago Closed 4 months ago

Wayland backend does not set opaque region

Categories

(Core :: Widget: Gtk, defect, P3)

Desktop
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: heftig, Assigned: stransky)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

The Wayland backend never uses wl_surface's set_opaque_region to set the opaque region of a window.

This means the compositor has to assume any part of the window can be transparent, hurting performance especially on iGPUs.

To verify: WAYLAND_DEBUG=1 firefox |& grep opaque

You can simply test the performance difference when you set mozilla.widget.use-argb-visuals to false at about config. I bet you won't see any difference as for instance mutter compositor uploads given wl_surface as EGL texture and uses EGL to composite/render the result.

Well, the point here is to give Mutter a hint that it doesn't need to render what is behind FF. So if there's a very demanding / inefficient client behind FF, maybe doing software painting and using shm-buffers, that client will continue to receive frame callbacks and drive cpu consumption. FF (and every other client) really should set the opaque region whenever possible, being a good wayland citizen.

To make the scenario even more clear: if all clients wouldn't bother to set the opaque region, mutter would need to repaint all of them, even if 5 others are above them.

(In reply to Martin Stránský [:stransky] from comment #1)

You can simply test the performance difference when you set mozilla.widget.use-argb-visuals to false at about config

That setting doesn't seem to have any effect. All buffers created are still using WL_SHM_FORMAT_ARGB8888 (and DRM_FORMAT_ARGB8888 in case acceleration or WebRender is enabled) and no opaque regions are set.

(In reply to Martin Stránský [:stransky] from comment #1)

you won't see any difference as for instance mutter compositor uploads given wl_surface as EGL texture and uses EGL to composite

How does EGL change things? It doesn't magically reduce overdraw.

To be clear here - I'm not against the idea to set to opaque_region to wl_surface, I just say I doubt it has any effect on mutter and perhaps other compositors (maybe sway can benefit from it when titlebar / argb visual is disabled). We may also use WL_SHM_FORMAT_XRGB8888 in case argb visual is disabled.

We may need to implement UpdateOpaqueRegion for mozcontainer (where it will be applied to wl_subsurface) and then we need to call it from nsWindow::UpdateOpaqueRegion().

(In reply to Martin Stránský [:stransky] from comment #4)

To be clear here - I'm not against the idea to set to opaque_region to wl_surface, I just say I doubt it has any effect on mutter and perhaps other compositors (maybe sway can benefit from it when titlebar / argb visual is disabled). We may also use WL_SHM_FORMAT_XRGB8888 in case argb visual is disabled.

It does have a effect and Mutter devs care about it, see recent or unfinished work on this:
https://gitlab.gnome.org/GNOME/mutter/merge_requests/698
https://gitlab.gnome.org/GNOME/mutter/merge_requests/692
https://gitlab.gnome.org/GNOME/mutter/commits/wip/carlosg/clip-regions

It does not have an effect on FF if it's in focus. But clients covered by FF will stop getting frame-callbacks, which can make a significant difference in client/compositor work.

(In reply to Martin Stránský [:stransky] from comment #4)

maybe sway can benefit from it when titlebar / argb visual is disabled

I think you're thinking the wrong way around? Setting the opaque region is not useful when ARGB is disabled, since an XRGB format cannot specify transparency and so the entire surface is guaranteed to be opaque.

The opaque region hints to the compositor which parts cannot contain transparent pixels, so it can use Z-buffering or the reverse painter's algorithm to avoid painting behind the region. Without this information, the entire background behind the window has to be painted before the window can be composited. Overdraw like this is very costly when the screen has many pixels and the GPU's memory is slow (e.g. UHD and an iGPU attached to system RAM).

Assignee: nobody → stransky
Priority: -- → P3
Keywords: checkin-needed

As far as I understand this patch makes sure that an opaque region set on the window is applied to the container's surface as well. In this case I would expect that the unpatched code sends one Wayland request for set_opaque_region (triggered by gdk_window_set_opaque_region) and the patched code sends two requests.

However, I'm seeing zero requests, with the patch or without. So it seems UpdateOpaqueRegion isn't being called.

Maybe the problem is that the NS widget code assumes the entire window is opaque by default? GDK and Wayland default to the entire window being transparent.

BTW, I guess whatever code ends up calculating the opaque region will have to handle the window corners (see GTKWindow's subtract_corners_from_region).

(In reply to Jan Alexander Steffens [:heftig] from comment #9)

As far as I understand this patch makes sure that an opaque region set on the window is applied to the container's surface as well. In this case I would expect that the unpatched code sends one Wayland request for set_opaque_region (triggered by gdk_window_set_opaque_region) and the patched code sends two requests.

That's good point, maybe nsWindow::UpdateOpaqueRegion() is not even called on gecko toplevel window as it's not "officially" transparent from gecko POV - see Bug 1469716 and related regressions.

Also I checked that "mozilla.widget.use-argb-visuals" overrides the alpha visual settings reliably, see:

https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/widget/gtk/nsWindow.cpp#3681

unless you have HW acceleration enabled (GL or WebRender). Also note that WebRender is on by default on nightly / some gfx cards so better to check about:support.

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/de8dc8975032
[Wayland] Set opaque region to wl_surface of toplevel window, r=jhorak

Keywords: checkin-needed

(In reply to Martin Stránský [:stransky] from comment #10)

Also I checked that "mozilla.widget.use-argb-visuals" overrides the alpha visual settings reliably

Well, that setting has some effect (the windows lose the transparent corners) but it doesn't have an effect on the buffer formats chosen, see:

WAYLAND_DEBUG=1 firefox |& grep 'new id wl_buffer'

Buffers created through wl_shm_pool.create_buffer always have format 0 (WL_SHM_FORMAT_ARGB8888).

And if either the DMABuf backend, acceleration or WebRender is enabled:
Buffers created through zwp_linux_buffer_params_v1.create_immed always have format 875713089 (DRM_FORMAT_ARGB8888).

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Not resolved yet.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

(In reply to Jan Alexander Steffens [:heftig] from comment #12)

(In reply to Martin Stránský [:stransky] from comment #10)

Also I checked that "mozilla.widget.use-argb-visuals" overrides the alpha visual settings reliably

Well, that setting has some effect (the windows lose the transparent corners)

That means the opaque region works as expected and alpha blending is not performed.

but it doesn't have an effect on the buffer formats chosen, see:

WAYLAND_DEBUG=1 firefox |& grep 'new id wl_buffer'

Buffers created through wl_shm_pool.create_buffer always have format 0 (WL_SHM_FORMAT_ARGB8888).

And if either the DMABuf backend, acceleration or WebRender is enabled:
Buffers created through zwp_linux_buffer_params_v1.create_immed always have format 875713089 (DRM_FORMAT_ARGB8888).

Yes, as the buffer format is hardcoded here:
https://searchfox.org/mozilla-central/source/widget/gtk/WindowSurfaceWayland.cpp#327

But the buffer format is not important here, important is how the final buffer (textute in mutter case) is applied. When there's no transparency the alpha channel is discarded.

What we need now is to add the transparent corners when system titlebar is disabled.

Flags: needinfo?(stransky)

(In reply to Martin Stránský [:stransky] from comment #15)

That means the opaque region works as expected and alpha blending is not performed.

But the problem is this information (the entire surface being opaque) isn't being conveyed to mutter, so it still needs to do blending.

To do that, either the buffer format needs to be alpha-less (XRGB8888) or an opaque region covering the entire surface needs to be set.

(In reply to Jan Alexander Steffens [:heftig] from comment #16)

(In reply to Martin Stránský [:stransky] from comment #15)

That means the opaque region works as expected and alpha blending is not performed.

But the problem is this information (the entire surface being opaque) isn't being conveyed to mutter, so it still needs to do blending.

Yes and it won't be as we want transparent corners so we can't have whole surface opaque.

To do that, either the buffer format needs to be alpha-less (XRGB8888) or an opaque region covering the entire surface needs to be set.

IIRC. you are asking to disable compositing for Firefox/Wayland entirely. But that's completely different bug so please file a new one for it.

(In reply to Martin Stránský [:stransky] from comment #17)

(In reply to Jan Alexander Steffens [:heftig] from comment #16)

(In reply to Martin Stránský [:stransky] from comment #15)

That means the opaque region works as expected and alpha blending is not performed.

But the problem is this information (the entire surface being opaque) isn't being conveyed to mutter, so it still needs to do blending.

Yes and it won't be as we want transparent corners so we can't have whole surface opaque.

To do that, either the buffer format needs to be alpha-less (XRGB8888) or an opaque region covering the entire surface needs to be set.

IIRC. you are asking to disable compositing for Firefox/Wayland entirely. But that's completely different bug so please file a new one for it.

No, this was just in relation to your case of mozilla.widget.use-argb-visuals = false but this doesn't interest me at all. It's just puzzling that it doesn't actually use alpha-less "visuals", i.e. buffer formats when disabled.

The normal case still needs the entire surface to be set as opaque, minus the corners, which remain transparent. Right now the entire surface is transparent.

(In reply to Jan Alexander Steffens [:heftig] from comment #18)

No, this was just in relation to your case of mozilla.widget.use-argb-visuals = false but this doesn't interest me at all. It's just puzzling that it doesn't actually use alpha-less "visuals", i.e. buffer formats when disabled.

Visual means how the surface is displayed. It's not related to how is stored internally - the alpha channel is discarded when RGB visual is used.
We can't use RGB surface format for regular windows as we want the transparent corners. That may be changed when mozilla.widget.use-argb-visuals=false and there isn't any performance penalty. That may be used for PIP windows for instance which are not transparent.

The normal case still needs the entire surface to be set as opaque, minus the corners, which remain transparent.

Corners subtraction is Bug 1584932.

Right now the entire surface is transparent.

nsWindow::UpdateOpaqueRegion() it's called on toplevel window which is another bug.

Flags: needinfo?(stransky)

Site node for testing on Gnome Shell / Mutter: currently Mutter ignores the opaque region in many cases if the display is scaled. So this should get tested on 100%.

(In reply to robert.mader from comment #20)

Site node for testing on Gnome Shell / Mutter: currently Mutter ignores the opaque region in many cases if the display is scaled. So this should get tested on 100%.

Thanks Robert, I do see that. Is there any mutter report opened so I can link it? Thanks.

Flags: needinfo?(robert.mader)
Duplicate of this bug: 1584932

There's https://gitlab.gnome.org/GNOME/mutter/issues/694 and a pending MR https://gitlab.gnome.org/GNOME/mutter/merge_requests/712. I don't think we will land it before 3.36, though.

Flags: needinfo?(robert.mader)

:stransky : please do not skip setting the opaque region on scaling! It's not a problem to do it and we will fix it in Mutter.

(In reply to robert.mader from comment #25)

:stransky : please do not skip setting the opaque region on scaling! It's not a problem to do it and we will fix it in Mutter.

I have to - right now it produces bold (~50 pixel width) black rectangle round Firefox main window (Fedora 30 / mutter-3.32.2-3.fc30.x86_64). It can be removed when new mutter hit distros.

(In reply to Martin Stránský [:stransky] from comment #26)

I have to - right now it produces bold (~50 pixel width) black rectangle round Firefox main window (Fedora 30 / mutter-3.32.2-3.fc30.x86_64). It can be removed when new mutter hit distros.

This sounds like the window's shadow is being treated as opaque. Are you sure this is a mutter bug?

(In reply to Jan Alexander Steffens [:heftig] from comment #27)

(In reply to Martin Stránský [:stransky] from comment #26)

I have to - right now it produces bold (~50 pixel width) black rectangle round Firefox main window (Fedora 30 / mutter-3.32.2-3.fc30.x86_64). It can be removed when new mutter hit distros.

This sounds like the window's shadow is being treated as opaque. Are you sure this is a mutter bug?

I can check with the fixed mutter version, from my testing the difference is just the screen scale change.

(In reply to Martin Stránský [:stransky] from comment #28)

(In reply to Jan Alexander Steffens [:heftig] from comment #27)

(In reply to Martin Stránský [:stransky] from comment #26)

I have to - right now it produces bold (~50 pixel width) black rectangle round Firefox main window (Fedora 30 / mutter-3.32.2-3.fc30.x86_64). It can be removed when new mutter hit distros.

This sounds like the window's shadow is being treated as opaque. Are you sure this is a mutter bug?

I can check with the fixed mutter version, from my testing the difference is just the screen scale change.

This should not happen. The bug in Mutter I was talking about is that it ignores the opaque region - thus it skips the optimization, making it a bit slower. It does not produce any glitches - that is a FF bug (or another, yet unknown Mutter bug). I would like to point out that GTK3 apps do set the opaque region just fine, so I'd guess it's in FF.

Also a small update, because I was wrong previously: as of Mutter 3.34 the opaque region is actually respected when the experimental fractional scaling is active. It still does not work with the default/non-fractional scaling, but the fix for that is much easier now, so I updated https://gitlab.gnome.org/GNOME/mutter/merge_requests/712 to make it work. So this might still land in 3.34.x.

(In reply to robert.mader from comment #29)

This should not happen. The bug in Mutter I was talking about is that it ignores the opaque region - thus it skips the optimization, making it a bit slower. It does not produce any glitches - that is a FF bug (or another, yet unknown Mutter bug). I would like to point out that GTK3 apps do set the opaque region just fine, so I'd guess it's in FF.

I'll try to create a simple testcase for that.

(In reply to robert.mader from comment #29)

This should not happen. The bug in Mutter I was talking about is that it ignores the opaque region - thus it skips the optimization, making it a bit slower. It does not produce any glitches - that is a FF bug (or another, yet unknown Mutter bug). I would like to point out that GTK3 apps do set the opaque region just fine, so I'd guess it's in FF.

Seems to be fixed now by the latest patch, Thanks.

Keywords: checkin-needed

Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e8087547a22
[Wayland] Set opaque region for toplevel window, r=jhorak

Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 4 months ago4 months ago
Resolution: --- → FIXED
Blocks: 1591488
Blocks: 1591489
You need to log in before you can comment on or make changes to this bug.