Closed Bug 1369949 Opened 7 years ago Closed 7 years ago

Random appearance of grey vertical bars in tab strip

Categories

(Core :: Graphics, defect)

55 Branch
All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr45 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: Dolske, Assigned: lsalzman)

References

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(4 files, 3 obsolete files)

Attached image Screenshot (deleted) —
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.
Attached image Screenshot 2 (deleted) —
From :jonathanGB, "see inbox in gmail tab & MattN in irccloud tab".
The first time I saw it was in the Friday nightly that included bug 1355764.
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?
Blocks: 1355764
Flags: needinfo?(dao+bmo)
Flags: needinfo?(cadeyrn)
Keywords: regression
Whiteboard: [photon-visual][triage]
I was able to reproduce the issue after disabling the hardware graphics acceleration and restarting Firefox.
Flags: needinfo?(cadeyrn)
This smells like a painting bug. If we have reliable steps to reproduce we can have a layout person look into it.
(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?
Flags: needinfo?(dao+bmo)
Whiteboard: [photon-visual][triage]
Component: Theme → Graphics
Keywords: steps-wanted
OS: Unspecified → Mac OS X
Product: Firefox → Core
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.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Keywords: steps-wanted
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.
Attached file testcase (deleted) —
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.
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.
Blocks: 1340627
No, that's wrong, too. Still debugging.
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.
Flags: needinfo?(lsalzman)
Attachment #8874944 - Attachment is obsolete: true
Attachment #8875058 - Attachment is obsolete: true
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.
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.
Assignee: mstange → lsalzman
Flags: needinfo?(lsalzman)
Attachment #8876225 - Flags: review?(mchang)
Has Regression Range: --- → yes
Has STR: --- → yes
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Version: Trunk → 55 Branch
v2, fix for vector append.
Attachment #8876225 - Attachment is obsolete: true
Attachment #8876225 - Flags: review?(mchang)
Attachment #8876284 - Flags: review?(mchang)
Attachment #8876284 - Flags: review?(mchang) → review+
Pushed by lsalzman@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f40ea9d14a11 compute precise clipping region for DrawTargetSkia::BorrowCGContext. r=mchang
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: