1369949 - Random appearance of grey vertical bars in tab strip

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Justin Dolske [:Dolske]]

Attachment: Screenshot

This seems to be a recent regression (last few days), a couple other people just mentioned seeing it in #fx-team.

I can reproduce this sporadically by wiggling the mouse back and forth on pinned tabs (but that's not the only place it occurs):

1) Pin a bunch of tabs
2) Select one in the middle
3) Rapidly move mouse from left to right, over the pinned tabs

Sometimes takes a minute, but a bar shows up as in screenshot. It will sometimes persist for a few more movements over the tab, and then disappear.

Kinda seems like a gfx invalidation problem, but starting in Themes since we've been changing stuff.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Screenshot 2

From :jonathanGB, "see inbox in gmail tab & MattN in irccloud tab".

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

The first time I saw it was in the Friday nightly that included bug 1355764.

Comment 5

[:Gijs (he/him)]

From the dupe:

(In reply to Sören Hentzschel from comment #0)
> Created attachment 8874412 [details]
> screenshot.png
> 
> Since the landing of the patches in bug 1355764 there are graphics glitches
> in the tab bar of Firefox. It took some time to make sure it was this bug
> which regressed the tab bar because it does not happen all the time. I had
> to use Firefox some time and switch between applications to see the issue. I
> don't know if switching applications was really necessary but I tried to
> simulate a usual working session on my computer to reproduce the issue. In
> the end, there was no doubt for mozregeression that bug 1355764 is the cause:
> 
> 27:22.69 INFO: No more inbound revisions, bisection finished.
> 27:22.69 INFO: Last good revision: 6975992de12186bee51d03dc8a0fea965dbf8ed7
> 27:22.69 INFO: First bad revision: 215cfdee973f3963aabde49296e2805697b4b108
> 27:22.69 INFO: Pushlog:
> https://hg.mozilla.org/integration/autoland/
> pushloghtml?fromchange=6975992de12186bee51d03dc8a0fea965dbf8ed7&tochange=215c
> fdee973f3963aabde49296e2805697b4b108
> 
> 27:25.75 INFO: Looks like the following bug has the  changes which
> introduced the regression:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1355764
> 
> As I said, it does not happen all the time, you have to use Firefox a bit to
> see the issue. It's not always the same position where the bug is, sometimes
> it's in the first tab, sometime another tab, I saw the same issue also with
> the new tab button. Moving the cursor over the tab or new tab button let the
> issue disappear. But it happens again and again while using Firefox.

Dão, any idea what's causing this? Can you reproduce? Is this stuff ifdef'd off for 55 once it goes to beta?

Does disabling hardware graphics acceleration make a difference?

Comment 6

[Sören Hentzschel]

I was able to reproduce the issue after disabling the hardware graphics acceleration and restarting Firefox.

Comment 8

[Markus Stange [:mstange]]

This smells like a painting bug. If we have reliable steps to reproduce we can have a layout person look into it.

Comment 9

[Dão Gottwald [:dao]]

(In reply to :Gijs from comment #5)
> Dão, any idea what's causing this?

Nope.

> Can you reproduce?

Haven't seen it happen yet.

Has somebody seen this somewhere else than Mac?

Comment 10

[Markus Stange [:mstange]]

Steps to reproduce:
 1. Pin two tabs.
 2. Select the second one.
 3. Move your mouse north of the window.
 4. Move your mouse over the first tab by crossing the window's top edge.

Then the glitch appears on the selected pinned tab.

I'll take a look at this.

Comment 11

[Markus Stange [:mstange]]

Attachment: Bug 1369949 - Don't expand the dirty rect for Mac native widget drawing.

Review commit: https://reviewboard.mozilla.org/r/146326/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146326/

Comment 12

[Markus Stange [:mstange]]

The underlying bug has probably been caused by either bug 1059031 or bug 1364007. If it was caused by the latter then I don't completely understand it yet.

I've started a try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=724b89ee514b that will either show the tests that required the 5px dirty rect inflation, or it will be green and make this fix trivial.

Comment 13

[Markus Stange [:mstange]]

Attachment: testcase

Comment 14

[Markus Stange [:mstange]]

The reason why bug 1355764 uncovered this was that we now repaint tabs at the same time as we repaint the urlbar dropmarker, if the mouse crosses the window's top edge. Having two disjunct invalidation regions, with one of them painting a -moz-appearance element, makes this bug more likely to appear.

Comment 15

[Markus Stange [:mstange]]

I finally tracked down the actual cause: Bug 1340627 changed BorrowCGContext so that the returned CGContext wouldn't have the DrawTarget's current clip rect applied to it.

Comment 16

[Markus Stange [:mstange]]

No, that's wrong, too. Still debugging.

Comment 17

[Markus Stange [:mstange]]

The problem is that mCanvas->isClipRect() returns true even though the clip is a union of two disjoint rectangles. getDeviceClipBounds returns the bounding box of that region.

Comment 18

[Markus Stange [:mstange]]

Attachment: Bug 1369949 - Only push device aligned clips.

Review commit: https://reviewboard.mozilla.org/r/146430/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146430/

Comment 19

[Markus Stange [:mstange]]

Comment on attachment 8874944 [details]
Bug 1369949 - Don't expand the dirty rect for Mac native widget drawing.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/146326/diff/1-2/

Comment 20

[Markus Stange [:mstange]]

I've moved these patches to bug 1370757 because they would only paper over the issue. The real bug is in comment 17 and needs a different patch.

Comment 21

[Lee Salzman [:lsalzman]]

Attachment: compute precise clipping region for DrawTargetSkia::BorrowCGContext

Unfortunately for us, in this recent Skia update, SkCanvas::getDeviceClipBounds got revamped to return conservative bounds on the clipping region, rather than the actual clip bounds.

So like Markus suspected, we started with a clipping region that had two disjoint rectangles in it. This region then got intersected with our dirty rect. The way the conservative bounds worked is it took the bounds of the original region, and intersected that with the bounds of the dirty rect, rather than intersecting the dirty rect with the region itself and then recomputing the bounds, like it used to before the Skia update. This erroneously enlarged clip bounds, when used for native widget drawing, allows the widget to show through at the edges in places where it shouldn't.

There is an "undocumented" API function in SkCanvas that lets us pry out the real underlying clipping region (and currently this is the only way), which I can then shove onto the CGContext. This region is tight, so it won't cause the inflation issue we are seeing here.

I had originally actually tried to use this approach during the Skia update, but had mistakenly opted for getDeviceClipBounds instead not realizing the underlying pitfall hidden within.

Comment 22

[Lee Salzman [:lsalzman]]

Attachment: compute precise clipping region for DrawTargetSkia::BorrowCGContext

v2, fix for vector append.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f40ea9d14a11
compute precise clipping region for DrawTargetSkia::BorrowCGContext. r=mchang

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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