|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
128.98 KB, video/webm
58 bytes, text/x-review-board-request
|Details | Review|
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
[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?
Thanks, I'll investigate.
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.
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.
Markus, as a simple, low-risk (as in, suitable to uplift to 49) fix, could we simply back out this Part 4 patch?
Good idea, yes, let's do that.
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/
^ 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
Confirmed that the backout fixes the Windows glitch as well in a local build.
> 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.)
Pushed by email@example.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!
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!
Fixed on: Win7_64, Nightly 51, 32bit, ID 20160805030444 (2016-08-05)
Great, thanks for testing! Let's get the fix uplifted then.
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.
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.
Track 49+ as this is a visible regression for Australis submenu.
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')
Rebased onto beta and landed a=gchang: https://hg.mozilla.org/releases/mozilla-beta/rev/b5f952be46cf