1509931 - [Wayland][OpenGL] CSD has sharp corners (drawn over GTK rounded border/shadow frame)

Status: VERIFIED FIXED

(Core :: Graphics, defect, P2)

Description

[Val Packett]

Attachment: firefox-wayland-corner.png

(continuing from bug 1507608)

When GL compositing is used on the Wayland backend, the GL subsurface that actual Firefox content is rendered to does not have rounded corners. It overdraws the parent GTK surface that contains the border and shadow.

The Default Firefox theme actually draws the Adwaita rounded corner, but the surface is still not transparent in the corners, so there's an ugly gray corner.

Comment 1

[ApB]

The same thing happens on linux also.

https://www.enlightenment.org/ss/display.php?image=e-5c1b7b13e6d7e3.27335093.png

Comment 2

[Martin Stránský [:stransky] (ni? me)]

I'm not sure how to fix that on Wayland as it does not support XShape mask which we use on X11.

Comment 3

[Val Packett]

huh, the corners were just masked out? But who draws the non-transparent gray pixels there in the first place?

Comment 4

[Jan Alexander Steffens [:heftig]]

Attachment: Default and Light theme corners, GNOME Shell window screenshots, scale 2, on striped background

This almost works for me, and the behavior is the same whether basic layers or WR is used.

As I understand, the way CSD works here is that the window uses a GTK window style which only draws the shadow around an inner widget drawn by Firefox. Adwaita gives us a soft shadow and a 1px semitransparent outline.

The Default theme has a slight glitch where the corners are not as transparent as they should be. I think the tab bar's "headerbar" styling is trying to draw a window shadow here which gets overlaid with the shadow drawn by the window. We need to either somehow remove the shadow from the headerbar style applied to the tab bar, or somehow mask out the drawing of the window shadow so that it only draws outside the inner widget.

The Light and Dark themes have no "headerbar" styling, so the tab bar has sharp corners which clash with the rounded outline from the window's Adwaita theme.

Comment 5

[limpid.mazarine]

Hi, the issue still exists on Firefox 74.0 on Fedora (Wayland). Any idea how to fix this?

Comment 6

[felix]

I can confirm this issue still exists in Fedora 34, Firefox 89, Wayland.

Comment 7

[Robert Mader [:rmader]]

This just became even more prominent in 95 with colorways. We need a way to support this with alpha visuals as XShape is neither available on Wayland nor on X11 with HW Webrender - i.e. it's only available in legacy fallback paths. As the default theme supports this (even though with a grey corner), I suppose this is not really a graphics but rather a theme issue.

Emilio, do you happen to know how the default theme gets its corners and what would need to be done to make it work for other themes as well?

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Wayland case seems to be a bit special as it produces gray corner even with Gtk theme (which works OK on X11/Alpha).
We may want to draw alpha over corners when custom theme is used and main window is not maximized or tiled.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

I'm not quite familiar with how this works, I think Martin is the right person to ask about it.

I can dig if you want but I'd guess that this kind of stuff (the :not(:-moz-lwtheme) bits and such) is related: https://searchfox.org/mozilla-central/rev/9bc5dcea99c59dc18eae0de7064131aa20cfbb66/browser/themes/linux/browser.css#389-399

Comment 10

[Robert Mader [:rmader]]

Thanks emilio, that's a good hint. The comment says

It may cause performance issues so let's put it under a preference and enable it for desktop environment which do that by default.

This is clearly outdated and from a pre Webrender time.

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Emilio, you're absolutely right :) There's original Bug 1408360 for it.

For custom themes we need to apply alpha component only and keep original color from theme (i.e. paint over alpha from theme).

Emilio, how is the best way how to do it? We can introduce a new appearance (-moz-window-titlebar-alpha for instance) which contains only alpha component from titlebar and paint it over theme color or use existing one (-moz-window-titlebar) but I'm not very experienced in js/css styling so I'll appreciate any help here.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #11)

Emilio, how is the best way how to do it? We can introduce a new appearance (-moz-window-titlebar-alpha for instance) which contains only alpha component from titlebar and paint it over theme color or use existing one (-moz-window-titlebar) but I'm not very experienced in js/css styling so I'll appreciate any help here.

I don't think that's super-great, I think I have an idea which should be relatively simple, stealing.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1509931 - Add support for chrome-only environment variables. r=stransky

This bit is taken straight from D73454 (I reviewed it but I guess
another pair of eyes is ok, it's really straight-forward).

Co-authored-by: Nicklas Boman <[e-mail]>

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1509931 - Expose titlebar radius as a chrome-only CSS environment variable. r=stransky

Mostly plumbing.

Depends on D128679

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1509931 - Use titlebar radius on Linux and make titlebar set-up work for lightweight themes. r=stransky!,dao!

Depends on D128680

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1509931 - Remove -moz-gtk-csd-transparent-background. r=stransky

We always use alpha visual for WebRender, and appearance: none is
unnecessary (root element has no intrinsic appearance).

Depends on D128681

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1509931 - Simplify titlebar buttons CSS. r=stransky

There's no need to use the media query to set the default styles of the
buttons, we only need to hide them if appropriate.

Depends on D128682

Comment 19

[Darkspirit]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

Created attachment 9246290 [details]
Bug 1509931 - Remove -moz-gtk-csd-transparent-background. r=stransky

We always use alpha visual for WebRender

KDE with disabled compositor, i3, etc. use alpha visual, but it's not transparent.
Could -moz-gtk-csd-transparent-background be defined with gdk_screen_is_composited()
and be used to forbid outer border-radius + outer shadow of main menu/addon/etc. widgets
to remove any Xshape usage (bug 1730991 comment 6)?
According to the Chromium bugtracker, Xshape also causes performance problems (bug 1730991 comment 7).

Comment 20

[Emilio Cobos Álvarez (:emilio)]

(In reply to Darkspirit from comment #19)

KDE with disabled compositor, i3, etc. use alpha visual, but it's not transparent.

WDYM, just that the background wouldn't be transparent?

Could -moz-gtk-csd-transparent-background be defined with gdk_screen_is_composited()
and be used to forbid outer border-radius + outer shadow of main menu/addon/etc. widgets
to remove any Xshape usage (bug 1730991 comment 6)?
According to the Chromium bugtracker, Xshape also causes performance problems (bug 1730991 comment 7).

I'm not sure about this (not familiar with XShape at all).

This patch stack makes Wayland look great, but on X11/XWayland I still see the compositor shadow drawn around the crisp corner when using a lightweight theme... Do you know whether that's avoidable Martin?

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Ah, I think I know why that might be...

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81ca2d28c253
Simplify titlebar buttons CSS. r=desktop-theme-reviewers,dao

Comment 23

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1509931 - Use a more precise toolbar radius. r=stransky

This seemed possible/maybe worth doing, but let me know if you'd rather
keep the 10 hard-coded.

Comment 24

[Emilio Cobos Álvarez (:emilio)]

I fixed the wayland overdraw and the X11 artifacts, so I think this is ready to go. I've tested this stack on KWin and GNOME, both X11 and Wayland. Will test i3 and such asap.

Comment 25

[Darkspirit]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #20)
Non-composited X11 (the legacy variant of X11: i3, KDE with manually disabled compositor, etc.) does not support transparency. Everything that would usually be transparent/alpha is just black/opaque. Menus and window corners had shadows on black background (=fat black borders).
The autoscroll icon was a black square with a white circle in it.

XShapeCombineMask is implemented for software rendering only. Firefox creates a stencil (like a cookie cutter) of where transparency needs to get cut off. X11 then applies this stencil on every frame. The outer shadow of a rounded main menu, addon widget or window titlebar has transparency, so it gets cut off by X11, leaving a rounded widget without shadow.

XShapeCombineMask is not implemented for hardware rendering.
That's why SW WR is enforced for menus, addons and autoscroll icon on non-composited X11 to make use of XShapeCombineMask while the main window is still using OpenGL.

XShapeCombineMask

• caused problems on Mutter back then
• crashes the GPU process on Nvidia: bug 1730991
• causes performance problems according to the Chromium bugtracker: bug 1733094 comment 18,https://bugs.chromium.org/p/chromium/issues/detail?id=1198080

Could XShapeCombineMask be removed and instead menus and the autoscroll icon loose their border-radius (become rectangular) and loose their shadow (to avoid black background) if gdk_screen_is_composited() is false?
IIUC, that would reduce code complexity, improve stability and performance.

A rectangular autoscroll icon on non-composited X11 would be noticeable. But users choose non-composited X11 to get more performance than with composited X11. Considering this as well, it doesn't make sense to use XShapeCombineMask which contradicts their intent and causes above problems.

Comment 26

[Emilio Cobos Álvarez (:emilio)]

Well, perhaps, but that seems tangential to this bug, isn't it? We don't use XShapeCombineMask for the titlebar and this patch stack doesn't start doing that.

If we do want to do the cleanup described above, we might need a new media query which determines this (but it should definitely not have csd in the name, since it is not about csd at all). So I'd rather clean up the existing one since it's confusing at best.

Anyways, things I've tested:

• KWin (Wayland/X11/XWayland)
• GNOME (Wayland/X11/XWayland) with a variety of GTK themes
• i3
• bspwm

And in all cases this patch stack doesn't regress behavior (and improves it in the obvious cases). So I'd say comment 25 should be a separate bug?

If someone has an exotic setup and wants to give this a try before it lands, or wants to provide feedback, builds should be here eventually: https://treeherder.mozilla.org/jobs?repo=try&revision=327533608921f19068d1b8898b094fb3b3cdfc9d

Comment 27

[Darkspirit]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #26)

If we do want to do the cleanup described above, we might need a new media query which determines this (but it should definitely not have csd in the name, since it is not about csd at all). So I'd rather clean up the existing one since it's confusing at best.

And in all cases this patch stack doesn't regress behavior (and improves it in the obvious cases). So I'd say comment 25 should be a separate bug?

Yes. I only remembered that a query like -moz-gtk-csd-transparent-background might be useful to allow Xshape removal (if it is desired).

Comment 28

[Martin Stránský [:stransky] (ni? me)]

Attachment: titlebar CSS rounded borders properties

Comment 29

[Martin Stránský [:stransky] (ni? me)]

btw. It would be great to get border-radius from menu elements (Adwaita uses it) and use it in gecko too, for context menus for instance. But that may be a different bug.

Comment 30

[Emilio Cobos Álvarez (:emilio)]

So, querying the individual border-radius properties doesn't work. I get:

(firefox:17330): Gtk-WARNING **: 00:39:54.651: Style property "border-top-left-radius" is not gettable

And same for the other corners of course. However, my patch works, because of this code. Basically, querying border-radius on gtk3 returns the border-top-left-radius, which happens to work for us, yay :)

So given there's no way to access the other border corners, and that this works for our purposes, I think we should probably just roll with it, thoughts?

Comment 31

[Robert Mader [:rmader]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #30)

So given there's no way to access the other border corners, and that this works for our purposes, I think we should probably just roll with it, thoughts?

I'd agree - should work well for almost everyone.

One question I had to think of is if we want to use rounded corners at the bottom at some point - more and more GTK4 apps do that, e.g. nautilus/files. But we can also leave that for the future.

Comment 32

[Martin Stránský [:stransky] (ni? me)]

Okay, fair enough.

Comment 33

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/81ca2d28c253

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/257c4f510cfd
Add support for chrome-only environment variables. r=stransky
https://hg.mozilla.org/integration/autoland/rev/a1e159fd2136
Expose titlebar radius as a chrome-only CSS environment variable. r=stransky

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9399923df7b
Remove -moz-gtk-csd-transparent-background. r=stransky,desktop-theme-reviewers,dao
https://hg.mozilla.org/integration/autoland/rev/bcdfe3daf980
Use a more precise toolbar radius. r=stransky

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2be88317a6c0
Fix rebase mistake.

Comment 37

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/257c4f510cfd
https://hg.mozilla.org/mozilla-central/rev/a1e159fd2136

Comment 38

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/c9399923df7b
https://hg.mozilla.org/mozilla-central/rev/bcdfe3daf980
https://hg.mozilla.org/mozilla-central/rev/2be88317a6c0

Comment 39

[Martin Stránský [:stransky] (ni? me)]

Emilio, I tested the patches and it looks great!

Comment 40

[Martin Stránský [:stransky] (ni? me)]

Emilio, when running testsuite with the patch (locally) I'm getting this crash:

2:05.41 GECKO(53217) Assertion failure: sInServoTraversal || NS_IsMainThread(), at /home/komat/src/objdir-opt/dist/include/mozilla/ServoUtils.h:33
2:05.41 GECKO(53217) #01: mozilla::ServoStyleSet::IsInServoTraversal() [/home/komat/src/objdir-opt/dist/include/mozilla/ServoStyleSet.h:103]
2:05.41 GECKO(53217) #02: mozilla::Preferences::InitStaticMembers() [/home/komat/src/modules/libpref/Preferences.cpp:3464]
2:05.41 GECKO(53217) #03: nsresult mozilla::Internals::GetPrefValue<int*&>(char const*, int*&, mozilla::PrefValueKind) [/home/komat/src/modules/libpref/Preferences.cpp:4439]
2:05.41 GECKO(53217) #04: mozilla::Preferences::GetInt(char const*, int*, mozilla::PrefValueKind) [/home/komat/src/modules/libpref/Preferences.cpp:0]
2:05.41 GECKO(53217) #05: nsXPLookAndFeel::GetFloatValue(mozilla::LookAndFeel::FloatID, float&) [/home/komat/src/widget/nsXPLookAndFeel.cpp:894]
2:05.41 GECKO(53217) #06: nsWindow::GetTitlebarRadius() [/home/komat/src/widget/gtk/nsWindow.cpp:6111]
2:05.41 GECKO(53217) #07: nsWindow::GetTitlebarRect() [/home/komat/src/widget/gtk/nsWindow.cpp:6438]
2:05.41 GECKO(53217) #08: mozilla::widget::GtkCompositorWidget::GetTransparentRegion() [/home/komat/src/widget/gtk/GtkCompositorWidget.cpp:0]
2:05.41 GECKO(53217) #09: mozilla::wr::RenderCompositorSWGL::AllocateMappedBuffer(mozilla::wr::Box2D<int, mozilla::wr::DevicePixel> const*, unsigned long) [/home/komat/src/gfx/webrender_bindings/RenderCompositorSWGL.cpp:149]
2:05.41 GECKO(53217) #10: mozilla::wr::RenderCompositorSWGL::StartCompositing(mozilla::wr::ColorF, mozilla::wr::Box2D<int, mozilla::wr::DevicePixel> const*, unsigned long, mozilla::wr::Box2D<int, mozilla::wr::DevicePixel> const*, unsigned long) [/home/komat/src/gfx/webrender_bindings/RenderCompositorSWGL.cpp:185]
2:05.41 GECKO(53217) #11: <webrender::compositor::sw_compositor::SwCompositor as webrender::composite::Compositor>::start_compositing [gfx/wr/webrender/src/compositor/sw_compositor.rs:1400]
2:05.41 GECKO(53217) #12: webrender::renderer::Renderer::draw_frame [gfx/wr/webrender/src/renderer/mod.rs:4582]
2:05.41 GECKO(53217) #13: webrender::renderer::Renderer::render_impl [gfx/wr/webrender/src/renderer/mod.rs:2009]
2:05.41 GECKO(53217) #14: webrender::renderer::Renderer::render [gfx/wr/webrender/src/renderer/mod.rs:0]
2:05.41 GECKO(53217) #15: wr_renderer_render [gfx/webrender_bindings/src/bindings.rs:623]
2:05.41 GECKO(53217) #16: mozilla::wr::RendererOGL::UpdateAndRender(mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*, mozilla::wr::RendererStats*) [/home/komat/src/gfx/webrender_bindings/RendererOGL.cpp:186]
2:05.42 GECKO(53217) #17: mozilla::wr::RenderThread::UpdateAndRender(mozilla::wr::WrWindowId, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType> const&, mozilla::TimeStamp const&, bool, mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*) [/home/komat/src/gfx/webrender_bindings/RenderThread.cpp:0]
2:05.42 GECKO(53217) [Parent 53217: Main Thread]: D/Widget GetScreenBounds [7fba6021a000] 95,55 -> 1280 x 1012, unscaled 95,55 -> 1280 x 1012
2:05.42 GECKO(53217) #18: mozilla::wr::RenderThread::HandleFrameOneDoc(mozilla::wr::WrWindowId, bool) [/home/komat/src/gfx/webrender_bindings/RenderThread.cpp:355]
2:05.42 GECKO(53217) [Parent 53217: Main Thread]: D/Widget Events sent from focus in event [7fba6021a000]
2:05.42 GECKO(53217) #19: mozilla::detail::RunnableMethodImpl<mozilla::wr::RenderThread*, void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool), true, (mozilla::RunnableKind)0, mozilla::wr::WrWindowId, bool>::Run() [/home/komat/src/objdir-opt/dist/include/nsThreadUtils.h:1203]

Any idea what can be wrong?

Comment 41

[Martin Stránský [:stransky] (ni? me)]

May nsWindow::GetTitlebarRadius() be called from render thread? (this is sw-wr).

Comment 42

[Martin Stránský [:stransky] (ni? me)]

We have a technical debt here as we don't use titlebar rendering in testsuite - I'll look at it.

Comment 43

[Emilio Cobos Álvarez (:emilio)]

Yeah, so nsWindow::GetTitlebarRadius can be called off the main thread which seems unfortunate. We can fix this easily by moving the titlebar radius to a member or such.

Comment 44

[Martin Stránský [:stransky] (ni? me)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #43)

Yeah, so nsWindow::GetTitlebarRadius can be called off the main thread which seems unfortunate. We can fix this easily by moving the titlebar radius to a member or such.

I can fix that as a part of Bug 1736518.

Comment 45

[Emilio Cobos Álvarez (:emilio)]

Oh, great, let me know if I can help. Probably a static atomic integer in nsLookAndFeelGtk is slightly easier to invalidate in practice.

Comment 47

[gschram]

I have just tested the latest nightly on Fedora 35 and it works perfectly for XWayland, however, when running Wayland the bottom corners are still not rounded and the top corners are rounded but still have a weird (barely noticeable unless you know) transparent sharp corner. Note that I am running a patched version of mutter (RPMs) that enables rounded corners in every app, however, Firefox is the only app that breaks the nice uniform look that I believe GTK4/libadwaita are also going for with the rounded corners on all sides (just like Windows 11 and macOS).

I understand that this is an edge case currently (with the patched mutter) but I foresee this becoming a "problem" when in a year or two all other apps also have rounded bottom corners and Firefox is the odd one out, I think fixing this now is a good idea to prevent technical debt.

Xwayland screenshot
Wayland screenshot
Other apps screenshot

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb8c23dda78d
Use titlebar radius on Linux and make titlebar set-up work for lightweight themes. r=stransky,dao

Comment 49

[Emilio Cobos Álvarez (:emilio)]

(In reply to gschram from comment #47)

I understand that this is an edge case currently (with the patched mutter) but I foresee this becoming a "problem" when in a year or two all other apps also have rounded bottom corners and Firefox is the odd one out, I think fixing this now is a good idea to prevent technical debt.

Getting bottom corners rounded on Wayland seems worth a separate bug.

Comment 50

[gschram]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #49)

(In reply to gschram from comment #47)

I understand that this is an edge case currently (with the patched mutter) but I foresee this becoming a "problem" when in a year or two all other apps also have rounded bottom corners and Firefox is the odd one out, I think fixing this now is a good idea to prevent technical debt.

Getting bottom corners rounded on Wayland seems worth a separate bug.

I have opened a separate bug here https://bugzilla.mozilla.org/show_bug.cgi?id=1743981 I have listed it as an enhancement.

Comment 51

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/eb8c23dda78d

Comment 52

[Acasandrei Beatrice (needinfo me)]

== Change summary for alert #32637 (as of Mon, 06 Dec 2021 09:45:12 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
5%	tresize	linux1804-64-shippable-qr	e10s stylo webrender	18.35 -> 17.39
5%	tresize	linux1804-64-shippable-qr	e10s stylo webrender-sw	17.60 -> 16.79
5%	tresize	linux1804-64-shippable-qr	e10s stylo webrender-sw	17.57 -> 16.77
4%	tresize	linux1804-64-shippable-qr	e10s fission stylo webrender	21.07 -> 20.29

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32637

Comment 53

[Catalin Sasca, Desktop QA [:csasca]]

Attachment: dim corners.png

Verified on 96.0a1 and 97.0a1 that the top side is rounded and look good on Wayland session under Ubuntu 20.04. Although while getting the notification to make Firefox as default, a dim effect is active and the rounded parts have corners because of that (see attachment and zoom in). Any idea? Thank you!

Comment 54

[Emilio Cobos Álvarez (:emilio)]

Bug 1745419 will fix that.

Comment 55

[Catalin Sasca, Desktop QA [:csasca]]

Thanks! Then I will set the flags accordingly.

Comment 56

[mikolov73]

I have posted a bug #1744496 in which I explain that on elementary OS 6.0 Odin the corners are still square. It seems on elementary OS -moz-window-titlebar doesn't work. -moz-gtk-csd-titlebar-radius does work, as you can see when the dark theme is enabled. Can anyone look at this bug?