From 5dc8293f335e66f54731cf1d13956cf241fdba20 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 11 Dec 2023 12:02:37 +0000 Subject: [PATCH 1/4] Window : Add `preCloseSignal()` --- Changes.md | 1 + python/GafferUI/Window.py | 9 ++++++++- python/GafferUITest/WindowTest.py | 26 ++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/Changes.md b/Changes.md index c1813c7a8e0..722ed177eae 100644 --- a/Changes.md +++ b/Changes.md @@ -36,6 +36,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. diff --git a/python/GafferUI/Window.py b/python/GafferUI/Window.py index 7ad920b83e8..b0b0f6872c8 100644 --- a/python/GafferUI/Window.py +++ b/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. diff --git a/python/GafferUITest/WindowTest.py b/python/GafferUITest/WindowTest.py index 07ab2fb8c02..aac7ce9d96e 100644 --- a/python/GafferUITest/WindowTest.py +++ b/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() From b75656ff3ae1ed9795cb4b39eb05f54a62205811 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 11 Dec 2023 13:38:50 +0000 Subject: [PATCH 2/4] GUI Config : Add LocalDispatcher shutdown checks Since all running jobs will be killed during shutdown prompt the user when closing the last ScriptWindow, offering the option of cancelling shutdown,. --- Changes.md | 1 + startup/gui/localDispatcher.py | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/Changes.md b/Changes.md index 722ed177eae..71485b4c31b 100644 --- a/Changes.md +++ b/Changes.md @@ -11,6 +11,7 @@ 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. diff --git a/startup/gui/localDispatcher.py b/startup/gui/localDispatcher.py index 1c1695e4883..f5f0a061b6c 100644 --- a/startup/gui/localDispatcher.py +++ b/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 ) From d00da75f34abfe8bd2e78d8e0095c4fe813d7ce4 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 11 Dec 2023 14:02:27 +0000 Subject: [PATCH 3/4] ApplicationMenu : Reimplement Quit by closing windows one by one This means we're using the ScriptWindow's own discard/cancel prompts rather than duplicating them here. Which also means that the LocalDispatcher kill-incomplete-jobs? prompt is shown on quit. --- python/GafferUI/ApplicationMenu.py | 40 +++++++++--------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/python/GafferUI/ApplicationMenu.py b/python/GafferUI/ApplicationMenu.py index 3b057384868..a18dc409479 100644 --- a/python/GafferUI/ApplicationMenu.py +++ b/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 ) : From 9a13360441f4f4fc0ad15726e1c24c3bdeb96f36 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 11 Dec 2023 14:34:21 +0000 Subject: [PATCH 4/4] ScriptWindow : Add "Save" option to "Discard Changes?" dialogue --- Changes.md | 1 + python/GafferUI/ScriptWindow.py | 43 ++++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/Changes.md b/Changes.md index 71485b4c31b..4d6d01f14fe 100644 --- a/Changes.md +++ b/Changes.md @@ -14,6 +14,7 @@ Improvements - 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 ----- diff --git a/python/GafferUI/ScriptWindow.py b/python/GafferUI/ScriptWindow.py index 45b98b122ff..aa7393df199 100644 --- a/python/GafferUI/ScriptWindow.py +++ b/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 :