macOS vibrancy doesn't work in WebRender + OS compositor configuration
Categories
(Core :: Web Painting, enhancement, P2)
Tracking
()
People
(Reporter: mstange, Assigned: mstange)
References
Details
Attachments
(7 files, 4 obsolete files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/x-phabricator-request
|
Details |
With the current default Webrender configuration (i.e. no OS compositor integration), vibrancy works as follows:
- The window contains an opaque CALayer for the window background color.
- Then there's the CALayer from a vibrant NSVisualEffectsView. This knocks out the window background color.
- Then there's a full-window CALayer with WebRender's rendering. Within this layer, OpenGL painting does the following:
- First, it draws a full-window sized opaque window background color.
- Then it draws the web content.
- Then it draws a "clear rect" primitive, which erases the window background color drawing in the tab bar again.
- Then it draws the content of the tab bar and the rest of the chrome on top.
The "clear rect" approach is hard to make work with WR OS compositor surfaces. The semantics of the "clear rect" primitive ask for the pixels to be cleared across the entire stack of WebRender rendering, which can consist of multiple overlapping OS compositor surfaces.
It would be much simpler for WebRender if there was no "erasing". Instead of drawing a background color and then clearing it, we should just not draw that background color.
And the easiest way to achieve that is probably to change the front-end CSS so that there's no CSS background-color definition on the root element of the window document.
So I think what I'll do is the following:
- Remove the CSS background-color from the stylesheet and replace it with
-moz-appearance: dialog
. (Reverting bug 461366, which is now 11 years old.) - On macOS, treat
-moz-appearance: dialog
similarly to how-moz-appearance: -moz-win-glass
is treated on Windows 7: Make Gecko leave the background transparent, but make sure the OS window doesn't actually turn transparent. - At this point there will be nothing to erase anymore, and we can get rid of the "clear rect" display items.
Assignee | ||
Comment 1•5 years ago
|
||
This might actually be a little more complicated when a lightweight theme is selected. At the moment, lightweight themes cause us to set -moz-appearance: none; background-color: var(--lwt-accent-color);
on the root element. With those properties, we can't have any vibrancy in the window under the new model. If we could set those properties on the toolbox instead, and leave the window with -moz-appearance: dialog
, then we can still have vibrant sidebars. But maybe we don't want vibrant sidebars if a theme is selected anyway.
Assignee | ||
Comment 2•5 years ago
|
||
On macOS, the OS window always comes with an opaque background for top level
windows. This is the case even if Gecko determines the root element of the
window to be transparent: Ever since bug 1162649, nsChildView/nsCocoaWindow
ignore calls to SetTransparencyMode for top level windows and always stay opaque.
Returning true from nsChildView::WidgetPaintsBackground() lets us indicate that
we do not need an opaque backstop color to be added at the bottom of the display
list. This backstop color would interfere with vibrant -moz-appearance rendering
under the new vibrancy model.
WidgetPaintsBackground() is only called in one place, in ComputeBackstopColor():
nscolor PresShell::ComputeBackstopColor(nsView* aDisplayRoot) {
nsIWidget* widget = aDisplayRoot->GetWidget();
if (widget && (widget->GetTransparencyMode() != eTransparencyOpaque ||
widget->WidgetPaintsBackground())) {
// Within a transparent widget, so the backstop color must be
// totally transparent.
return NS_RGBA(0, 0, 0, 0);
}
// Within an opaque widget (or no widget at all), so the backstop
// color must be totally opaque. The user's default background
// as reported by the prescontext is guaranteed to be opaque.
return GetDefaultBackgroundColorToDraw();
}
On Windows 7, the widget returns eTransparencyBorderlessGlass from
GetTransparencyMode(), which also avoids the backstop color.
Depends on D51457
Assignee | ||
Comment 3•5 years ago
|
||
For regular elements, whenever -moz-appearance is used, the CSS background is
ignored. Root elements were behaving specially, and the background color also
needed to be adjusted.
For example, for Windows 7, we have the following CSS rule;
:root {
background-color: transparent;
-moz-appearance: -moz-win-borderless-glass;
}
This change makes the root element more consistent with other elements, so the
extra background-color: transparent
declaration is no longer necessary.
This change does not let content documents opt out of forced opaqueness:
Root content documents still get an opaque background color from an existing
check further down in this method.
Depends on D51458
Assignee | ||
Comment 4•5 years ago
|
||
This is preferable over a hardcoded color because it lets Gecko choose where the
window background should come from. We would like the background to be handled
by the OS widget, and prevent Gecko from painting a CSS background color.
Depends on D51459
Assignee | ||
Comment 5•5 years ago
|
||
The window background will be contributed by the widget itself, which renders
them underneath Gecko's rendering.
As a result, -moz-appearance: dialog is now equivalent to the combination
-moz-appearance: none; background-color: transparent.
This change does not turn the widget itself transparent because nsCocoaWindow
does not allow top level windows to become transparent (ever since bug 1162649).
If we ever add support for top level widgets with transparent backgrounds again,
we will probably want to treat -moz-appearance: dialog differently from
-moz-appearance: none; background-color: transparent, but for now this is fine.
This change means that Gecko's rendering will go into transparent buffers. This
may result in some loss of subpixel AA in various cases.
In the main browser window, there are CSS backgound colors that cover all the
non-vibrant areas of the window, so in that window we still render mostly onto
opaque pixels.
Depends on D51460
Assignee | ||
Comment 6•5 years ago
|
||
With -moz-appearance: dialog now always being transparent, setting up our own
vibrant views for sheet windows is no longer necessary. We now let the regular
sheet window background show through, and that background is already vibrant.
Depends on D51461
Assignee | ||
Comment 7•5 years ago
|
||
Now that there is no Gecko-contributed background color in the window any more,
there's nothing that needs to be cleared away. This simplifies things.
Depends on D51462
Assignee | ||
Comment 8•5 years ago
|
||
This is needed because under the new vibrancy model, vibrant -moz-appearance
values only work on elements that have nothing rendered behind them. The elements
with the vibrant appearance become truly transparent.
Depends on D51464
Assignee | ||
Comment 9•5 years ago
|
||
This code was assuming that the only non-opaque parts of compositor rendering would be the
parts of the window that had vibrancy. But now that the default window background is transparent,
we can have non-vibrant parts where we render into transparency. Dialog windows such as sheet
windows are an example of this.
So instead of using the non-vibrant region of the window as its opaque region, we now use
the region that is covered by opaque Gecko layers. This region is a lot more conservative:
For example, the main browser chrome is now entirely transparent, because the chrome's opaque
parts share a layer with its transparent parts.
As a result, this change slightly affects the CALayer partitioning in the main browser window:
The entire browser chrome is now transparent, not just the tab bar.
The web content area is still opaque.
I think this will be fine. The thing I'm most concerned about is that scrolling inside web
content might cause invalidations of pixels from the chrome, because then we'd recomposite
the CALayers that cover the vibrant tab bar. This doesn't seem to happen most of the time
though, from what I can tell.
Depends on D51465
Comment 10•5 years ago
|
||
I pushed a try build so I can try the changes out later today:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e14c55dd17fb0fe6bbcc8e98e77269047f96c0a
Assignee | ||
Comment 11•5 years ago
|
||
New try push with the build problems fixed (hopefully):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b74b70f97790232845dc8bc38424f2b691e94bf
Updated•5 years ago
|
Comment 12•5 years ago
|
||
Comment 13•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d0cf63ab4616
https://hg.mozilla.org/mozilla-central/rev/fdbf0586295d
https://hg.mozilla.org/mozilla-central/rev/8c02fa3fcc22
https://hg.mozilla.org/mozilla-central/rev/fbdd3e1c6155
https://hg.mozilla.org/mozilla-central/rev/d5f51e8bbbcb
https://hg.mozilla.org/mozilla-central/rev/f4b92056ed05
https://hg.mozilla.org/mozilla-central/rev/1664ebaed4a6
https://hg.mozilla.org/mozilla-central/rev/4d9f24928383
Updated•5 years ago
|
Assignee | ||
Comment 14•5 years ago
|
||
I need more time to fix the regression that this caused. So I'm going to prepare a backout-patch for 72.
Assignee | ||
Comment 15•5 years ago
|
||
Approval Request Comment
[Feature/Bug causing the regression]: This bug
[User impact if declined]: Regressions: bug 1596826, bug 1599366, bug 1601183
[Is this code covered by automated tests?]: some of it is
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: See the other bugs
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not very
[Why is the change risky/not risky?]: it's a backout, should be low-risk
[String changes made/needed]: none
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7346baab256a7666a86e1a538545561698f3d5d3
Updated•5 years ago
|
Comment 16•5 years ago
|
||
backout |
Updated•5 years ago
|
Comment 17•5 years ago
|
||
Comment 18•5 years ago
|
||
Does this need to be pushed to beta (gecko 73) again after the central-to-beta merge on Monday?
Comment 19•5 years ago
|
||
This is an update for bug 1596826 that is a regression from this one.
There are 29 total failures in the last 7 days on macosx1014-64 debug and macosx1014-64-shippable opt.
Recent failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=283556900&repo=autoland&lineNumber=1878
[task 2020-01-06T02:55:07.063Z] 02:55:07 INFO - TEST-START | browser/base/content/test/general/browser_tab_detach_restore.js
[task 2020-01-06T02:55:08.012Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.013Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.013Z] 02:55:08 INFO - GECKO(2234) | [GFX1-]: Receive IPC close with reason=AbnormalShutdown
[task 2020-01-06T02:55:08.013Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.013Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.013Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.013Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.014Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.014Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.014Z] 02:55:08 INFO - GECKO(2234) | Exiting due to channel error.
[task 2020-01-06T02:55:08.199Z] 02:55:08 INFO - TEST-INFO | Main app process: exit 1
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - Buffered messages logged at 02:55:07
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - Entering test bound
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - Console message: OpenGL compositor Initialized Succesfully.
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - Version: 2.1 INTEL-12.9.22
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - Vendor: Intel Inc.
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - Renderer: Intel Iris OpenGL Engine
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - FBO Texture Target: TEXTURE_2D
[task 2020-01-06T02:55:08.200Z] 02:55:08 INFO - TEST-PASS | browser/base/content/test/general/browser_tab_detach_restore.js | Should have properly copied the permanentKey -
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - TEST-PASS | browser/base/content/test/general/browser_tab_detach_restore.js | Should have restore data for the closed window -
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - Console message: OpenGL compositor Initialized Succesfully.
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - Version: 2.1 INTEL-12.9.22
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - Vendor: Intel Inc.
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - Renderer: Intel Iris OpenGL Engine
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - FBO Texture Target: TEXTURE_2D
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - Buffered messages finished
[task 2020-01-06T02:55:08.210Z] 02:55:08 ERROR - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_tab_detach_restore.js | application terminated with exit code 1
[task 2020-01-06T02:55:08.210Z] 02:55:08 INFO - runtests.py | Application ran for: 0:02:35.883497
[task 2020-01-06T02:55:08.211Z] 02:55:08 INFO - zombiecheck | Reading PID log: /var/folders/gg/h30_s40x70b507f3mjqtg4_m000017/T/tmpur84y6pidlog
Since that is a restricted bug, i cannot update there. Markus please take a look.
Assignee | ||
Comment 20•5 years ago
|
||
Yes, let's back this out on 73 again.
I have compiled the Beta branch with this patch applied locally and everything seemed to be fine. I didn't do a try push.
Comment 21•5 years ago
|
||
backout |
Backed out from Beta for 73.0b5. This remains fixed on m-c for 74+.
https://hg.mozilla.org/releases/mozilla-beta/rev/ff911052beb1
Comment 22•5 years ago
|
||
Comment 23•5 years ago
|
||
Backing this out on trunk now since the regressions haven't yet been fixed.
https://hg.mozilla.org/integration/autoland/rev/ecc5c853bb77
beta 74 is next.
Comment 24•5 years ago
|
||
Comment 25•5 years ago
|
||
Backout merge to central: https://hg.mozilla.org/mozilla-central/rev/ecc5c853bb77
Assignee | ||
Comment 26•5 years ago
|
||
Thanks for taking care of this, Julien.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 29•5 years ago
|
||
I've picked this back up. There are four bugs that I want to fix before re-landing:
- Sheet windows should keep their vibrant background rather than becoming white. (I don't think anybody filed a bug for this last time this landed.)
- The bug about sidebar selections (bug 1602193) needs to be fixed.
- The Windows 7 regressions (bug 1599366, bug 1601183) I can avoid by preserving the background-color behavior for glass -moz-appearance values.
- XUL menu scroll arrows should be implemented differently (bug 1624599).
The last time this landed, it also caused a bunch of crashes, both for our users, and, intermittently, in CI tests. Before landing, we'll have to run the affected CI tests on a try push often enough to make sure the crashes don't come back, and if they do, fix them.
Comment 30•4 years ago
|
||
Comment on attachment 9105886 [details]
Bug 1592739 - Make nsChildView::WidgetPaintsBackground() return true. r=tnikkel
Revision D51458 was moved to bug 1644459. Setting attachment 9105886 [details] to obsolete.
Comment 31•4 years ago
|
||
Comment on attachment 9105888 [details]
Bug 1592739 - Style the browser window's root element with -moz-appearance: dialog on macOS (which is the default). r=ntim
Revision D51460 was moved to bug 1644459. Setting attachment 9105888 [details] to obsolete.
Comment 32•4 years ago
|
||
Comment on attachment 9105893 [details]
Bug 1592739 - Make the sidebar non-vibrant when any lwtheme is in use. r=ntim
Revision D51465 was moved to bug 1644461. Setting attachment 9105893 [details] to obsolete.
Updated•4 years ago
|
Comment 33•4 years ago
|
||
Comment on attachment 9105890 [details]
Bug 1592739 - Remove vibrancy code for sheet windows. r=spohl
Revision D51462 was moved to bug 1645150. Setting attachment 9105890 [details] to obsolete.
Comment 34•4 years ago
|
||
Comment 36•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/238e2fc3986e
https://hg.mozilla.org/mozilla-central/rev/299531010a94
https://hg.mozilla.org/mozilla-central/rev/4d9cf726af44
https://hg.mozilla.org/mozilla-central/rev/55b1d1bb8659
Updated•4 years ago
|
Comment 38•4 years ago
|
||
This change seems to have broken dark mode vibrancy (even without WR enabled).
2020-06-16 Nightly with dark mode: https://i.imgur.com/6QFi4A3.png
2020-06-17 Nightly with dark mode: https://i.imgur.com/uVlSqgH.png
Note that I'm using a tiny userChrome.css to get dark mode vibrancy in the titlebar, I don't know if it's intended to keep this available at all:
`
.theme-body {
background: transparent !important;
}
#TabsToolbar, .table-widget-empty-text
{
-moz-appearance: -moz-mac-vibrant-titlebar-dark !important;
}
`
Assignee | ||
Comment 39•4 years ago
|
||
(In reply to Alex from comment #38)
This change seems to have broken dark mode vibrancy (even without WR enabled).
This is somewhat expected because the dark theme sets a dark background color on the window. This background color now interferes with the vibrancy effect. To make this work as expected, we'd need to move the background color to an element that does not overlap the tab bar, similar to what we need to do for lwthemes (bug 1594132).
Comment 40•4 years ago
|
||
Thank you for the explanation. Now I understand what's happening here and why the light mode vibrancy still works as before.
So, do you think you will make this change to allow for tab bar vibrancy*? I for one would appreciate it. In any case, thank you for considering my request.
(*Or even whole chrome vibrancy? Even if I'm not using them, I think some people make quite interesting themes with it, e.g. https://i.redd.it/q0hpdbkic1g41.png)
Updated•4 years ago
|
Comment 41•4 years ago
|
||
A small piece of info: someone on reddit found a way to preserve vibrancy by adding this to userChrome.css:
:root{ background: transparent !important }
Description
•