Closed Bug 1592739 Opened 5 years ago Closed 4 years ago

macOS vibrancy doesn't work in WebRender + OS compositor configuration

Categories

(Core :: Web Painting, enhancement, P2)

All
macOS
enhancement

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox-esr78 --- disabled
firefox72 --- disabled
firefox73 --- disabled
firefox74 --- disabled
firefox75 --- disabled
firefox77 --- disabled
firefox78 --- disabled
firefox79 --- fixed

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.
Depends on: 1593150

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.

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

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

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

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

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

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

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

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

I pushed a try build so I can try the changes out later today:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e14c55dd17fb0fe6bbcc8e98e77269047f96c0a

Priority: -- → P2
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/d0cf63ab4616 Make nsChildView::WidgetPaintsBackground() return true. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/fdbf0586295d Ignore the background-color CSS value on the window document's root element if that element has a -moz-appearance. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/8c02fa3fcc22 Style the browser window's root element with -moz-appearance: dialog on macOS (which is the default). r=ntim https://hg.mozilla.org/integration/autoland/rev/fbdd3e1c6155 Make -moz-appearance: dialog render nothing. r=spohl https://hg.mozilla.org/integration/autoland/rev/d5f51e8bbbcb Remove vibrancy code for sheet windows. r=spohl https://hg.mozilla.org/integration/autoland/rev/f4b92056ed05 Stop clearing the background behind vibrant -moz-appearance items. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/1664ebaed4a6 Make the sidebar non-vibrant when any lwtheme is in use. r=ntim https://hg.mozilla.org/integration/autoland/rev/4d9f24928383 Stop using the vibrant region as the transparent region. r=mattwoodrow
Regressions: 1595408
Regressions: 1599366
Regressions: 1602193
Regressions: 1601183

I need more time to fix the regression that this caused. So I'm going to prepare a backout-patch for 72.

Attached file backout patch (deleted) —

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

Attachment #9116569 - Flags: approval-mozilla-beta?
Attachment #9116569 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Regressions: 1594132
Comment on attachment 9116569 [details] backout patch (removing approval flag to get this out of uplift queries)
Attachment #9116569 - Flags: approval-mozilla-beta+

Does this need to be pushed to beta (gecko 73) again after the central-to-beta merge on Monday?

Flags: needinfo?(mstange)

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.

Attached patch backout patch for 73 (deleted) — Splinter Review

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.

Flags: needinfo?(mstange)

Backed out from Beta for 73.0b5. This remains fixed on m-c for 74+.
https://hg.mozilla.org/releases/mozilla-beta/rev/ff911052beb1

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.

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

Thanks for taking care of this, Julien.

Status: REOPENED → ASSIGNED
Depends on: 1624599
Depends on: 1617088

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.

Depends on: 1644459
Depends on: 1644461
Depends on: 1493789

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.

Attachment #9105886 - Attachment is obsolete: true

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.

Attachment #9105888 - Attachment is obsolete: true

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.

Attachment #9105893 - Attachment is obsolete: true
Attachment #9105887 - Attachment description: Bug 1592739 - Ignore the background-color CSS value on the window document's root element if that element has a -moz-appearance. r=tnikkel → Bug 1592739 - Ignore the background-color CSS value on the window document's root element if that element has a non-glass -moz-appearance. r=tnikkel

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.

Attachment #9105890 - Attachment is obsolete: true
No longer regressions: 1594132
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/238e2fc3986e Ignore the background-color CSS value on the window document's root element if that element has a non-glass -moz-appearance. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/299531010a94 Make -moz-appearance: dialog render nothing. r=spohl https://hg.mozilla.org/integration/autoland/rev/4d9cf726af44 Stop clearing the background behind vibrant -moz-appearance items. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/55b1d1bb8659 Stop using the vibrant region as the transparent region. r=mattwoodrow
No longer depends on: 1617088

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;
}
`

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

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)

Target Milestone: mozilla72 → mozilla79

A small piece of info: someone on reddit found a way to preserve vibrancy by adding this to userChrome.css:

:root{ background: transparent !important }

Regressions: 1672081
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: