1619367 - Some tab titles are brighter on macOS

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Screenshot of bright "Google" tab

I think I've been seeing this the last couple of days. It only happens intermittently and goes away when you click on the the tab.

Comment 1

[Sören Hentzschel]

I don't think that it's a WebRender issue. I see this since a few weeks on macOS with WebRender disabled.

Comment 2

[Sören Hentzschel]

To give some more information: I was able to reproduce the isssue on an old iMac with macOS 10.11 at work. Now I have a MacBook Pro at work with macOS 10.13 where I can reproduce the issue as well. But I am not able to reproduce the issue on my private MacBook Pro with macOS 10.15. WebRender is disabled on all three systems. Unfortunately my private MacBook Pro is the only system where I can use mozregression - and that's my only unaffected device.

Comment 3

[Sören Hentzschel]

I was able to run mozregression on an affected device and got bug 1611462 as regressing bug (I executed mozregression two times to make sure).

11:00.22 INFO: Last good revision: fb7e027b119daca3020641b22698a47e570bcdbf
11:00.22 INFO: First bad revision: ff238eaf45c0e29c686aad03732799c96c04c866
11:00.22 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fb7e027b119daca3020641b22698a47e570bcdbf&tochange=ff238eaf45c0e29c686aad03732799c96c04c866

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Is this webrender-specific?

Comment 6

[Sören Hentzschel]

WebRender is disabled on all the devices I tested.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Ah, sorry, had missed comment 2.

I don't have a mac, so this is not going to be easy to track down... Dao, do tabs use sticky / relative positioning somehow, or something of that sort? If so, how do they change?

All position property changes should trigger a repaint, so I think that if anything my patch was just uncovering a pre-existing bug... but we'll see.

• absolute or fixed always reframe.
• relative <-> sticky and static <-> sticky repaints via this check
• relative <-> static repaints via this check.

So we should always end up repainting unless I'm missing something.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Soren, how do I consistently repro this? Will try to steal some mac somewhere... :)

Comment 9

[Sören Hentzschel]

I don't have reliable steps to reproduce because it does not happen with every tab. So what I did was to run mozregression with --profile="…" --profile-persistence clone so I was able to test with my current browsing session with six pinned and five unpinned tabs. After switching tabs and/or opening new tabs and switching again it always happened shortly thereafter. If it helps I can make a screencast later today.

Please also note that I was only able to reproduce the issue on 2 of 3 computers. Unfortunately all run with a different version of macOS (10.11, 10.13 and 10.15) and I don't know if the version of macOS is important to reproduce this issue or if there is another reason why I can't reproduce on the third system (10.15).

Comment 10

[Dão Gottwald [:dao]]

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

Dao, do tabs use sticky / relative positioning somehow, or something of that sort? If so, how do they change?

We use relative positioning for the selected tab:

https://searchfox.org/mozilla-central/rev/d6f957415cf009995ecb539ef1425316d82164a9/browser/themes/shared/tabs.inc.css#87

and then also for some other elements within tabs and within the tab strip (just search in the above file).

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Miko, we did RDL bugs that got z-sorting wrong when z-index changes dynamically under some circumstances, right?

I think this is a case of that, what makes the text less bright is this rule: https://searchfox.org/mozilla-central/rev/61f224ec08ddc6f9a93ac45c8c3c5f7159be7c2a/browser/themes/osx/browser.css#553

That somehow is incorrectly not applying.

I tried to repro this on my girfriend's mac and failed. Sören, can you confirm this doesn't repro if you set layout.display-list.retain.chrome=false? (may need a restart)

Comment 13

[Sören Hentzschel]

With layout.display-list.retain.chrome=false I can still reproduce the issue after a restart of Firefox.

Comment 14

[Jessie [:jbonisteel] pls NI]

We will need to have reliable STR to diagnose and fix this issue.

Comment 15

[Miko Mynttinen]

I managed to find STR for this:

1. Run mozregression --launch 2020-03-25 --pref "gfx.webrender.all:true" --profile-persistence reuse -p ~/Code/test-profile
2. Open three tabs of reddit.com
3. Visit all the tabs once to reset their title
4. Do something that causes Firefox to lose focus (alt+tab or click some other window)

What is the expected behavior of the tab titles? On my Windows machine, every tab title seems to be bright when Firefox has focus.

Comment 16

[Jessie [:jbonisteel] pls NI]

Jeff, curious if those STR reproduce the issue for you too?

Comment 17

[Markus Stange [:mstange]]

(In reply to Miko Mynttinen [:miko] from comment #15)

What is the expected behavior of the tab titles? On my Windows machine, every tab title seems to be bright when Firefox has focus.

I think this is unrelated - the artifact doesn't depend on window focus. The artifact also seems to be Mac-specific.

I find this easiest to reproduce when I open a whole lot of tabs so that the tab bar scrolls. Then, as I scroll around, some tabs briefly bright and then darken again.
I was able to capture a video recording and some display list dumps of this happening, but couldn't find any indication of the problem in the display list - the relevant items looked the same before, during and after the artifact. The display item structure for a tab looks like this:

nsDisplayContainer p=0x10bade3f0 f=0x1290190e8(Box(hbox)(2) class:tab-content) key=28 bounds(65340,540,3420,960) layerBounds(96300,540,3420,960) visible(8040,0,70440,1980) building(8040,0,70440,1980) componentAlpha(66600,585,2822,870) clip() asr() clipChain() ref=0x11f68d020 agr=0x11f6d03c8
  CompositorHitTestInfo p=0x12931e170 f=0x1290190e8(Box(hbox)(2) class:tab-content) key=27 bounds(0,0,0,0) layerBounds(30960,0,0,0) visible(8040,0,70440,1980) building(8040,0,70440,1980) componentAlpha(0,0,0,0) clip(8040,0,70440,1980) asr() clipChain(0x10bae0298 <8040,0,70440,1980> [root asr]) ref=0x11f68d020 agr=0x11f6d03c8 hitTestInfo(0x5) hitTestArea(64800,0,4500,1980)
  Opacity p=0x129319528 f=0x1290191b0(ImageBox(image)(2) class:tab-icon-image) key=41 bounds(65340,540,960,960) layerBounds(96300,540,960,960) visible(65340,540,960,960) building(8040,0,70440,1980) componentAlpha(0,0,0,0) clip() asr() clipChain() ref=0x11f68d020 agr=0x11f6d03c8 hitTestInfo(0x5) hitTestArea(65340,540,960,960) (opacity 0.9) layer=0x132420000
    XULImage p=0x10bade5a0 f=0x1290191b0(ImageBox(image)(2) class:tab-icon-image) key=80 bounds(65340,540,960,960) layerBounds(96300,540,960,960) visible(65340,540,960,960) building(8040,0,70440,1980) componentAlpha(0,0,0,0) clip(8040,0,70440,1980) asr() clipChain(0x10bae0298 <8040,0,70440,1980> [root asr]) ref=0x11f68d020 agr=0x11f6d03c8 layer=0x132420000
  Opacity p=0x129320250 f=0x129019328(XULScroll(hbox)(5) class:tab-label-container) key=41 bounds(66660,585,2100,870) layerBounds(97620,585,2100,870) visible(8040,0,70440,1980) building(8040,0,70440,1980) componentAlpha(66600,585,2822,870) clip() asr() clipChain() ref=0x11f68d020 agr=0x11f6d03c8 (opacity 0.7)
    Mask p=0x129319868 f=0x129019328(XULScroll(hbox)(5) class:tab-label-container) key=40 bounds(66660,585,2100,870) layerBounds(97620,585,2100,870) visible(66660,585,2100,870) building(8040,0,70440,1980) componentAlpha(66600,585,2822,870) clip() asr() clipChain() ref=0x11f68d020 agr=0x11f6d03c8 hitTestInfo(0x7) hitTestArea(66660,399,2100,1182) layer=0x11fd0e800 effects=(opacity(0.700000))
      CompositorHitTestInfo p=0x12529ac60 f=0x129019260(Box(hbox)(5) class:tab-label-container) key=27 bounds(0,0,0,0) layerBounds(30960,0,0,0) visible(0,0,0,0) building(66660,399,2100,1182) componentAlpha(0,0,0,0) clip(66660,399,2100,1182) asr() clipChain(0x10bae2fb8 <66660,399,2100,1182> [root asr]) ref=0x11f68d020 agr=0x11f6d03c8 hitTestInfo(0x5) hitTestArea(66660,399,2702,1182)
      Text p=0x10bade698 f=0x129019668(Text(0)"New Tab") key=71 bounds(66600,585,2822,870) layerBounds(97560,585,2822,870) visible(66660,585,2100,870) building(66660,399,2100,1182) componentAlpha(66600,585,2822,870) clip(66660,399,2100,1182) asr() clipChain(0x10bae2fb8 <66660,399,2100,1182> [root asr]) ref=0x11f68d020 agr=0x11f6d03c8 layer=0x122564000

Notably, we have a mask here, and an opacity. The opacity is probably combined into the mask rendering somehow - I recall some code that combines the two but I don't remember exactly how it does that.
The brightening looks to me as if the mask contents accumulate: Rendering the same half-transparent mask into a mask surface multiple times would result in a mask that "darkens less", i.e. results in brighter text.

My hunch is that we re-use a surface for the mask which contains old content, and we forget to clear it.

Comment 18

[Miko Mynttinen]

Thank you Markus, that helped me to track this down.

I believe that this bug is caused by nsDisplayOpacity deferring opacity handling to nsDisplayMasksAndClipPaths1, which is not included in the WR mask image invalidation2. I think to detect all mask changes, the checks from nsDisplayMasksAndClipPaths::ComputeInvalidationRegion() and nsDisplayEffectsBase::ComputeInvalidationRegion()3 are needed. Same invalidation is also needed for nsDisplayFilters.

A simple fix for now is to disable this optimization with WebRender.

Comment 19

[Miko Mynttinen]

Attachment: Bug 1619367 - Fix mask/filter opacity optimizations with WebRender r=jrmuizel

Comment 20

[Emilio Cobos Álvarez (:emilio)]

But this seemed to happen with WR off per comment 6?

Comment 21

[Miko Mynttinen]

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

But this seemed to happen with WR off per comment 6?

It is definitely possible that the patch for bug 1611462 exposed similar problems with FrameLayerBuilder.

Comment 22

[Markus Stange [:mstange]]

Nice, thanks for debugging the WR problem, Miko.

I looked a bit into the non-WR problem and it seems like that's an entirely different bug. In non-WR, these masks are always painted via the inactive path, and no mask surfaces are retained. The contents of the mask surfaces look correct: the mask surfaces for background tabs always have the 0.7 opacity applied inside them, and the mask surfaces for foreground tabs are more opaque. So I think with non-WR the problem is happening inside the masked content. Maybe the problem is even in the text-on-vibrancy drawing code.

Here's the code I used to dump the mask surface contents:

if (maskSurface) {
  nsCString str = gfxUtils::GetAsDataURI(maskSurface);
  uint32_t hash = HashString(str);
  nsPrintfCString filename("/Users/mstange/Desktop/mask-surfaces/%x.png", hash);
  gfxUtils::WriteAsPNG(maskSurface, filename.get());
  printf("maskSurface: %s\n", filename.get());
}

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a62453df7268
Disable SVG opacity optimization with WebRender r=jrmuizel

Comment 26

[Daniel Varga [:dvarga]]

Backed out changeset a62453df7268 (Bug 1619367) for causing web-platform-tests-reftest failures at /css/vendor-imports/mozilla/mozilla-central-reftests/masking/mask-opacity-1a.html

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=a62453df726856abd472ebf1310ac6dfa9f55bf2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=297624102&repo=autoland&lineNumber=12722

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=297624102&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=c224327f2bddd5f1a00b2c8edc6b098188884202

[task 2020-04-14T23:40:48.214Z] 23:40:48     INFO - TEST-PASS | leakcheck | default no leaks detected!
[task 2020-04-14T23:40:48.215Z] 23:40:48     INFO - leakcheck | Processing leak log file c:\users\task_1586906632\appdata\local\temp\tmpzzxtmo\runtests_leaks_4572_gpu_pid10588.log
[task 2020-04-14T23:40:48.215Z] 23:40:48     INFO - TEST-UNEXPECTED-FAIL | leakcheck | gpu missing output line for total leaks!
[task 2020-04-14T23:40:48.215Z] 23:40:48     INFO - leakcheck | Processing leak log file c:\users\task_1586906632\appdata\local\temp\tmpzzxtmo\runtests_leaks_4572_tab_pid2664.log
[task 2020-04-14T23:40:48.216Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.216Z] 23:40:48     INFO - == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 2664
[task 2020-04-14T23:40:48.216Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.216Z] 23:40:48     INFO -      |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
[task 2020-04-14T23:40:48.217Z] 23:40:48     INFO -      |                                      | Per-Inst   Leaked|   Total      Rem|
[task 2020-04-14T23:40:48.217Z] 23:40:48     INFO -    0 |TOTAL                                 |       43        0|   35542        0|
[task 2020-04-14T23:40:48.227Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.227Z] 23:40:48     INFO - nsTraceRefcnt::DumpStatistics: 1008 entries
[task 2020-04-14T23:40:48.227Z] 23:40:48     INFO - TEST-PASS | leakcheck | tab no leaks detected!
[task 2020-04-14T23:40:48.228Z] 23:40:48     INFO - leakcheck | Processing leak log file c:\users\task_1586906632\appdata\local\temp\tmpzzxtmo\runtests_leaks_4572_tab_pid2780.log
[task 2020-04-14T23:40:48.228Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.228Z] 23:40:48     INFO - == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 2780
[task 2020-04-14T23:40:48.228Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.229Z] 23:40:48     INFO -      |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
[task 2020-04-14T23:40:48.229Z] 23:40:48     INFO -      |                                      | Per-Inst   Leaked|   Total      Rem|
[task 2020-04-14T23:40:48.229Z] 23:40:48     INFO -    0 |TOTAL                                 |       33        0|  226013        0|
[task 2020-04-14T23:40:48.239Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.239Z] 23:40:48     INFO - nsTraceRefcnt::DumpStatistics: 1019 entries
[task 2020-04-14T23:40:48.240Z] 23:40:48     INFO - TEST-PASS | leakcheck | tab no leaks detected!
[task 2020-04-14T23:40:48.240Z] 23:40:48     INFO - leakcheck | Processing leak log file c:\users\task_1586906632\appdata\local\temp\tmpzzxtmo\runtests_leaks_4572_tab_pid6328.log
[task 2020-04-14T23:40:48.240Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.241Z] 23:40:48     INFO - == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 6328
[task 2020-04-14T23:40:48.241Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.241Z] 23:40:48     INFO -      |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
[task 2020-04-14T23:40:48.242Z] 23:40:48     INFO -      |                                      | Per-Inst   Leaked|   Total      Rem|
[task 2020-04-14T23:40:48.242Z] 23:40:48     INFO -    0 |TOTAL                                 |       40        0|   90738        0|
[task 2020-04-14T23:40:48.252Z] 23:40:48     INFO - 
[task 2020-04-14T23:40:48.252Z] 23:40:48     INFO - nsTraceRefcnt::DumpStatistics: 932 entries

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dfb973b3e25c
Fix mask/filter opacity optimizations with WebRender r=jrmuizel

Comment 28

[Cristian Brindusan [:cbrindusan]]

Backed out changeset dfb973b3e25c (Bug 1619367) for reftest failures at mask-opacity-invalidation-1.html.
https://hg.mozilla.org/integration/autoland/rev/d8036a25708a39d8b3f868400a63e4d0dbef58ab

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=dfb973b3e25c880564106f80969c0cacfecda213&selectedJob=297941801

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=297941801&repo=autoland&lineNumber=26761

Comment 29

[Miko Mynttinen]

Adjusting reftest fuzziness could be more convenient. :)

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cec498c10bc1
Fix mask/filter opacity optimizations with WebRender r=jrmuizel

Comment 31

[Dorel Luca [:dluca]]

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

Comment 32

[Sören Hentzschel]

A fix for the WebRender path landed, but it's still unfixed for the non-WebRender path which is the default on macOS.

Comment 33

[Markus Stange [:mstange]]

I've filed bug 1631424 on the non-WebRender bug.