GNOME Bugzilla – Bug 683067
Make it easier to see which boxes are turned off
Last modified: 2016-03-31 14:01:37 UTC
Its impossible to tell which boxes are hibernated/paused and which are turned off in the collection view. If you click on a shaded screenshot you expect to end up at something like that screenshot, but if the box was turned off it will show some random "last saved" screenshot, and then when you click it will start booting. I talked about this with jimmac, and the result is that: 1) We only show thumbnails for running and paused VMs (greyed out for paused) 2) For turned off VMs we show a black thumbnail, overlaid with a OS logo if availible. 3) To avoid having black or almost black screensaver/lock-screen thumbnails that look like a turned off VMs we disallow these thumbnails, keeping the last thumbnail instead.
Created attachment 222992 [details] [review] Always show the fallback icon for stopped VMs Showing a random last-saved screenshot isn't right for stopped VMs as this is not at all what will be seen when clicking on them, and its hard to tell which VMs are turned off.
Created attachment 222993 [details] [review] Ignore almost-black screenshots Black and almost black screenshots are almost always screensaver or lock screens. These just make the collection view icons be full of black screenshots after a while, which is not very useful. It is also easy to confuse with turned off boxes which we want to make black. To fix this we just ignore all screenshot that are too dark.
These patches are a step on the way, we now only have to make draw_fallback_vm draw a black with an optional logo on it.
Created attachment 223037 [details] [review] Always show the fallback icon for stopped VMs Showing a random last-saved screenshot isn't right for stopped VMs as this is not at all what will be seen when clicking on them, and its hard to tell which VMs are turned off.
Created attachment 223038 [details] [review] Ignore almost-black screenshots Black and almost black screenshots are almost always screensaver or lock screens. These just make the collection view icons be full of black screenshots after a while, which is not very useful. It is also easy to confuse with turned off boxes which we want to make black. To fix this we just ignore all screenshot that are too dark.
Created attachment 223039 [details] [review] Draw stopped machines as black screenshots Later we want to also overlay logos if availible.
Created attachment 223040 [details] [review] Add new SAVED MachineState This way we can show a greyed out last screenshot in the collection view, rather than a completely black one. While restoring takes a while, the screenshot is typically what you will get back to when restored, so this makes more sense.
With this set things are looking pretty good. All we need is a logo added in draw_stopped_vm() when we have that availible.
Review of attachment 223037 [details] [review]: Looks good but I really think this should simply be merged with 'Draw stopped machines as black screenshots'
Review of attachment 223038 [details] [review]: Assuming by 'ignore' you mean we show fallback icon and you have tested this against a few VMs, ACK!
Review of attachment 223039 [details] [review]: availible -> available ACK otherwise.
Review of attachment 223040 [details] [review]: Looks good otherwise. ::: src/libvirt-machine.vala @@ +125,3 @@ domain.resumed.connect (() => { state = MachineState.RUNNING; }); + domain.stopped.connect (() => { + if (Signal.get_invocation_hint (domain).detail == Quark.from_string ("saved")) you don't need that, you can simply use the signal details syntax and have two handlers: domain.stopped["saved"].connect(..); domain.stopped["otherdetails"].connect(..);
Attachment 223038 [details] pushed as bd3aa14 - Ignore almost-black screenshots Attachment 223039 [details] pushed as d134749 - Draw stopped machines as black screenshots Attachment 223040 [details] pushed as 4f6f421 - Add new SAVED MachineState
There is still an issue where showing a vm and force shutdown it will make it show as paused, not stopped.
Created attachment 223072 [details] [review] Don't save last screenshot on display close if stopped If the display is closed we take a last screenshot and use this. However, it is disconnected due to e.g. a forced shutdown then we incorrectly override black "turned off" screenshot, so check this.
Review of attachment 223072 [details] [review]: "_if_ it is disconnected" in the log Rest of the patch looks good though I'm not very familiar with this code
Attachment 223072 [details] pushed as d1c1434 - Don't save last screenshot on display close if stopped
*** Bug 678463 has been marked as a duplicate of this bug. ***