Top corners of Firefox windows have weird black protrusions from the rounded edges

Created attachment 9027623
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.

The same thing happens on linux also.

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

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

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

Created attachment 9121264
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.

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

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

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?

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.

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

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.

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.

*** Bug 1706752 has been marked as a duplicate of this bug. ***

(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.

Created attachment 9246287
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 <email address hidden>

Created attachment 9246288
Bug 1509931 - Expose titlebar radius as a chrome-only CSS environment variable. r=stransky

Mostly plumbing.

Depends on D128679

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

Depends on D128680

Created attachment 9246290
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

Created attachment 9246291
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

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)
> Created attachment 9246290
> 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).

(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?

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

Pushed by <email address hidden>:
https://hg.mozilla.org/integration/autoland/rev/81ca2d28c253
Simplify titlebar buttons CSS. r=desktop-theme-reviewers,dao

Created attachment 9246314
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.

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.

(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](https://bug1479135.bmoattachments.org/attachment.cgi?id=8995686) (=fat black borders).
The autoscroll icon was a [black square with a white circle](https://bug1649246.bmoattachments.org/attachment.cgi?id=9160184) in it.

[XShapeCombineMask](https://searchfox.org/mozilla-central/search?q=XShapeCombineMask&path=) 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](https://searchfox.org/mozilla-central/rev/37373cdeed20695af1ff1fd090f0736d9bf15e65/widget/gtk/nsWindow.cpp#6729,6793) 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](https://searchfox.org/mozilla-central/rev/37373cdeed20695af1ff1fd090f0736d9bf15e65/widget/gtk/nsWindow.cpp#9084) 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.

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

(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).

Created attachment 9246335
titlebar CSS rounded borders properties

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.

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](https://gitlab.gnome.org/GNOME/gtk/-/blob/gtk-3-20/gtk/gtkcssshorthandpropertyimpl.c#L961-977). 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?

(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.

Okay, fair enough.

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

Pushed by <email address hidden>:
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

Pushed by <email address hidden>:
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

Pushed by <email address hidden>:
https://hg.mozilla.org/integration/autoland/rev/2be88317a6c0
Fix rebase mistake.

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

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

Emilio, I tested the patches and it looks great!

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&, b...

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

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

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.

(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.

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

*** Bug 1673079 has been marked as a duplicate of this bug. ***

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](https://github.com/yilozt/mutter-rounded) ([RPMs](https://gregor160300.stackstorage.com/s/I4YFXu82ay6mNE0C)) 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](https://i.ibb.co/vw87ST1/Screenshot-from-2021-11-18-12-30-25.png)
[Wayland screenshot](https://i.ibb.co/H7RH0tg/Screenshot-from-2021-11-18-12-30-55.png)
[Other apps screenshot](https://i.ibb.co/R3Xcn4G/Screenshot-from-2021-11-18-12-31-39.png)

Pushed by <email address hidden>:
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

(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.

(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.

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

== 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](https://firefox-source-docs.mozilla.org/testing/perfdocs/talos.html#tresize) | linux1804-64-shippable-qr | e10s stylo webrender | 18.35 -> 17.39 |
| 5% | [tresize](https://firefox-source-docs.mozilla.org/testing/perfdocs/talos.html#tresize) | linux1804-64-shippable-qr | e10s stylo webrender-sw | 17.60 -> 16.79 |
| 5% | [tresize](https://firefox-source-docs.mozilla.org/testing/perfdocs/talos.html#tresize) | linux1804-64-shippable-qr | e10s stylo webrender-sw | 17.57 -> 16.77 |
| 4% | [tresize](https://firefox-source-docs.mozilla.org/testing/perfdocs/talos.html#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

Created attachment 9254735
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!

Bug 1745419 will fix that.

Thanks! Then I will set the flags accordingly.

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?