Fixes crash due to accessing selection after graph_scene is deleted #218 #230
Conversation
zxlive/edit_panel.py
Outdated
| @@ -106,3 +106,7 @@ def _input_circuit(self) -> None: | |||
| cmd = UpdateGraph(self.graph_view, new_g) | |||
| self.undo_stack.push(cmd) | |||
| self.graph_scene.select_vertices(new_verts) | |||
|
|
|||
| def shutdown(self): | |||
There was a problem hiding this comment.
Why do you need this method here in edit_panel if it is already implemented in base_panel?
zxlive/proof_panel.py
Outdated
| @@ -421,6 +421,10 @@ def _refresh_rewrites_model(self) -> None: | |||
| # TODO: Right now this calls for every single vertex selected, even if we select many at the same time | |||
| self.graph_scene.selectionChanged.connect(model.update_on_selection) | |||
|
|
|||
| def shutdown(self): | |||
There was a problem hiding this comment.
Why do you need this method here in proof_panel if it is already implemented in base_panel?
zxlive/rule_panel.py
Outdated
| @@ -80,3 +80,7 @@ def get_rule(self) -> CustomRule: | |||
| self.graph_scene_right.g, | |||
| self.name_field.text(), | |||
| self.description_field.text()) | |||
|
|
|||
| def shutdown(self): | |||
There was a problem hiding this comment.
Why do you need this method here in rule_panel if it is already implemented in base_panel?
zxlive/graphscene.py
Outdated
|
|
||
| #Clear all items from the scene to ensure no dangling references remain | ||
| self.clear() | ||
|
|
There was a problem hiding this comment.
Do you mind cleaning up some extra empty lines in this function?
|
Hi, @RazinShaikh I have made the suggested changes, here is the commit. |
|
I wonder if it wouldn't be simpler and more future-proof to simply clear the node selection when the user asks to close the app? |
|
@dlyongemallo that actually makes more sense |
| @@ -223,6 +223,12 @@ def active_panel(self) -> Optional[BasePanel]: | |||
|
|
|||
|
|
|||
| def closeEvent(self, e: QCloseEvent) -> None: | |||
| #Call shutdown on all tabs before closing | |||
There was a problem hiding this comment.
I think you may be overthinking this. I think all that is needed to fix the issue is to add a single line in close_tab (just above line 329 self.tab_widget.removeTab(i)), like:
widget.graph_scene.clearSelection()
Please try to see if that works.
Thanks for the PR, Kaushik. Can you make the suggested change and add a test to confirm it works? I think that should suffice. |
Summary
The
MainWindowclass is where I integrated the cleanup routine for theGraphScene.MainWindowhas already acloseEventmethod. This is where we call the shutdown method of eachGraphSceneinstance before the application closes. However, theGraphSceneinstances are not directly managed byMainWindowbut rather by the panels likeGraphEditPanel,ProofPanel, andRulePanelwhich are derived fromBasePanel. Each of these panels contains an instance ofGraphScene.Here's the cleanup implementation:
BasePanelclass.Firslty, I added a
shutdownmethod to theGraphSceneclass. Then, added a corresponding method in theBasePanelclass that callsGraphScene.shutdown()Next, I ensured each panel subclassing
BasePanelclass provides its own shutdown method.Finally, I modified the
closeEventof theMainWindowclass to shutdown on each panel before closing.This method ensures that each
GraphSceneis properly cleaned up before the main window closes.