GNOME Bugzilla – Bug 694227
Tracker bug for regressions in background handling
Last modified: 2013-02-20 16:37:13 UTC
Misc stuff, as I find it while testing.
Created attachment 236862 [details] [review] WorkspaceThumbnail: fix typo in stacking code Was causing thumbnails to be below the background.
Review of attachment 236862 [details] [review]: OK.
Created attachment 236868 [details] [review] Background: ignore cancellation errors If the request cancellable is cancelled, the requesting background was destroyed, so don't bother reporting the result.
Comment on attachment 236862 [details] [review] WorkspaceThumbnail: fix typo in stacking code Attachment 236862 [details] pushed as f3df62f - WorkspaceThumbnail: fix typo in stacking code
Created attachment 236873 [details] [review] layout: Hide the message tray and OSK during the startup animation
Created attachment 236874 [details] [review] LayoutManager: add missing constant
Created attachment 236875 [details] [review] BackgroundMenu: fix regression from new background handling BackgroundMenu must ensure the actor it attaches to is reactive, and the layout manager must create a background menu for the first background too. Not ready to go in now, needs the startingUp fix.
Review of attachment 236873 [details] [review]: LGTM
Created attachment 236876 [details] [review] layout: add back BACKGROUND_ANIMATION_FADE_TIME constant I lost it at the last minute when refactoring some code following review.
Created attachment 236877 [details] [review] layout: add back startingUp state variable I dropped it on accident during last minute refactoring
Created attachment 236878 [details] [review] layout: invert isGreeter check I somehow lost an exclamation point.
Review of attachment 236874 [details] [review]: oh you already found this one.
Review of attachment 236876 [details] [review]: Lol (see comment #6)
Review of attachment 236877 [details] [review]: Ok
Review of attachment 236878 [details] [review]: Ah, I see the logic now.
Review of attachment 236875 [details] [review]: seems fine.
Comment on attachment 236874 [details] [review] LayoutManager: add missing constant Attachment 236874 [details] pushed as e4af954 - LayoutManager: add missing constant
Comment on attachment 236868 [details] [review] Background: ignore cancellation errors (pushing a slightly different patch after discussion with gcampax on irc)
Attachment 236873 [details] pushed as 1188b1b - layout: Hide the message tray and OSK during the startup animation Attachment 236875 [details] pushed as 079041c - BackgroundMenu: fix regression from new background handling Attachment 236877 [details] pushed as 6484da2 - layout: add back startingUp state variable Attachment 236878 [details] pushed as 905c493 - layout: invert isGreeter check
Created attachment 236880 [details] [review] screenShield: don't use Bin for BackgroundManager container St.Bin() really only expects one child at a time, and the BackgroundManager will add two. This can cause assertion failures when destroying one of the background actors.
Created attachment 236881 [details] [review] background: destroy actors don't remove them from their parent This reads better, and it also fixes a warning when background is changed while transitioning to the overview.
Review of attachment 236881 [details] [review]: OK.
(pushing both since the second patch seemingly causes Clutter:ERROR:./clutter-actor.c:5778:clutter_actor_dispose: assertion failed: (priv->parent == NULL) without the first) Attachment 236880 [details] pushed as 34443da - screenShield: don't use Bin for BackgroundManager container Attachment 236881 [details] pushed as dcacbb3 - background: destroy actors don't remove them from their parent
Created attachment 236890 [details] [review] layout: Don't bother updating the regions in the greeter The greeter is always full-screen, so this should never matter.
Created attachment 236891 [details] [review] layout: Remove freezeUpdateRegions/thawUpdateRegions Due to a bad rebase causing freezeUpdateCount to never get initialized, these functions effectively did nothing. Since we're going to go to a different mechanism for freezing region updates, let's just tear these out now instead of fixing them before tearing them out.
Created attachment 236892 [details] [review] layout: Rebuild struts and the input region when the Shell starts up This means that windows will be positioned correctly with respect to the panel when the shell starts up, and there won't be adjusting after the session animation zooms in entirely.
Created attachment 236893 [details] [review] layout: Clean up consoleBackgroundGroup We shouldn't leave a reference to a destroyed actor just hanging around.
Created attachment 236894 [details] [review] layout: Set the no_clear_hint on initialize With the new startup animations and background work, we should never see the stage.
Review of attachment 236890 [details] [review]: Does this fix anything or is it just an micro optimization?
Review of attachment 236891 [details] [review]: OK.
Review of attachment 236892 [details] [review]: OK.
Review of attachment 236890 [details] [review]: It's more of a code cleanup that came out after I started to remove freezeUpdateRegions / thawUpdateRegions. Not essential.
Review of attachment 236893 [details] [review]: OK.
Review of attachment 236894 [details] [review]: This is needed for multi monitor setups, there will be empty areas of the stage (visible in screenshots and/or screencasts), when the monitors have different dimensions.
Review of attachment 236894 [details] [review]: Does this mean that the no_clear_hint before is wrong?
(In reply to comment #35) > Review of attachment 236894 [details] [review]: > > Does this mean that the no_clear_hint before is wrong? was talking about the bg color see bug 683514 ... the clear hint prevents clutter from clearing the stage on every paint (which isn't cheap with a large stage as ours). So strictly speaking yes (even though it seems to work this way) but I wouldn't remove it because most of the time we don't have such areas visible. We probably should unset it if we end up with such a geometry but otherwise leave it for the more common case.
that bug suggests we should put a background with noise-texture there
Attachment 236890 [details] pushed as de3d3c1 - layout: Don't bother updating the regions in the greeter Attachment 236891 [details] pushed as d5d5177 - layout: Remove freezeUpdateRegions/thawUpdateRegions Attachment 236892 [details] pushed as 5d3c901 - layout: Rebuild struts and the input region when the Shell starts up Attachment 236893 [details] pushed as b4678a1 - layout: Clean up consoleBackgroundGroup OK, let's push this for now.