Merge pull request GafferHQ#5586 from johnhaddon/localDispatcherShutd…

…ownChecks

GUI Config : Add LocalDispatcher shutdown checks

Changes.md

@@ -11,8 +11,10 @@ Improvements
   - Added a new dockable LocalJobs editor, to replace the floating window previously accessible via the "Execute/Local Jobs" menu item.
   - Task output is now shown in the UI.
   - Jobs are no longer removed from the UI as soon as they complete.
+  - Incomplete jobs are now killed automatically when the application is closed, after prompting to confirm that shutdown should go ahead.
 - Cache : Increased default computation cache size to 8Gb. Call `Gaffer.ValuePlug.setCacheMemoryLimit()` from a startup file to override this.
 - Dispatcher : Reduced internal overhead of `dispatch()` call, with one benchmark showing around a 3x speedup.
+- ScriptWindow : Added "Save" option to dialogue shown when closing a window containing unsaved changes.
 
 Fixes
 -----
@@ -36,6 +38,7 @@ API
 - IconPathColumn :
   - Added constructor which allows the full header CellData to be specified.
   - Added `prefix()` and `property()` accessors.
+- Window : Added `preCloseSignal()`, which allows connected slots to prevent a window from being closed.
 - LocalDispatcher :
   - Added `Job.status()` and `Job.statusChangedSignal()` methods.
   - Added `Job.messages()` and `Job.messagesChangedSignal()` methods.

python/GafferUI/ApplicationMenu.py

@@ -55,37 +55,21 @@ def quit( menu ) :
 	scriptWindow = menu.ancestor( GafferUI.ScriptWindow )
 	application = scriptWindow.scriptNode().ancestor( Gaffer.ApplicationRoot )
 
-	unsavedNames = []
-	for script in application["scripts"].children() :
-		if script["unsavedChanges"].getValue() :
-			f = script["fileName"].getValue()
-			f = f.rpartition( "/" )[2] if f else "untitled"
-			unsavedNames.append( f )
-
-	if unsavedNames :
-
-		dialogue = GafferUI.ConfirmationDialogue(
-			"Discard Unsaved Changes?",
-			"The following files have unsaved changes : \n\n" +
-			"\n".join( [ " - " + n for n in unsavedNames ] ) +
-			"\n\nDo you want to discard the changes and quit?",
-			confirmLabel = "Discard and Quit"
-		)
+	# Defer the actual closing of windows till an idle event, because our menu
+	# item is a child of one of the windows, and deleting it now could cause a crash.
+	GafferUI.EventLoop.addIdleCallback( functools.partial( __closeAllScriptWindows, application ) )
 
-		if not dialogue.waitForConfirmation( parentWindow=scriptWindow ) :
-			return
-
-	# Defer the actual removal of scripts till an idle event - removing all
-	# the scripts will result in the removal of the window our menu item is
-	# parented to, which would cause a crash as it's deleted away from over us.
-	GafferUI.EventLoop.addIdleCallback( functools.partial( __removeAllScripts, application ) )
-
-def __removeAllScripts( application ) :
+def __closeAllScriptWindows( application ) :
 
 	for script in application["scripts"].children() :
-		application["scripts"].removeChild( script )
-
-	return False # remove idle callback
+		window = GafferUI.ScriptWindow.acquire( script, createIfNecessary = False )
+		if window is None :
+			continue
+		if not window.close() :
+			# Window refused to close, cancelling the Quit action.
+			break
+
+	return False # Remove idle callback
 
 __aboutWindow = None
 def about( menu ) :

python/GafferUI/ScriptWindow.py

@@ -118,12 +118,22 @@ def _acceptsClose( self ) :
 		f = self.__script["fileName"].getValue()
 		f = f.rpartition( "/" )[2] if f else "untitled"
 
-		dialogue = GafferUI.ConfirmationDialogue(
+		dialogue = _ChoiceDialogue(
 			"Discard Unsaved Changes?",
-			"The file %s has unsaved changes. Do you want to discard them?" % f,
-			confirmLabel = "Discard"
+			f"The file \"{f}\" has unsaved changes. Do you want to discard them?",
+			choices = [ "Cancel", "Save", "Discard" ]
 		)
-		return dialogue.waitForConfirmation( parentWindow=self )
+		choice = dialogue.waitForChoice( parentWindow=self )
+
+		if choice == "Discard" :
+			return True
+		elif choice == "Save" :
+			## \todo Is it a bit odd that ScriptWindow should depend on FileMenu
+			# like this? Should the code be moved somewhere else?
+			GafferUI.FileMenu.save( self.menuBar() )
+			return True
+		else :
+			return False
 
 	def __closed( self, widget ) :
 
@@ -194,6 +204,31 @@ def __staticScriptRemoved( scriptContainer, script ) :
 		if not len( scriptContainer.children() ) and GafferUI.EventLoop.mainEventLoop().running() :
 			GafferUI.EventLoop.mainEventLoop().stop()
 
+## \todo Would this be worthy of inclusion in GafferUI?
+class _ChoiceDialogue( GafferUI.Dialogue ) :
+
+	def __init__( self, title, message, choices, **kw ) :
+
+		GafferUI.Dialogue.__init__( self, title, sizeMode=GafferUI.Window.SizeMode.Automatic, **kw )
+
+		with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Vertical, spacing = 8 ) as column :
+
+			GafferUI.Label( message )
+
+		self._setWidget( column )
+
+		for choice in choices :
+			self.__lastButton = self._addButton( choice )
+
+	def waitForChoice( self, **kw ) :
+
+		self.__lastButton._qtWidget().setFocus()
+		button = self.waitForButton( **kw )
+
+		if button is None :
+			return None
+		else :
+			return button.getText()
 
 class _WindowTitleBehaviour :
 

python/GafferUI/Window.py

@@ -94,6 +94,7 @@ def __init__( self, title="GafferUI.Window", borderWidth=0, resizeable=None, chi
 		else :
 			self.setSizeMode( sizeMode )
 
+		self.__preCloseSignal = GafferUI.WidgetSignal()
 		self.__closedSignal = GafferUI.WidgetSignal()
 
 		self.setChild( child )
@@ -359,13 +360,19 @@ def close( self ) :
 		if not self.getVisible() :
 			return False
 
-		if self._acceptsClose() :
+		if self._acceptsClose() and not self.__preCloseSignal( self ) :
 			self.setVisible( False )
 			self.closedSignal()( self )
 			return True
 		else :
 			return False
 
+	## Emitted when `close()` is called. Slots may return `True` to prevent
+	# the window from being closed.
+	def preCloseSignal( self ) :
+
+		return self.__preCloseSignal
+
 	## Subclasses may override this to deny the closing of a window triggered
 	# either by user action or by a call to close(). Simply return False to
 	# prevent the closing.

python/GafferUITest/WindowTest.py

@@ -46,6 +46,7 @@
 import IECore
 
 import Gaffer
+import GafferTest
 import GafferUI
 import GafferUITest
 
@@ -459,5 +460,30 @@ def testChildWindowDuringShutdown( self ) :
 			# If the bug is fixed, nothing should have been printed.
 			self.assertEqual( tmpStdErr.getvalue(), "" )
 
+	def testPreCloseSignal( self ) :
+
+		window = GafferUI.Window()
+		window.setVisible( True )
+
+		preCloseSlotResult = True
+		def preCloseSlot( w ) :
+
+			nonlocal preCloseSlotResult
+			return preCloseSlotResult
+
+		window.preCloseSignal().connect( preCloseSlot, scoped = False )
+		preCloseCapturingSlot = GafferTest.CapturingSlot( window.preCloseSignal() )
+		closedSlot = GafferTest.CapturingSlot( window.closedSignal() )
+
+		self.assertFalse( window.close() )
+		self.assertEqual( len( preCloseCapturingSlot ), 0 )
+		self.assertEqual( len( closedSlot ), 0 )
+
+		preCloseSlotResult = False
+
+		self.assertTrue( window.close() )
+		self.assertEqual( len( preCloseCapturingSlot ), 1 )
+		self.assertEqual( len( closedSlot ), 1 )
+
 if __name__ == "__main__":
 	unittest.main()

startup/gui/localDispatcher.py

@@ -36,6 +36,52 @@
 
 import Gaffer
 import GafferDispatch
+import GafferUI
+import GafferDispatchUI
 
 Gaffer.Metadata.registerValue( GafferDispatch.LocalDispatcher, "executeInBackground", "userDefault", True )
 GafferDispatch.Dispatcher.setDefaultDispatcherType( "Local" )
+
+def __scriptWindowPreClose( scriptWindow ) :
+
+	numScripts = len( scriptWindow.scriptNode().parent() )
+	if numScripts > 1 :
+		return False
+
+	# The last window is about to be closed, which will quit the
+	# application. Check for LocalJobs that are still running,
+	# and prompt the user.
+
+	incompleteJobs = [
+		job for job in
+		GafferDispatch.LocalDispatcher.defaultJobPool().jobs()
+		if job.status() in (
+			GafferDispatch.LocalDispatcher.Job.Status.Waiting,
+			GafferDispatch.LocalDispatcher.Job.Status.Running,
+		)
+	]
+
+	if len( incompleteJobs ) == 0 :
+		return False
+
+	dialogue = GafferUI.ConfirmationDialogue(
+		"Kill Incomplete Jobs?",
+		"{} LocalDispatcher job{} still running and will be killed".format(
+			len( incompleteJobs ),
+			"s are" if len( incompleteJobs ) > 1 else " is"
+		),
+		confirmLabel = "Kill"
+	)
+
+	# If `Cancel` was pressed, prevent the window from being closed.
+	return dialogue.waitForConfirmation( parentWindow = scriptWindow ) == False
+
+def __scriptAdded( container, script ) :
+
+	window = GafferUI.ScriptWindow.acquire( script, createIfNecessary = False )
+	if window is None :
+		return
+
+	window.preCloseSignal().connect( __scriptWindowPreClose, scoped = False )
+
+application.root()["scripts"].childAddedSignal().connect( __scriptAdded, scoped = False )