1483533 - tab bar flickering when interacting with tabs (open, close, scroll) (ClientStorage)

Status: VERIFIED FIXED

(Core :: Graphics, defect, P3)

Description

[Sören Hentzschel]

Attachment: flickering.mov

[Tracking Requested - why for this release]:

When opening or closing a tab there are visual glitches in the tab bar. It doesn't happen in Firefox 62 Beta, only in Firefox 63 Nightly. Operating system is the latest macOS Mojave Public Beta (it was already present with the previous Betas).

Tentatively setting as dependency of bug 1466336 because I can't reproduce on another machine with macOS 10.11. But since I can't also reproduce on Firefox 62 I am not sure if it's really depends on the version of macOS.

I attached a video showing the issue. First you see the opening and closing of tabs in Firefox 62 Beta without problems, then I switched to Firefox 63 Nightly where you can see the issue.

Comment 1

[Sören Hentzschel]

Attachment: flickering2.mov

There is also an extreme flickering when scrolling within the tab bar.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hi Sören,

Do you have WebRender enabled, by chance?

Comment 3

[Sören Hentzschel]

All webrender prefs in about:config have their default values. about:support says:

WEBRENDER	
opt-in by default: WebRender is an opt-in feature

WEBRENDER_QUALIFIED	
blocked by env: No qualified hardware

Comment 4

[Sören Hentzschel]

Attachment: about-support.txt

I attached my about:support.

Another interesting note: There are graphical glitches while scrolling in the tab bar but not as bad bad as in the attached video. Once I have "about:welcome" open in a tab these graphical glitches are extreme like in the video. When I remove the animated graphic via devtools the issue is not gone but similar to the issue wihtout "about:welcome" being opened.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

This smells like a graphics issue...

Is this a recent regression? If so, do you have time to help us find a regression range with mozregression?

Comment 6

[Sören Hentzschel]

When I filed the bug it was hard to find the regression because it hadn't always occurred. Luckily  25 minutes ago I found out that a open "about:welcome" tab makes the problem always visible. So I was finally able to find the regressing bug.

16:51.05 INFO: Last good revision: 1c945ede0071fac3f11a6cddd48cb68517a1132a
16:51.05 INFO: First bad revision: 96ce98b72056cd2c19c3640b4b7834b73dcc42ac
16:51.05 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1c945ede0071fac3f11a6cddd48cb68517a1132a&tochange=96ce98b72056cd2c19c3640b4b7834b73dcc42ac

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Ruh roh.

dthayer is out, so I'm redirecting this mstange... any idea what we should do here? Apparently, Mojave is coming out pretty soon?

Comment 8

[Markus Stange [:mstange]]

Oh no! :(

I wonder why this shows up in the tab bar and nowhere else. If it's caused by bad texture contents, we should be seeing it everywhere...

Matt, do you have a machine with the 10.14 Beta that you could use to investigate this?

Comment 9

[Marcia Knous [:marcia]]

Asking Camelia to look at this to see if she can reproduce the issue noted in Comment 8.

Comment 10

[Sören Hentzschel]

A note: I experienced the problem with my old MacBook Pro Late 2013. Now I have a new MacBook Pro 2018 and I can't reproduce the problem on the new machine. Unfortunately that means I can't help with testing / providing more data because I sold my old MacBook.

Comment 11

[Camelia Badau [:cbadau], Desktop QA]

Attachment: issue.mov

We tested on macOS 10.14 (beta 6 build - 18A365a) using latest Nightly 63.0a1 (2018-08-03) and Nightly 63.0a1 (2018-08-16) and here are the following mentions:

- on MacBook Pro (Retina, 15-inch, Late 2013) with Intel Iris Pro 1536MB - we didn't manage to reproduce the issue: tab bar DOESN'T flicker when interacting with tabs (open, close, scroll)  

- on iMac (21.5-inch, Late 2012) with NVIDIA GeForce GT 640M 512MB - we REPRODUCED the issue: the tab bar flickers when interacting with tabs (open, close, scroll). Please see "issue.mov" attachment. 

It seems it is related to macOS with NVIDIA Graphics. 

The issue isn't reproducible on Firefox 62 Beta 19.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked for 63 since it's a recent regression.

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

We have the regression window in comment #6

Comment 14

[Pascal Chevrel:pascalc (PTO until April 26)]

Doug, do you have an update on this regression? Thanks!

Comment 15

[Alex Thayer [:alexical] (she/her)]

(In reply to Pascal Chevrel:pascalc from comment #14)
> Doug, do you have an update on this regression? Thanks!

Took a bit to find a loaner of a Mac with nvidia hardware - it's shipping now. As soon as it arrives I'll start working on this actively.

Comment 16

[Alex Thayer [:alexical] (she/her)]

Just received the iMac with nvidia hardware - the tab bar is flickering on scroll even on High Sierra. Hopefully this is the same issue, but in either case it needs to be fixed :/

Comment 17

[Alex Thayer [:alexical] (she/her)]

So, removing the GL_TEXTURE_STORAGE_HINT_APPLE fiddling fixes things on nvidia hardware. Unfortunately it also degrades performance a little bit. I'm trying to investigate why this breaks things for nvidia cards. My working theory is that glTestObjectAPPLE / glFinishObjectAPPLE are not properly being respected by the nvidia driver, and we're writing to the buffers before the GPU is actually done with them.

Potentially other synchronization mechanisms could work, so I'll be investigating that next.

Comment 18

[Alex Thayer [:alexical] (she/her)]

Sören, can you verify that this build[1] fixes the issue on your end? It works for me on an iMac with Nvidia hardware which was flickering in the same way you describe, but I wanted to verify that it works for you as well.

[1] https://queue.taskcluster.net/v1/task/d3HSLehhRmi6reXvFQm1dg/runs/0/artifacts/public/build/target.dmg

Comment 19

[Sören Hentzschel]

Unfortunately I can't verify the fix because I no longer have my old MacBook with the graphics card from Nvidia and my new MacBook has a graphics card from AMD.

Comment 20

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1483533 - Delay texture delete for DirectMapTextureSource r?jrmuizel

I wish I understood a little better what precisely is going on
here. What seems to be the problem is calling glDeleteTextures
too early, but I can't pin down exactly when "too early" is.
In any case I can no longer reproduce the issue with this patch
applied, and I cannot observe any performance degradation, and
it's not a remarkably risky patch, so I'm opting to cut the
investigation short. Any insights would be appreciated though.

Comment 21

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9009704 [details]
Bug 1483533 - Delay texture delete for DirectMapTextureSource r?jrmuizel

Jeff Muizelaar [:jrmuizel] has approved the revision.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59759503b889
Delay texture delete for DirectMapTextureSource r=jrmuizel

Comment 23

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecdd5ebf4a43
Backed out changeset 59759503b889 for test failures RefPtr CLOSED TREE

Comment 24

[Andrei Ciure[:aciure]]

Backed out changeset 59759503b889 (bug 1483533) for test failures RefPtr

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=199864613&revision=59759503b889c2e3a2644e5a0376e8d9dee6afe4

failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=199864613&revision=59759503b889c2e3a2644e5a0376e8d9dee6afe4&failure_classification_id=2

backout: https://hg.mozilla.org/integration/autoland/rev/ecdd5ebf4a435ac57add63a333aa21e81a7401bc

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc263497b339
Delay texture delete for DirectMapTextureSource r=jrmuizel

Comment 26

[Arthur Iakab [arthur_iakab]]

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

Comment 27

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Doug, this is a tracked bug for 63, can you please request beta uplift? Thanks!

Comment 28

[Alex Thayer [:alexical] (she/her)]

Woops. This appears to not actually be resolved yet. It appears what I thought was a minor change to resolve a shutdown leak unexpectedly undid the fix.

The fix worked under the expectation that glDeleteTextures was to blame, but after going down the rabbit hole the real culprit is the freeing of the buffer[1] which is happening prematurely. Turns out, as far as I can tell, glFinishObjectAPPLE and glTestObjectAPPLE are just plainly left unimplemented, or else implemented in a way that is useless to us. The delete is just the only thing that happens quickly enough to manifest the problem on nvidia hardware, it seems. Replacing the delete with a memset to 0xff produces a white flash.

Deferring the delete[] makes the problem go away and I can't get it to come back through legitimate use, but it still makes me uneasy that the synchronization mechanism simply doesn't work.

I'm going to bang my head against this a little more.

[1]: https://searchfox.org/mozilla-central/rev/a0333927deabfe980094a14d0549b589f34cbe49/gfx/layers/composite/TextureHost.cpp#1247

Comment 29

[Jeff Muizelaar [:jrmuizel]]

glFinishObjectAPPLE definitely seems to be doing something on Intel hardware:

6.00 ms    0.2%	1.00 ms	 	                   mozilla::layers::DirectMapTextureSource::Sync(bool)
5.00 ms    0.2%	0 s	 	                    gleFinishObject
3.00 ms    0.1%	0 s	 	                     gldFinishObject
3.00 ms    0.1%	0 s	 	                      IOAccelResourceFinishEvent
3.00 ms    0.1%	0 s	 	                       IOConnectCallMethod
3.00 ms    0.1%	0 s	 	                        io_connect_method
3.00 ms    0.1%	0 s	 	                         mach_msg
3.00 ms    0.1%	0 s	 	                          mach_msg_trap
3.00 ms    0.1%	0 s	 	                           mach_msg_overwrite_trap
3.00 ms    0.1%	0 s	 	                            ipc_kmsg_send
3.00 ms    0.1%	0 s	 	                             ipc_kobject_server
3.00 ms    0.1%	0 s	 	                              0xffffff8000342a80
3.00 ms    0.1%	0 s	 	                               is_io_connect_method
3.00 ms    0.1%	0 s	 	                                IOAccelSharedUserClient2::externalMethod(unsigned int, IOExternalMethodArguments*, IOExternalMethodDispatch*, OSObject*, void*)
3.00 ms    0.1%	0 s	 	                                 IOUserClient::externalMethod(unsigned int, IOExternalMethodArguments*, IOExternalMethodDispatch*, OSObject*, void*)
3.00 ms    0.1%	0 s	 	                                  shim_io_connect_method_scalarI_scalarO
3.00 ms    0.1%	0 s	 	                                   IOAccelSharedUserClient2::finish_object_event(unsigned int, unsigned int)
3.00 ms    0.1%	0 s	 	                                    IOAccelEventMachineFast2::finishEventUnlocked(IOAccelEvent*)
3.00 ms    0.1%	0 s	 	                                     IOAccelEventMachine2::waitForStamp(int, unsigned int, unsigned int*)
3.00 ms    0.1%	3.00 ms	 	                                      thread_block_reason
s


I got this from Instruments by setting "Record Kernel Callstacks" in Recording Options and filtering by gleFinishObject.

Comment 30

[Jeff Muizelaar [:jrmuizel]]

You might be able to see if things are working a little bit by just setting a breakpoint on IOAccelResourceFinishEvent

Comment 31

[Alex Thayer [:alexical] (she/her)]

1.00 ms    0.0%	0 s	 	                  mozilla::layers::BufferTextureHost::UnbindTextureSource()
1.00 ms    0.0%	0 s	 	                   mozilla::layers::BufferTextureHost::ReadUnlock()
1.00 ms    0.0%	0 s	 	                    mozilla::layers::DirectMapTextureSource::Sync(bool)
1.00 ms    0.0%	1.00 ms	 	                     gleFinishObject


It ends there. This coupled with the fact that glTestObjectAPPLE only ever returns false makes me pretty confident that it's just left unimplemented. I ended up implementing the same idea using glFenceSync after the texture is used for drawing and glClientWaitSync before unlocking it, and it resolves the issue. I'm sanity checking the performance on intel and AMD hardware now.

Comment 32

[Alex Thayer [:alexical] (she/her)]

(I am a little bit surprised that any samples at all show up for gleFinishObject. I'm not sure what accounts for that. But in any case whatever it is doing is not much.)

Comment 33

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1483533 - Undo deferment of glDeleteTextures r?jrmuizel

Comment 34

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1483533 - Switch to glClientWaitSync for texture syncing r?jrmuizel

glFenceSync/glClientWaitSync just seem to be more well supported
on nvidia hardware, and they work fine as well on AMD/intel, so
I'm transitioning to that.

Depends on D6463

Comment 35

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9010843 [details]
Bug 1483533 - Undo deferment of glDeleteTextures r?jrmuizel

Jeff Muizelaar [:jrmuizel] has approved the revision.

Comment 36

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9010844 [details]
Bug 1483533 - Switch to glClientWaitSync for texture syncing r?jrmuizel

Jeff Muizelaar [:jrmuizel] has approved the revision.

Comment 37

[Alex Thayer [:alexical] (she/her)]

Re: the cost of a fence per texture, it can show up in cases with very large numbers of textures like the motionmark clipped rects test, but it's not a huge contributor. See the profile here: https://perfht.ml/2xFPDZO

That being said it would be nice for it to not show up at all. I could look at a more coarse fencing solution, or I could scope this to only nvidia hardware. What do you think?

Comment 38

[Jeff Muizelaar [:jrmuizel]]

I think it's ok to go with the fence solution for a first pass. Let's look into a other solutions in a follow up.

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa12e9425f90
Undo deferment of glDeleteTextures r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/dc678b66fcb3
Switch to glClientWaitSync for texture syncing r=jrmuizel

Comment 40

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/fa12e9425f90
https://hg.mozilla.org/mozilla-central/rev/dc678b66fcb3

Comment 41

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 9010844 [details]
Bug 1483533 - Switch to glClientWaitSync for texture syncing r?jrmuizel

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1265824
[User impact if declined]: Flickering in the tab bar on OSX with nvidia hardware
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Ideally if we could test on more OSX nvidia devices to confirm the fix.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Mildly
[Why is the change risky/not risky?]: It changes the synchronization mechanism for ClientStorage on OSX. If it went wrong it could lead to similar flickering on different hardware. This is unlikely though.
[String changes made/needed]: None

Comment 42

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9010844 [details]
Bug 1483533 - Switch to glClientWaitSync for texture syncing r?jrmuizel

Fix for a tracked 63 regression, uplift approved for 63 beta 10. Thanks.

Comment 43

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fb0e9934b6ce

Comment 44

[Gabi Cheta [:Gabi] Desktop QA]

Reproduced the issue on Nightly 63.0a1 (2018-08-03)on iMac (21.5-inch, Late 2012) with NVIDIA GeForce GT 640M 512MB macOS 10.14
Verified as fixed using Beta 63.0b10 and Nightly 64.0a1 on iMac (21.5-inch, Late 2012) with NVIDIA GeForce GT 640M 512MB macOS 10.14.