Closed
Bug 1369949
Opened 7 years ago
Closed 7 years ago
Random appearance of grey vertical bars in tab strip
Categories
(Core :: Graphics, defect)
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)
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.
Reporter | ||
Comment 1•7 years ago
|
||
From :jonathanGB, "see inbox in gmail tab & MattN in irccloud tab".
Comment 2•7 years ago
|
||
The first time I saw it was in the Friday nightly that included bug 1355764.
Comment 5•7 years ago
|
||
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?
Updated•7 years ago
|
Whiteboard: [photon-visual][triage]
Comment 6•7 years ago
|
||
I was able to reproduce the issue after disabling the hardware graphics acceleration and restarting Firefox.
Flags: needinfo?(cadeyrn)
Comment 8•7 years ago
|
||
This smells like a painting bug. If we have reliable steps to reproduce we can have a layout person look into it.
Comment 9•7 years ago
|
||
(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)
Updated•7 years ago
|
Whiteboard: [photon-visual][triage]
Updated•7 years ago
|
Comment 10•7 years ago
|
||
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
Updated•7 years ago
|
Keywords: steps-wanted
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
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•7 years ago
|
||
Comment 14•7 years ago
|
||
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•7 years ago
|
||
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
Comment 16•7 years ago
|
||
No, that's wrong, too. Still debugging.
Comment 17•7 years ago
|
||
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)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8874944 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8875058 -
Attachment is obsolete: true
Comment 20•7 years ago
|
||
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.
Assignee | ||
Comment 21•7 years ago
|
||
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 | ||
Updated•7 years ago
|
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox53:
--- → unaffected
status-firefox54:
--- → unaffected
status-firefox55:
--- → affected
status-firefox-esr45:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Version: Trunk → 55 Branch
Assignee | ||
Comment 22•7 years ago
|
||
v2, fix for vector append.
Attachment #8876225 -
Attachment is obsolete: true
Attachment #8876225 -
Flags: review?(mchang)
Attachment #8876284 -
Flags: review?(mchang)
Updated•7 years ago
|
Attachment #8876284 -
Flags: review?(mchang) → review+
Comment 23•7 years ago
|
||
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f40ea9d14a11
compute precise clipping region for DrawTargetSkia::BorrowCGContext. r=mchang
Comment 24•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
status-firefox57:
affected → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•