If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Graphics artifacts in Australis panels, when I open sidebars

VERIFIED FIXED in Firefox 49

Status

()

Core
Layout
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: arni2033, Assigned: botond)

Tracking

({regression})

49 Branch
mozilla51
regression
Points:
---

Firefox Tracking Flags

(firefox47 unaffected, firefox48 unaffected, firefox49+ fixed, firefox50 fixed, firefox51 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Created attachment 8771374 [details]
screencast 1 - Graphics artifacts in Australis panels.webm

>>>   My Info:   Win7_64, Nightly 50, 32bit, ID 20160714030208 (2016-07-14)
STR:
1. Click Australis Menu button in toolbar
2. Click on (?) button in Australis Menu
3. Wait 5 seconds w/o moving mouse

AR:  Graphic artifacts
ER:  No graphic artifacts

This is regression from bug 1273250. Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=abfe6f9b1e082aff84afcf83337747386a2031ef&tochange=eeee65feb9ade388336f2a67786bd444ad58fd64

Updated

a year ago
status-firefox47: --- → unaffected
status-firefox48: --- → unaffected
status-firefox49: --- → affected
status-firefox50: --- → affected
[Tracking Requested - why for this release]: This is a very visible regression in the Firefox menu when opening any of the subviews such as History, Developer, Synced Tabs, Bookmarks, Forget, and others and we shouldn't ship Firefox with it.

Botond, can you take this?
tracking-firefox49: --- → ?
Flags: needinfo?(botond)
(Assignee)

Comment 2

a year ago
Thanks, I'll investigate.
Assignee: nobody → botond
Flags: needinfo?(botond)
(Assignee)

Comment 3

a year ago
Interestingly, I only see the particular glitch depicted in the video on Windows. On Linux, I see a much milder and harder-to-see glitch. Based on the timing I assume it's the same underlying problem.
(Assignee)

Comment 4

a year ago
I've traced the problem to the Part 4 patch of bug 1273250.

That patch took code in FrameLayerBuilder that looked like this:

  bool clipMovesWithLayer = (animatedGeometryRoot == animatedGeometryRootForClip);

  bool shouldFixToViewport = !clipMovesWithLayer &&
      !(*animatedGeometryRoot)->GetParent() &&
      item->ShouldFixToViewport(mBuilder);

and simplified shouldFixToViewport to be just:

  bool shouldFixToViewport = !clipMovesWithLayer;

(and then inlined this definition into use sites).

At the time we believed this to be just a refactoring, but the fact that this caused a regression suggests that the second two conditions are important.

In the affected scenario where those extra conditions would have made shouldFixToViewport false, 'item' has type nsDisplayTransform. I note that nsDisplayTransform was changed to override AnimatedGeometryRootForScrollMetadata() (thus making animatedGeometryRoot != animatedGeometryRootForClip) in bug 1240073.
(Assignee)

Comment 5

a year ago
Markus, as a simple, low-risk (as in, suitable to uplift to 49) fix, could we simply back out this Part 4 patch?
Flags: needinfo?(mstange)
Good idea, yes, let's do that.
Flags: needinfo?(mstange)
(Assignee)

Comment 7

a year ago
Created attachment 8777510 [details]
Bug 1287075 - Back out bug 1273250, part 4 for regressing the rendering of the Australis submenus.

Review commit: https://reviewboard.mozilla.org/r/69048/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69048/
Attachment #8777510 - Flags: review?(mstange)
(Assignee)

Comment 8

a year ago
^ This is a backout of the Part 4 patch, rebased to apply to current trunk.
Comment on attachment 8777510 [details]
Bug 1287075 - Back out bug 1273250, part 4 for regressing the rendering of the Australis submenus.

https://reviewboard.mozilla.org/r/69048/#review66132
Attachment #8777510 - Flags: review?(mstange) → review+
(Assignee)

Comment 10

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef43c0468372
(Assignee)

Comment 11

a year ago
Confirmed that the backout fixes the Windows glitch as well in a local build.
(Assignee)

Comment 12

a year ago
> as a simple, low-risk (as in, suitable to uplift to 49) fix, could
> we simply back out this Part 4 patch?

(Markus and I discussed the possibility of implementing a more proper fix afterwards, and decided that it's not worth doing, because Markus' work in bug 1214146 will rewrite the affected code anyways.)

Comment 13

a year ago
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/354e5ccbd5b9
Back out bug 1273250, part 4 for regressing the rendering of the Australis submenus. r=mstange
Thanks for the quick fix!

Comment 15

a year ago
backoutbugherder
https://hg.mozilla.org/mozilla-central/rev/354e5ccbd5b9
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
(Assignee)

Comment 16

a year ago
The fix should now be in the latest nightly. Arni, could you verify that you no longer see the glitch in the latest nightly? Thanks!
Flags: needinfo?(arni2033)
(Reporter)

Comment 17

a year ago
Fixed on:   Win7_64, Nightly 51, 32bit, ID 20160805030444 (2016-08-05)
Status: RESOLVED → VERIFIED
Flags: needinfo?(arni2033)
(Assignee)

Comment 18

a year ago
Great, thanks for testing! Let's get the fix uplifted then.
(Assignee)

Comment 19

a year ago
Comment on attachment 8777510 [details]
Bug 1287075 - Back out bug 1273250, part 4 for regressing the rendering of the Australis submenus.

Approval Request Comment
[Feature/regressing bug #]:
  Bug 1273250 

[User impact if declined]:
  When a submenu of the hamburger ("Australis") menu (such as
  History, Developer, or Help) is opened, a graphical glitch
  (a few extra columns of pixels being rendered on the left-
  hand side of the window) can be seen briefly. The glitch goes
  away after a second or so, but it's still unsightly.

[Describe test coverage new/current, TreeHerder]:
  The fix was verified by me locally, and by the reporter.

[Risks and why]: 
  Low. The fix reverts one of the patches of the regressing bug,
  thus restoring the affected part of the code to a previous 
  state that was known to work.

[String/UUID change made/needed]:
  None.
Attachment #8777510 - Flags: approval-mozilla-beta?
Attachment #8777510 - Flags: approval-mozilla-aurora?
Comment on attachment 8777510 [details]
Bug 1287075 - Back out bug 1273250, part 4 for regressing the rendering of the Australis submenus.

This patch fixes a regression of Australis submenus. Take it in 49 beta and aurora.
Attachment #8777510 - Flags: approval-mozilla-beta?
Attachment #8777510 - Flags: approval-mozilla-beta+
Attachment #8777510 - Flags: approval-mozilla-aurora?
Attachment #8777510 - Flags: approval-mozilla-aurora+
Track 49+ as this is a visible regression for Australis submenu.
tracking-firefox49: ? → +

Comment 22

a year ago
backoutbugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/d7f9d73f11d8
status-firefox50: affected → fixed
has problems applying to beta

warning: conflicts while merging layout/base/FrameLayerBuilder.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(botond)
(Assignee)

Comment 24

a year ago
Rebased onto beta and landed a=gchang:

https://hg.mozilla.org/releases/mozilla-beta/rev/b5f952be46cf
Flags: needinfo?(botond)
Thanks!
status-firefox49: affected → fixed
You need to log in before you can comment on or make changes to this bug.